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
Modern Unix key bindings for IDLE #71360
Comments
The "IDLE Classic Unix" key set in IDLE provides Emacs-like key bindings. But modern Linux environments (KDE, Gnome, etc) use different key bindings in standard editors. Proposed patch provides new key set named "IDLE Modern Unix" that uses key bindings common in KDE (and mainly in Gnome). It is more habitual for average user. In additional the patch makes IDLE to use platform-depending key set by default. |
Seems reasonable. 3 or more years ago, there was an issue about needing to define pairs like Control-Key-c and Control-Key-C in order to have crtl + c/C-key work the same regardless of the caps lock setting. I also remember that the effect of Caps lock was system dependent. But I forget the details. We should find the issue and document the system dependent rules, perhaps in config.py, perhaps in config-keys.def. Adding a new default key set will likely not be as simple as we wish. The problem is the same one we had when adding a new default color theme last fall. If one selects the new definition, older releases will try and fail to load what is not there. Keys have a different backup logic than themes, including the 'last resort fallback' in IdleConf.GetCoreKeys. With thems the result was errors and or an exit. Have you, or can you check what happens if you select 'Modern Unix' and run an existing release. If necessary, we can use the same workaround we did with theme, which is described in the docstring for IdleConf.CurrentTheme. Part the theme patch was making sure that all of the rest of IDLE accessed the current theme through the rewritten CurrentTheme instead of through GetOption('main', 'Theme', ...). I would like to add some unittests, but a manual test protocol is needed at least to test an existing release. Tests should be run on each of the three systems, and for each backported version. When we have a tested patch ready, we could ask New to test on Mac, or do you have access to one now? |
Proposed scheme binds different actions on hotkeys with and without Shift: Ctrl-Z/Ctrl-Shift-Z, Ctrl-S/Ctrl-Shift-S. If don't touch CapsLock this is not an issue. Yes, there is a problem with new buildin keys set in old IDLE versions. A lot of warnings is emitted on stderr. The solution for the "IDLE Dark" theme is not complete, since after adding next new theme we will encounter the same problem and should add options "name3", "name4"... Updated patch uses the same solution for new keys set. But it also makes warnings be less annoying. Every warning is emitted just once, and only one warning is emitted for unknown keys set. |
Original IDLE Dark patch: bpo-24820. Follow-up for cross-version issues (initially for themes, but with a eye to keys, etc): bpo-25313. I intended that 'name2' be future proof (msg252372). I believe my idea was that if it were to point to a future 4th default theme that did not exist, there would be a graceful fallback to 'name', which defaults to IDLE Classic. But I am not sure if I tested this, so I should recheck. If it does not work, a fix should go in all current versions. I believe the patch did achieve no warnings for older versions, as least on my machine. I would have the same goal for keys: no warning, future proof. A test of the latter before committing requires two independent repositories, both with the patch, one with the extra definitions. I currently have two 3.6 repositories and will do such a test once we have code that we think should work. When one selects IDLE Dark as Default Theme, active or not, a subdued red message is displayed: "New theme, see Help". (The latter part should change to a more explicit "click 'Help' below" so there is no possible confusion with the main menu Help. The Highlighting specific part of the popup suggests saving IDLE Dark as a custom theme, with a new name, This makes the theme available to older versions and solves compatibility problems. The same would apply to a new key set. In bpo-24781, Mark Roseman proposed revamping the preferences dialog, both externally and internally. The main idea for the Theme tab is to present users with a single list of available themes and (mostly) hide the builtin versus custom distinction. (It cannot be ignored completely as builtins cannot be deleted and editing requires saving as custom.) https://bugs.python.org/file40111/highlight3.png has an image. We might consider the same idea for key sets. |
New patch makes IDLE silently fall back to default key set and adds a note about new key set on Keys configuration tab. |
Ping. |
I looked at _set3 and it looks plausible and possibly complete other than testing. I added something about testing customizations to bpo-27099. This applied cleanly to 3.6. 3.6 started, dialog opened, new key set selected, dialog closed, config-main.cfg looks good. Open dialog, same as custom, close dialog, config-keys and config main changed as expected. Open 3.5, open dialog, everything looks fine. Now the problems.
In any case, the solution for both problems is to either allow deletion of non-selected sets, and only such, or to switch to *something* before re-reading. I open a new issue, bpo-27245, for this. 2. More serious, the warnigs are followed by
Exception in Tkinter callback
Traceback (most recent call last):
File "F:\Python\dev\35\lib\tkinter\__init__.py", line 1550, in __call__
return self.func(*args)
File "F:\Python\dev\35\lib\idlelib\configDialog.py", line 1182, in Ok
self.Apply()
File "F:\Python\dev\35\lib\idlelib\configDialog.py", line 1186, in Apply
self.DeactivateCurrentConfig()
File "F:\Python\dev\35\lib\idlelib\configDialog.py", line 1166, in DeactivateCurrentConfig
instance.RemoveKeybindings()
File "F:\Python\dev\35\lib\idlelib\EditorWindow.py", line 771, in RemoveKeybindings
self.text.event_delete(event, *keylist)
File "F:\Python\dev\35\lib\idlelib\MultiCall.py", line 391, in event_delete
self.__binders[triplet[1]].unbind(triplet, func)
File "F:\Python\dev\35\lib\idlelib\MultiCall.py", line 234, in unbind
doit()
File "F:\Python\dev\35\lib\idlelib\MultiCall.py", line 232, in <lambda>
doit = lambda: self.bindedfuncs[triplet[2]][triplet[0]].remove(func)
ValueError: list.remove(x): x not in list Hitting OK repeats warnings and traceback (the last line might have been a bit different the first time). Only Cancel would close. |
I'm aware of the bug with deleting current key set. idle_modern_unix_key_set3.patch effectively fixes it. |
After applying this patch I'm going to redesign Highlighting and Keys tabs in the Settings dialog. |
I don't understand
I look forward to contributions to improving the dialog. Redoing the font tab is bpo-24776. The highlight tab is bpo-24781. The latest mockups look like improvements to me, but we can discuss further. (bpo-24781 also has Mark's proposed refactoring of the dialog code, but I have not reviewed in detail.) There are numerous issues connected with key definitions. |
Interesting, I was not able to reproduce a bug if just save as custom theme and apply, but after creating and deleting the second custom theme I got a mystical behavior of IDLE -- radiobuttons are not consistent with comboboxes, OK can't be pressed, etc. |
There was a bug, saving custom key set didn't have any effect if previous key set was IDLE Modern Unix. |
Ned, Serhiy's patch 4 solves one of your concerns in bpo-20580 with + @staticmethod Please confirm that Classic OSX is the right choice over Classic Mac. |
"Please confirm that Classic OSX is the right choice over Classic Mac" It is. At least, when we install IDLE on OS X, Clsssic OS X is the default set. I don't know why Classic Mac exists; I'm guessing it is left over from Mac OS 9 days (pre OS X). |
Ping. |
The change in v4 is the addition of |
I can't reproduce your issue (or understand it correctly). Could you please show the [Keys] section in your config-main.cfg? |
I rebooted and reran build in case this was another funny heisenbug. It isn't. Before patch, start 3.6 repository IDLE. Click Options, Configure IDLE, and Keys tab. Look at 'Use a Custom Key Set'. It says 'Terry', which is correct. After patch: repeat. It says 'IDLE Modern Unix'. Custom Key Binding box reflects change. Repeat with 3.5 repository IDLE: it still says Terry. Ditto for installed 3.6.0a2. My custom [Keys] section: [Keys] The problem is that name2 should be treated only as default, never as custom, and should be used only if default=True. This is true in CurrentTheme. However, your revised CurrentKeys uses it first and unconditionally. Hence the observed behavior. CurrentKeys should use the same logic as CurrentTheme. In fact, the common logic should factored out into a separate function with two or three parameters and used by both. If, as you claim, CurrentTheme is buggy, it should be fixed before using it as the template for both. To test and claim that it was future proof, I did something like the following last fall. First set config-main.cfg [Keys] name2 to something non-existent, that might be added in the future. [Theme] Then start, for instance, 3.5.2. The non-existent IDLE Future is ignored and replaced with the default default IDLE Classic, as intended in the code. No warnings. What exactly did you do to claim otherwise? --- [Keys] This, of course, is also wrong on non-Windows, which is why you changed it. But it is also being ignored when custom is selected. The problem must be in how configdialog gets the default key set when the current keyset is custom. The widget is a DynOptionMenu, created in CreatePageKeys, line 335, with no content. The options are set in LoadKeyCfg, lines 1055 and 1069. The latter is used when the current key is custom. It sets the grayed out default option to the alphabetically first among default key sets. itemList = idleConf.GetSectionList('default', 'keys')
itemList.sort()
self.optMenuKeysBuiltin.SetMenu(itemList, itemList[0]) itemList[0] should be replaced by your new system-sensitive idleConf.DefaultKeys(). |
Updated patch addresses your comments Terry. Since CurrentTheme checks that the corresponding section exists, I no longer think it is buggy. |
Seems to be working correctly now, so I will review the code in detail. I will look into extracting a common get_theme_keys function and editing the long docstring currently attached to CurrentTheme. I want to look at paired functions in configdialog, such as def VarChanged_builtinTheme/Keys to see if there are any unnecessary differences. The hardest thing is adding tests. Correct answers depend on the configuration vales, both default and user overrides. See bpo-24737 for the latter. |
After committing the patch for this issue I'm going to refactor the code related to getting/setting theme/keys settings. For now it is too complicated, the logic is scattered between two files, some code is duplicated. |
The attached patch has a new function current_colors_and_keys that combines ideas and code from both CurrentTheme and CurrentKeys. The latter are now trivial wrappers. It has a new test_config that tests both this function and _warn. The changes to configdialog were tested 'by hand' by loading config-main.cfg into an editor, applying every theme and keyset combination I could think of, and each time re-loading the user file to see what was written. The patterns produces were used in testing current_colors_and_keys. These tests and similar configdialog tests could be automated once I revise the user confighandler so it can write to a StringIO. Barring a reason not to, I plan to apply tomorrow to 3.6 so this gets into alpha 3. Backports can be done later, maybe after the configdialog tests can be automated. |
New changeset e6e6c71776b0 by Terry Jan Reedy in branch 'default': |
New changeset 792e3294b59e by Terry Jan Reedy in branch 'default': |
Can this issue be closed? |
Yes. If I decide to do a backport to either 3.5 or 2.7, I can reopen or just commit. |
Any backport must include the bugfix in bpo-27821. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: