Skip to content

Conversation

@ringof
Copy link
Contributor

@ringof ringof commented Apr 2, 2022

Co-authored-by: Saimadhav Heblikar (Saimadhav.Heblikar)
Source for this PR is derived from a patch submitted: https://bugs.python.org/file35592/keybinding-issue12387-v1.diff
for which these changes were not followed up with a review or inclusion as of 2014.

The issue reported remains so - this is easily proven on Linux systems by running IDLE and trying to save to an existing file with Control-s and caps lock enabled. The fix here resolves the issue by the means suggested in the bpo issue discussion.

Consider this a draft PR; the test removed raises the question on how best to implement a new test - input from reviews is appreciated.

@terryjreedy
Copy link
Member

I don't remember ever reviewing Heblikar's patch, certainly not in any detail. You revised it to not do 'v.extend(v2)' on Macs. Why? Are you being conservative? or do you know it to be wrong on Macs. I still have the problem of not being able to test on Linux, and mostly not on macOS. I think that this is mostly not needed on Windows, but I do not like the duplication in the Windows part of the definition file.

@ringof
Copy link
Contributor Author

ringof commented Apr 2, 2022

You revised it to not do 'v.extend(v2)' on Macs. Why? Are you being conservative? or do you know it to be wrong on Macs.

Thank you for the rapid review. Yes to the first - 'if it ain't broke, don't fix it' and I don't have a Mac to test it on right now. I'll see if I can scrounge one up over the next week; I'd like consistency as well.

@terryjreedy
Copy link
Member

The mac keyset is broken in other ways than this, so I might prefer leaving it alone until there is a real fix. I have a Macbook, but getting a development system set up on a Mac is much harder than it apparently is on Linux.

@ringof
Copy link
Contributor Author

ringof commented Apr 2, 2022

Understood. It seems to me there are two routes to handle this. One is a maximal approach - developing a fix for all the supported OSes, which then resolves the bug. The other is to accept this PR (essentially the old patch) as it fixes IDLE for Unix/Linux users today, keep the bug open, and then come around to fixing it properly for all supported OSes. While I believe in not letting the perfect be the enemy of the good, apparently nobody's been hurting too bad from this over the last 8 years, and what I'm after is to help close the oldest and easy bugs. Thoughts?

@terryjreedy terryjreedy changed the title bpo-12387: IDLE keyboard shortcuts function irespective of caps-lock state gh-56596: IDLE keyboard shortcuts function irespective of caps-lock state Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants