GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-133363: Fix Cmd completion for lines beginning with `! ` by godlygeek · Pull Request #133364 · python/cpython

Conversation

@godlygeek

@godlygeek godlygeek commented May 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

When a line begins with ! and there's no do_shell method defined, parseline returns None as the cmd, which incorrectly leads to None being concatenated to complete_ and triggering a TypeError.

Instead, recognize None as a sentinel that means we should call completedefault, as an empty string already is.

  • Issue: cmd.Cmd fails tab completion for lines beginning with ! (bang space) #133363

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@godlygeek
Fix Cmd completion for lines beginning with !
12e2495 
When a line begins with `!` and there's no `do_shell` method defined,
`parsecmd` returns `None` as the `cmd`, which incorrectly leads to
`None` being concatenated to `complete_` and triggering a `TypeError`.

Instead, recognize `None` as a sentinel that means we should call
`completedefault`, as an empty string already is.
@bedevere-app bedevere-app Bot added the awaiting review label May 4, 2025
@bedevere-app bedevere-app Bot mentioned this pull request May 4, 2025
cmd.Cmd fails tab completion for lines beginning with ! (bang space) #133363
Closed
@godlygeek godlygeek mentioned this pull request May 4, 2025
gh-133351: Fix Cmd completion for lines beginning with ! #133362
Closed
gaogaotiantian
gaogaotiantian reviewed May 4, 2025
View reviewed changes
Comment thread Lib/cmd.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
gaogaotiantian
gaogaotiantian reviewed May 4, 2025
View reviewed changes
Comment thread Lib/test/test_cmd.py Outdated

output = run_pty(script, input)

self.assertIn(b'ello', output)

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ello because that's what is completed? hello is not part of the output?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Oh, because that's what is used in the previous test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@godlygeek godlygeek May 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ello because that's what's completed, yes. hello is part of the output, but I'm just matching the test above that I copy-pasted from, which only checks for ab_completion_test, heh

The tests do both pass if I adjust them to check for hello and tab_completion_test respectively.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I just realized I co-authored that test. Ha.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@godlygeek
Check falsiness instead
973bef6
gaogaotiantian
gaogaotiantian reviewed May 4, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2025-05-03-21-55-33.gh-issue-133363.PTLnRP.rst Outdated
The :class:`cmd.Cmd` class has been fixed to call the ``completedefault``
method whenever the ``do_shell`` method is not defined and tab completion is
requested for a line beginning with ``!``. Previously ``completedefault``
was called only if there were no spaces between the ``!`` and the cursor

@gaogaotiantian gaogaotiantian May 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Previously completedefault was called only if there were no spaces between the ! and the cursor position where tab completion was attempted.

Is that even true? I thought the completion never works:

        elif line[0] == '!':
            if hasattr(self, 'do_shell'):
                line = 'shell ' + line[1:]
            else:
                return None, None, line

Anyway, normally in the news we only need to say what is fixed, we don't need to do a complete description of what's the previous behavior. News entry should be a concise sentence.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@godlygeek godlygeek May 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is that even true? I thought the completion never works:

Yes, because parsecmd is only called if begidx > 0, so in the !foo<Tab> case (begidx=0 endidx=3) we don't hit this buggy codepath.

Anyway, normally in the news we only need to say what is fixed, we don't need to do a complete description of what's the previous behavior. News entry should be a concise sentence.

OK, I've shortened it and just say that it's fixed to reliably call the method, since previously it was inconsistent.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Okay but it's still not good, because that's not really a name. It still won't give us the correct completion.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Let's not worry about that, we can do that in pdb I believe. I don't want to change cmd so close to beta freeze. I'll loop back to this after beta freeze.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@godlygeek godlygeek May 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Okay but it's still not good, because that's not really a name. It still won't give us the correct completion.

True, though that seems to need a bigger change if you want to fix that... Do you want to special case ! at the start of the line, and do:

            if begidx>0:
                ...
            elif line.startswith("!"):
                compfunc = self.completedefault
            else:
                compfunc = self.completenames

