Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

taleinat
Copy link
Contributor

@taleinat taleinat commented May 28, 2021

(Alternative PR to GH-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:

  • '<<smart-backspace>>': ['<Key-BackSpace>']
  • '<<newline-and-indent>>': ['<Key-Return>', '<Key-KP_Enter>']
  • '<<smart-indent>>': ['<Key-Tab>']

I manually verified that having a config-keys.cfg file in .idlerc with 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.)

This removes three key bindings from the key
configuration mechanism, which are required to
always be bound to specific keys for IDLE to
behave 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>
Copy link
Member

@terryjreedy terryjreedy left a 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.

Comment on lines +302 to +303
# Bind keys to pseudoevents for non-configurable key-specific handlers.
text.event_add('<<smart-backspace>>', '<Key-BackSpace>')
Copy link
Member

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.

Comment on lines +3 to +5
``<<newline-and-indent>>`` (Return, Enter) and ``<<smart-backspace>>``
(Backspace) from the configuration mechanism, making them hard-coded
instead.
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree.

Copy link
Contributor Author

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.

Copy link
Contributor Author

@taleinat taleinat Jul 5, 2021

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.

Copy link
Member

@terryjreedy terryjreedy Jul 5, 2021

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.

@github-actions
Copy link

github-actions bot commented Jul 3, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jul 3, 2021
@ambv ambv removed the needs backport to 3.9 only security fixes label May 17, 2022
@ambv
Copy link
Contributor

ambv commented May 17, 2022

This missed the boat for inclusion in Python 3.9 which accepts security fixes only as of today.

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.11 only security fixes label May 20, 2022
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 9, 2022
@terryjreedy terryjreedy self-assigned this Sep 18, 2022
@terryjreedy terryjreedy changed the title bpo-43654: IDLE: fix config disabling tab completion gh-87820: IDLE: fix config disabling tab completion Sep 18, 2022
@terryjreedy terryjreedy removed the type-bug An unexpected behavior, bug, or error label Sep 18, 2022
@hugovk hugovk removed the needs backport to 3.10 only security fixes label Apr 7, 2023
@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes and removed needs backport to 3.11 only security fixes labels May 9, 2024
@tomasr8 tomasr8 removed the needs backport to 3.12 only security fixes label Apr 10, 2025
@python-cla-bot
Copy link

python-cla-bot bot commented Apr 18, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants