Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-110944: Make pdb completion work for alias and convenience vars #110945

Merged
merged 7 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion Lib/pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def __init__(self, completekey='tab', stdin=None, stdout=None, skip=None,
try:
import readline
# remove some common file name delimiters
readline.set_completer_delims(' \t\n`@#$%^&*()=+[{]}\\|;:\'",<>?')
readline.set_completer_delims(' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no tests in this PR.

What could this change break?

Copy link
Member Author

@gaogaotiantian gaogaotiantian Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no test for completion, presumably because it's not easy to test. It would change the behavior when the user tries to do a complete (hit <tab> in most cases) and there's a $ in the line. In that case, $ would be not considered as a delimiter (the text to be completed would include $). I can't think of any real cases where this matters. Actually, I can't think of a case where the user uses $ in their pdb commands (other than using convenience variable of course).

except ImportError:
pass
self.allow_kbdint = False
Expand Down Expand Up @@ -686,6 +686,18 @@ def set_convenience_variable(self, frame, name, value):
# Generic completion functions. Individual complete_foo methods can be
# assigned below to one of these functions.

def completenames(self, text, line, begidx, endidx):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is overwritting the completenames method defined in cmd.Cmd, which is called when the user tries to complete the command.

Pdb class has some implicit inheritance from cmd.Cmd class to achieve customized behaviors. I can add some comments here if it helps.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding a test that covers this would be better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you can (easily) set up a readline test to really trigger the "complete" feature of readline. Even test_cmd does not test against complete features for cmd.Cmd. I can add some unitttest just to test this method (feeding mock arguments to the function), but a real functional test involving readline would be a large amount of work - and we should probably do that for cmd first if we consider adding it.

# Overwrite completenames() of cmd so for the command completion,
# if no current command matches, check for expressions as well
commands = super().completenames(text, line, begidx, endidx)
for alias in self.aliases:
if alias.startswith(text):
commands.append(alias)
if commands:
return commands
else:
return self._complete_expression(text, line, begidx, endidx)

def _complete_location(self, text, line, begidx, endidx):
# Complete a file/module/function location for break/tbreak/clear.
if line.strip().endswith((':', ',')):
Expand Down Expand Up @@ -720,6 +732,10 @@ def _complete_expression(self, text, line, begidx, endidx):
# complete builtins, and they clutter the namespace quite heavily, so we
# leave them out.
ns = {**self.curframe.f_globals, **self.curframe_locals}
if text.startswith("$"):
# Complete convenience variables
conv_vars = self.curframe.f_globals.get('__pdb_convenience_variables', {})
return [f"${name}" for name in conv_vars if name.startswith(text[1:])]
if '.' in text:
# Walk an attribute chain up to the last part, similar to what
# rlcompleter does. This will bail if any of the parts are not
Expand Down
21 changes: 21 additions & 0 deletions Lib/test/test_pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -3289,6 +3289,27 @@ def test_basic_completion(self):
self.assertIn(b'continue', output)
self.assertIn(b'hello!', output)

def test_expression_completion(self):
script = textwrap.dedent("""
value = "speci"
import pdb; pdb.Pdb().set_trace()
""")

# Complete: value + 'al'
input = b"val\t + 'al'\n"
# Complete: p value + 'es'
input += b"p val\t + 'es'\n"
# Complete: $_frame
input += b"$_fra\t\n"
# Continue
input += b"c\n"

output = run_pty(script, input)

self.assertIn(b'special', output)
self.assertIn(b'species', output)
self.assertIn(b'$_frame', output)


def load_tests(loader, tests, pattern):
from test import test_pdb
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support alias and convenience vars for :mod:`pdb` completion
Loading