Visitar URL original
completion for remote pdb is not ideal · Issue #133351 · python/cpython · GitHub
Skip to content

completion for remote pdb is not ideal #133351

Description

@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

Activity

  1. gaogaotiantian commented on May 3, 2025

    @gaogaotiantian
    MemberAuthor

    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

  2. godlygeek commented on May 3, 2025

    @godlygeek
    Contributor

    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.)

  3. gaogaotiantian commented on May 4, 2025

    @gaogaotiantian
    MemberAuthor

    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.

  4. godlygeek commented on May 4, 2025

    @godlygeek
    Contributor

    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.

  5. godlygeek commented on May 4, 2025

    @godlygeek
    Contributor

    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.

  6. gaogaotiantian commented on May 4, 2025

    @gaogaotiantian
    MemberAuthor

    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.

  7. godlygeek commented on May 4, 2025

    @godlygeek
    Contributor

    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?

  8. gaogaotiantian commented on May 4, 2025

    @gaogaotiantian
    MemberAuthor

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

  9. godlygeek commented on May 4, 2025

    @godlygeek
    Contributor

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

  10. godlygeek commented on May 4, 2025

    @godlygeek
    Contributor

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

  11. gaogaotiantian commented on May 4, 2025

    @gaogaotiantian
    MemberAuthor

    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.

  12. added
    stdlibStandard Library Python modules in the Lib/ directory
    on May 4, 2025
  13. added a commit that references this issue on May 4, 2025
  14. godlygeek commented on May 4, 2025

    @godlygeek
    Contributor

    I think this can be closed now

  15. added a commit that references this issue on Jul 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions