completion for remote pdb is not ideal

[gaogaotiantian]

Bug report

Bug description:

Due to various reasons, remote pdb client does not sync with the server entirely. For example, for multi-line inputs, it will read the full content on the client then send it to the server.

However, this introduced the issue for completion - the server doesn't know it. When user requests a completion for the multi-line code, server pdb doesn't know what completion should be provided (more specifically, whether pdb commands should be provided).

The simplest way to deal with it is to add an extra field "mode" in completion protocol - unless we sync the client and server perfectly, we will need it because the server knows nothing about the client.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-133351: Fix Cmd completion for lines beginning with !  #133362
• gh-133351: Fix remote PDB's multi-line block tab completion #133387

[gaogaotiantian]

I put it under the bug label for now, but maybe it's a feature. It's very close to beta freeze now, I think we should at least update the protocol so we can try to keep the protocol version and solve the issue for 3.14. I don't think the code is too complicated.

@pablogsal @godlygeek

[godlygeek]

Hm. I see two alternative fixes that wouldn't require changing the protocol schema:

1. If we're doing a multi-line input and asking for completion for a line other than the first, prefix the line we send to the server with ! so that the server knows it must be a Python command. We'll need to adjust the offsets by 1, but otherwise I think things would Just Work.

2. Maybe we should send the whole buffered multi-line input to the server, instead of just the current line? PDB doesn't currently use that information in its completions, but maybe it should, in a perfect world. As an example, imagine someone writes:

(PDB) def foo():
...       some_long_variable_name = []
...       some<Tab>

PDB can't currently complete the local variable name, but wouldn't it be nice if it could? It could heuristically do that by parsing the written-but-not-yet-executed text looking for assignment statements and adding the names on the LHS to its list of possible completions. But for that to work, the server would need to get the full context of what's buffered, not just the last line of it. (Admittedly though this is a slippery slope, you'll never get something perfect without being able to actually execute the code and introspect its objects, so it's reasonable to decide that you don't want to implement this feature at all because of the fact that there would always be requests to do more - like recognize that it's a list and complete list methods for it, or the like.)

[gaogaotiantian]

I don't like sending with ! because that's the explicit "pdb command" indicator for pdb. The server needs to know that this is something that should not include pdb command completions.

PyREPL does not even handle the local variable names defined in the previous lines for now. I think that is rather complicated because that requires constantly compile code that might not be compilable.

Even if you passed the whole multi-line string, you probably will need some logic to figure out whether that's a Python command or not (checking \n? not sure if it's reliable). The existing protocol is mimicking what readline actually does - if we change the text, we probably will simulate something that readline is not supposed to deal with (does readline work with multiline context?).

If you think it's easier to deal with this case with a full context, we can try that.

[godlygeek]

I don't like sending with ! because that's the explicit "pdb command" indicator for pdb. The server needs to know that this is something that should not include pdb command completions.

Huh. It seems like this is something that regular PDB gets wrong, actually. If I do:

(Pdb) !apple = "apple"
(Pdb) !abba = "abba"
(Pdb) !a<Tab>

I would expect it to complete the two variable names, since the ! means that it's completing in the context of a Python statement to be exec'd, but it doesn't show any completions.

If we fix that, so that

(Pdb) !anything<Tab>

and

(Pdb) if True:
...       anything<Tab>

show the same completions, then my quick fix will work.

[godlygeek]

There's a cmd.Cmd bug here, that's why it's not working.

cpython/Lib/cmd.py

Lines 275 to 280 in 2bc8365

	cmd, args, foo = self.parseline(line)
	if cmd == '':
	compfunc = self.completedefault
	else:
	try:
	compfunc = getattr(self, 'complete_' + cmd)

parseline can return with cmd set to None, and if it does the if cmd == '': block doesn't run, so it instead tries to do compfunc = getattr(self, 'complete_' + cmd), but since cmd is None that fails with a TypeError instead of the AttributeError that it's expecting to handle, and so it never falls back to completedefault like it was meant to.

After fixing that, ! a<Tab> (with a space after the !) does complete the 'a' names that it should, and we should be able to get the behavior that we want by having remote PDB send a "! " prefix for the line, and incrementing both the indexes by 2.

[gaogaotiantian]

Ah right my brain went somewhere else. ! should forces default behavior.

Do you want to make a PR to fix that? I can do it too. Either change line 276 to cmd == '' or cmd is None or just flip the logic and do if cmd:. We do need regression test and a news entry.

[godlygeek]

After #133364 is applied I think this patch makes completion do what you want:

diff --git a/Lib/pdb.py b/Lib/pdb.py
index 343cf4404d7..eaea762e2db 100644
--- a/Lib/pdb.py
+++ b/Lib/pdb.py
@@ -2933,6 +2933,7 @@ def __init__(self, pid, sockfile, interrupt_script):
         self.completion_matches = []
         self.state = "dumb"
         self.write_failed = False
+        self.multiline_block = False

     def _ensure_valid_message(self, msg):
         # Ensure the message conforms to our protocol.
@@ -2979,6 +2980,7 @@ def _send(self, **kwargs):
             self.write_failed = True

     def read_command(self, prompt):
+        self.multiline_block = False
         reply = input(prompt)

         if self.state == "dumb":
@@ -3003,6 +3005,7 @@ def read_command(self, prompt):
             return prefix + reply

         # Otherwise, valid first line of a multi-line statement
+        self.multiline_block = True
         continue_prompt = "...".ljust(len(prompt))
         while codeop.compile_command(reply, "<stdin>", "single") is None:
             reply += "\n" + input(continue_prompt)
@@ -3105,9 +3108,13 @@ def complete(self, text, state):

             origline = readline.get_line_buffer()
             line = origline.lstrip()
-            stripped = len(origline) - len(line)
-            begidx = readline.get_begidx() - stripped
-            endidx = readline.get_endidx() - stripped
+            if self.multiline_block:
+                # We're completing a line contained in a multi-line block.
+                # Force the remote to treat it as a Python expression.
+                line = "! "
+            offset = len(origline) - len(line)
+            begidx = readline.get_begidx() - offset
+            endidx = readline.get_endidx() - offset

             msg = {
                 "complete": {

Can you try that out and see if you spot any other problems?

[gaogaotiantian]

Is this correct? Shouldn't it be line = "! " + line?

[godlygeek]

Hah! Yes it should. Believe it or not it actually works even with that bug! 😆

[godlygeek]

It does also work with that bug fixed, so we should definitely do what you suggest, heh

[gaogaotiantian]

I guess text was the only thing used in completedefault in pdb.Pdb so the others do not matter, but yeah we should fix it or it will confuse the people in the future.

[godlygeek]

I think this can be closed now