Even that's not enough to make PDB's completion work without the space, though, because that'll give a text="!h" and PDB will say that none of the object names it knows of start with "!". Which means that making that work requires changes in the PDB module, too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I was wrong. ! was by default a delimiter and that's why your current code passes. cmd is handling this correctly because begidx will >0 when you complete !h<tab>. pdb somehow removed ! from delimiters and I did not figure out why. However, we can deal with in pdb only, this is not a cmd issue.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
godlygeek reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
gaogaotiantian
gaogaotiantian reviewed May 4, 2025
View reviewed changes
Comment thread Lib/test/test_cmd.py Outdated
""")

# '! h' and complete 'ello' to '! hello'
input = b"! h\t\n"

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We should do add check here for !h\t\n (without space) as well. No need for another method, just another input.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@godlygeek godlygeek May 4, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

That does need to be another method, because the assertions are:

        self.assertIn(b'hello', output)
        self.assertIn(b'tab completion success', output)

and those would pass if either of the two inputs succeeded, not only if both of them did.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@godlygeek godlygeek May 4, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Or maybe we could use a subtest for this? I know how to avoid duplication with pytest, but I'm not really familiar with unittest. Does this work?

        # '! h' or '!h' and complete 'ello' to 'hello'
        for input in [b"! h\t\n", b"!h\t\n"]:
            with self.subTest(input=input):
                output = run_pty(script, input)
                self.assertIn(b'hello', output)
                self.assertIn(b'tab completion success', output)

Seems to...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gaogaotiantian gaogaotiantian May 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I mean just use the same script and do two run_pty - I don't think you even need a subtest. It's a very small case.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
godlygeek added 2 commits May 3, 2025 22:07
@godlygeek
Make news entry less descriptive
1c6248e
@godlygeek
Test with and without a space after !
c259bef
gaogaotiantian
gaogaotiantian approved these changes May 4, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels May 4, 2025
@gaogaotiantian
gaogaotiantian self-requested a review May 4, 2025 02:27
@godlygeek godlygeek mentioned this pull request May 4, 2025
completion for remote pdb is not ideal #133351
Closed
gaogaotiantian
gaogaotiantian approved these changes May 4, 2025
View reviewed changes
@gaogaotiantian
gaogaotiantian merged commit 1d9406e into python:main May 4, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label May 4, 2025
@godlygeek
godlygeek deleted the fix_cmd_bang_completion branch May 4, 2025 03:34
@bedevere-bot

bedevere-bot commented May 4, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE RHEL8 3.x (tier-2) has failed when building commit 1d9406e.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/559/builds/5937) and take a look at the build logs.
  4. Check if the failure is related to this commit (1d9406e) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/559/builds/5937

Failed tests:

  • test.test_multiprocessing_spawn.test_processes

Failed subtests:

  • test_interrupt - test.test_multiprocessing_spawn.test_processes.WithProcessesTestProcess.test_interrupt

Summary of the results of the build (if available):

==

Click to see traceback logs 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=8)
                                                  ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/multiprocessing/spawn.py", line 122, in spawn_main
    exitcode = _main(fd, parent_sentinel)
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/multiprocessing/spawn.py", line 132, in _main
    self = reduction.pickle.load(from_parent)
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/test/test_multiprocessing_spawn/test_processes.py", line 2, in <module>
    from test._test_multiprocessing import install_tests_in_module_dict
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/test/_test_multiprocessing.py", line 53, in <module>
    import multiprocessing.managers
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/multiprocessing/managers.py", line 1182, in <module>
    _BaseDictProxy = MakeProxyType('_BaseDictProxy', (
        '__contains__', '__delitem__', '__getitem__', '__ior__', '__iter__',
    ...<2 lines>...
        'keys', 'pop', 'popitem', 'setdefault', 'update', 'values'
        ))
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/multiprocessing/managers.py", line 978, in MakeProxyType
    exec('''def %s(self, /, *args, **kwds):
    ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    return self._callmethod(%r, args, kwds)''' % (meth, meth), dic)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 0, in <module>
    
    
    from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=8)
    
KeyboardInterrupt
FAIL


Traceback (most recent call last):
  File "<string>", line 1, in <module>
    from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=5, pipe_handle=9)
                                                  ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/multiprocessing/spawn.py", line 122, in spawn_main
    exitcode = _main(fd, parent_sentinel)
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/multiprocessing/spawn.py", line 132, in _main
    self = reduction.pickle.load(from_parent)
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/test/test_multiprocessing_spawn/test_processes.py", line 2, in <module>
    from test._test_multiprocessing import install_tests_in_module_dict
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/test/_test_multiprocessing.py", line 6, in <module>
    import unittest.mock
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/unittest/mock.py", line 27, in <module>
    import asyncio
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/asyncio/__init__.py", line 8, in <module>
    from .base_events import *
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/asyncio/base_events.py", line 45, in <module>
    from . import staggered
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/asyncio/staggered.py", line 10, in <module>
    from . import tasks
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/asyncio/tasks.py", line 29, in <module>
    from . import timeouts
  File "<frozen importlib._bootstrap>", line 1371, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1342, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 938, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 758, in exec_module
  File "<frozen importlib._bootstrap_external>", line 891, in get_code
  File "<frozen importlib._bootstrap_external>", line 514, in _compile_bytecode
KeyboardInterrupt
FAIL


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-ppc64le/build/Lib/test/_test_multiprocessing.py", line 578, in test_interrupt
    self.assertEqual(exitcode, 1)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
AssertionError: -2 != 1
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
diegorusso added a commit to diegorusso/cpython that referenced this pull request May 4, 2025
@diegorusso
Merge remote-tracking branch 'origin/main' into call-len
c03babc 
* origin/main: (111 commits)
  pythongh-91048: Add filename and line number to external inspection routines (pythonGH-133385)
  pythongh-131178: Add tests for `ast` command-line interface (python#133329)
  Regenerate pcbuild.sln in Visual Studio 2022 (python#133394)
  pythongh-133042: disable HACL* HMAC on Emscripten (python#133064)
  pythongh-133351: Fix remote PDB's multi-line block tab completion (python#133387)
  pythongh-109700: Improve stress tests for interpreter creation (pythonGH-109946)
  pythongh-81793: Skip tests for os.link() to symlink on Android (pythonGH-133388)
  pythongh-126835: Rename `ast_opt.c` to `ast_preprocess.c` and related stuff after moving const folding to the peephole optimizier (python#131830)
  pythongh-91048: Relax test_async_global_awaited_by to fix flakyness (python#133368)
  pythongh-132457: make staticmethod and classmethod generic (python#132460)
  pythongh-132805: annotationlib: Fix handling of non-constant values in FORWARDREF (python#132812)
  pythongh-132426: Add get_annotate_from_class_namespace replacing get_annotate_function (python#132490)
  pythongh-81793: Always call linkat() from os.link(), if available (pythonGH-132517)
  pythongh-122559: Synchronize C and Python implementation of the io module about pickling (pythonGH-122628)
  pythongh-69605: Add PyREPL import autocomplete feature to 'What's New' (python#133358)
  bpo-44172: Keep reference to original window in curses subwindow objects (pythonGH-26226)
  pythonGH-133231: Changes to executor management to support proposed `sys._jit` module (pythonGH-133287)
  pythongh-133363: Fix Cmd completion for lines beginning with `! ` (python#133364)
  pythongh-132983: Introduce `_zstd` bindings module (pythonGH-133027)
  pythonGH-91048: Add utils for printing the call stack for asyncio tasks (python#133284)
  ...
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@godlygeek @Pranjal095
pythongh-133363: Fix Cmd completion for lines beginning with ! (py…
1e2eb72 
…thon#133364)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gaogaotiantian gaogaotiantian gaogaotiantian approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@godlygeek @bedevere-bot @gaogaotiantian
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.