Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-87820: IDLE: fix config disabling tab completion#26421
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
gh-87820: IDLE: fix config disabling tab completion#26421
Conversation
This removes three key bindings from the keyconfiguration mechanism, which are required toalways be bound to specific keys for IDLE tobehave properly:* '<<smart-backspace>>': ['<Key-BackSpace>'],* '<<newline-and-indent>>': ['<Key-Return>', '<Key-KP_Enter>'],* '<<smart-indent>>': ['<Key-Tab>'],Signed-off-by: Tal Einat <532281+taleinat@users.noreply.github.com>
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 would like to merge the code and .def changes as are but am not quite sure that we should. See comment on the issue.
| # Bind keys to pseudoevents for non-configurable key-specific handlers. | ||
| text.event_add('<<smart-backspace>>', '<Key-BackSpace>') |
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.
The only function reason to keep these fixed keys separate from those that follow is that the pseudoevent -- handler bindings are done above for these and below for the existing fixed key keys. And this is related to where the handler are defined. So keep the separation for now.
| ``<<newline-and-indent>>`` (Return, Enter) and ``<<smart-backspace>>`` | ||
| (Backspace) from the configuration mechanism, making them hard-coded | ||
| instead. |
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.
These two are not relevant to tab completion. So it we do these together, I think we need to change the PR title (see my issue title revision) and reword this a bit.
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 agree.
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'm not sure the PR/commit title must describe all of the technical changes made. The additional change to the<<newline-and-indent>> and<<smart-backspace>> bindings has negligible significance to users, since even before this change, changing these bindings breaks IDLE rather badly.
Having this detail in the NEWS entry, and possibly in the body of the commit message, seems right to me.
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.
To be clear, I now think that the existing PR title is good, since it describes the only significant bug being fixed.
terryjreedyJul 5, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Today I think the title is fine as is. The message that follows can say more.
This PR is stale because it has been open for 30 days with no activity. |
This missed the boat for inclusion in Python 3.9 which accepts security fixes only as of today. |
python-cla-botbot commentedApr 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Uh oh!
There was an error while loading.Please reload this page.
(Alternative PR toGH-26403.)
The underlying issue causing tab-completion to be broken after using the config dialog: Two events are bound to
<Key-Tab>:<<smart-indent>>and<<autocomplete>>. The order these are bound is important: The binding to<<autocomplete>>must be done last so that it will be invoked first. The current bug is that<<smart-indent>>is unbound and then bound again by the config dialog, which changes the order and breaks tab-completion.This PR removes three key bindings from the key configuration mechanism, which are required to always be bound to specific keys for IDLE to behave properly:
I manually verified that having a
config-keys.cfgfile in.idlercwith those keys defined in it doesn't cause any issues after this change is made. (IdleConf.GetCoreKeys()is written in a way that ensures this.)