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-88564: IDLE - fix 2 Edit menu hotkey displays #99743

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

Conversation

terryjreedy
Copy link
Member

@terryjreedy terryjreedy commented Nov 24, 2022

On Mac Cocoa, ^+backspace and ^+space were mis-displayed as ^B and ^S. Replace 'backslash' with '' everywhere, just as 'slash', for example, is replaced with '/'. On Mac Cocoa, change 'space' to 'Space', which then displays as 'Space' instead of 'S'. ('Space' is also used for the Mac-specific Emoji & Symbols entry.)

Co-authored by ronaldoussoren

On Mac Cocoa, ^+backspace and ^+space were misdisplayed
as ^B and ^S. Replace 'backslash' with '\' everywhere,
just as 'slash', for example, is replaced with '/'.
On Mac Cocoa, change'space' to 'Space', which then displays
as 'Space' instead of 'S'.  ('Space' is also used for the
Mac-specific Emoji & Symbols entry.)
@terryjreedy
Copy link
Member Author

@ronaldoussoren Besides Windows, I tested this on Catalina by editing installed editor.py. Is the space-> Space fix only needed for Cocoa?

@terryjreedy
Copy link
Member Author

The test failure is hanging for 20 minutes in
test_case_soft_keyword (idlelib.idle_test.test_colorizer.ColorDelegatorTest.test_case_soft_keyword) .
I tested python3.x -m test.test_idle -v on my MacBook Air and got the same failure even with the 2 new lines commented out and with 3.11 and 3.10. I am baffled.
@ned-deily You have any ideas either?

@ronaldoussoren
Copy link
Contributor

This PR does not work for me. If I manually apply the patch to an installed copy of Python 3.12 IDLE crashes when I try to open the Edit menu ("segmentation fault"). Reverting the patch fixes the crash.

The crash report has a stack trace somewhere deep inside system frameworks with Tk on the call stack.

The fix is likely only needed for Cocoa, otherwise we'd have reports from other platforms as well. But that's not something I can test, I can only test using the copy of Tk shipped with out installers.

@hugovk hugovk removed the needs backport to 3.10 only security fixes label Apr 7, 2023
@terryjreedy
Copy link
Member Author

PR not working right yet.

Copy link
Contributor

@ronaldoussoren ronaldoussoren left a comment

Choose a reason for hiding this comment

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

I've tested this PR with a slight modification to make it work on top of #113513:

diff --git a/Lib/idlelib/editor.py b/Lib/idlelib/editor.py
index 8bf517d41f..36fde4af73 100644
--- a/Lib/idlelib/editor.py
+++ b/Lib/idlelib/editor.py
@@ -1678,6 +1678,7 @@ def prepstr(s):
  'bracketleft': '[',
  'bracketright': ']',
  'slash': '/',
+ 'backslash': '\\',
 }
 
 def get_accelerator(keydefs, eventname):
diff --git a/Lib/idlelib/macosx.py b/Lib/idlelib/macosx.py
index 8637e8ebf2..60c58d59a3 100644
--- a/Lib/idlelib/macosx.py
+++ b/Lib/idlelib/macosx.py
@@ -263,6 +263,8 @@ def setupApp(root, flist):
     global _idle_root
     _idle_root = root
     if isAquaTk():
+        from idlelib import editor
+        editor.keynames["space"] = "Space"
         hideTkConsole(root)
         overrideRootMenu(root, flist)
         addOpenEventSupport(root, flist)

That PR makes it an error to call isAquaTk before macosx.setupApp is called to avoid problems with Tk Aqua (see that PR for background).

The menu now looks correct, and I've tested that the shortcuts work as well:

image

My change to the diff should work both with and without #113513. Please move setting idlelib.editor.keynames['space'] to macosx.setupApp as in the diff above to avoid creating merge conflict when that PR is merged.

@bedevere-app
Copy link

bedevere-app bot commented Dec 29, 2023

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@ronaldoussoren ronaldoussoren added the needs backport to 3.12 bug and security fixes label Dec 31, 2023
@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes and removed needs backport to 3.11 only security fixes labels May 9, 2024
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