homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: IDLE : Bug in keybinding validity check
Type: behavior Stage: resolved
Components: IDLE Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: Saimadhav.Heblikar, terry.reedy
Priority: normal Keywords: patch

Created on 2014年05月18日 05:54 by Saimadhav.Heblikar, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
keybinding.diff Saimadhav.Heblikar, 2014年05月18日 05:54 review
Pull Requests
URL Status Linked Edit
PR 2428 merged terry.reedy, 2017年06月27日 04:08
PR 2433 merged terry.reedy, 2017年06月27日 05:28
Messages (11)
msg218734 - (view) Author: Saimadhav Heblikar (Saimadhav.Heblikar) * Date: 2014年05月18日 05:54
Steps to reproduce the bug:
1. IDLE > options > configure idle > keys
2. Try to replace a keybinding for an action with that of another action which has more than one keybinding.
For eg : Default binding of copy=<Control-Key-c> <Control-Key-C>.
So, try to replace any other keybinding with <Control-Key-c>
3. This change is accepted
Now the <Control-Key-C> binding will be assigned to two actions.
This bug only applies if the other action has more than one binding.
In case the other action has only one binding, an error is raised(as expected).
Attaching a patch to fix this issue.
msg218777 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014年05月19日 01:57
> Now the <Control-Key-C> binding will be assigned to two actions.
I do not see this. I only see <Control-Key-c> duplicated, which is bad enough to be patched. What did you leave out ;-).
Also see #12387, which we need to finish, and Ned's link to http://wiki.tcl.tk/28331 
A patch for KeysOK should include test_bindings.py with class KeysOK_Test. As with the test of name_ok in test_configname.py (sp?), the method should be embedded in a dummy class and use a dummy message box.
The problem with the method goes deeper. It is not called if the 'advanced' (hand entry, therefore primitive) pane is used. As near as I can tell (and if I am wrong, please tell me), multiple bindings, which are required to ignore lower-upper case issues, can only be entered with this 'advanced' method. Hence, most new entries are not checked at all. The basic pane, which is advanced in that one can only enter possibly valid combination, should be modified to allow multiple entries. (The two panes should be called 'Easy' and 'Error-prone ;-)
As it is, 'keys' is a misnomer[note], "keys.strip()" is a no-op in that 'keys' is auto-generated, and "keySequence = keys.split()" is the same as "keySequence = [keys]"[note]. Many of the tests only test mistakes that can happen with hand-entry, which is not tested. I believe that the duplicate check is the only one needed (except possible for the modifier check, which I have not looked at enough to tell).
[note] If the easy pane were modified to define multiple keys, and all were passed at once, then the two noted lines would become valid, but then each key, would have to be tested individually against the flat list.
This suggests to me that the flattening should be done just once and that KeysOK should become KeyOK, or rather key_ok. It might even be replaced by "newkey in flat_list" or something nearly that simple.
msg220625 - (view) Author: Saimadhav Heblikar (Saimadhav.Heblikar) * Date: 2014年06月15日 09:54
A small bug in line 185 in keybindingDialog.py. 'F2' appears twice and 'F3' is missing. Since this is a typo, I did not create a new issue.
msg228300 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014年10月03日 03:08
#6739 may also be relevant
msg296481 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017年06月20日 19:35
#6739 also has a patch to refuse invalid key bindings.
msg296562 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017年06月21日 15:49
I believe patch affects same area of file as patch for #6739.
msg296650 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017年06月22日 19:16
#6739 is about rejecting *invalid* sequences, this is about rejecting a *duplicate* valid sequence. Both fixes are needed.
msg296796 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017年06月24日 23:45
configdialog.ConfigDialog.getNewKeys() calls config_key.GetKeysDialog with a list of lists of one or more sequences (currentKeySequences). GetKeysDialog.KeysOK looks for keys.split() in currentKeySequences. Since KeysOK on only called for the no-space 'keys' produced by the basic dialog, keys.split is always [keys]. This can only match one of the length-1 lists in currentKeySequences.
The patch semi-flattens the latter to a list of lists of 1 sequence each
. But instead of looking for [keys] in the result, better and faster to omit the list wrappers and look for keys in a list or set of sequences.
A separate improvement would be to have a reverse map of sequences to pseudoevents, so the error message can specify the exact conflict instead of just saying 'some conflict'. I have separately thought that the [keys] tab of ConfigDialog should be able to display such a mapping to help one plan a new key mapping.
Off topic for this issue: the keys tab could have a 'load key-def file' function that would check a definition and if ok, load it.
msg297005 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017年06月27日 05:23
New changeset 44913e584bcf4e2a0e1a6372c304c2d5ea521fc6 by terryjreedy in branch 'master':
bpo-21519: IDLE basic custom key entry better detects duplicates. (#2428)
https://github.com/python/cpython/commit/44913e584bcf4e2a0e1a6372c304c2d5ea521fc6
msg297011 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017年06月27日 05:58
New changeset 93b88e995373c48713c9f7d4b32fe1d0166709e5 by terryjreedy in branch '3.6':
[3.6] bpo-21519: IDLE basic custom key entry better detects duplicates. (GH-2428) (#2433)
https://github.com/python/cpython/commit/93b88e995373c48713c9f7d4b32fe1d0166709e5
msg297012 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017年06月27日 06:33
Thanks for the patch. The F3-missing bug has been fixed.
History
Date User Action Args
2022年04月11日 14:58:03adminsetgithub: 65718
2017年06月27日 06:33:21terry.reedysetstatus: open -> closed
resolution: fixed
messages: + msg297012

stage: test needed -> resolved
2017年06月27日 05:58:20terry.reedysetmessages: + msg297011
2017年06月27日 05:28:00terry.reedysetpull_requests: + pull_request2481
2017年06月27日 05:23:57terry.reedysetmessages: + msg297005
2017年06月27日 04:08:36terry.reedysetpull_requests: + pull_request2479
2017年06月24日 23:45:54terry.reedysetmessages: + msg296796
2017年06月22日 19:16:19terry.reedysetmessages: + msg296650
2017年06月21日 15:49:14terry.reedysetmessages: + msg296562
2017年06月20日 19:35:03terry.reedysetassignee: terry.reedy
messages: + msg296481
versions: + Python 3.6, Python 3.7, - Python 2.7, Python 3.4
2014年10月03日 03:08:41terry.reedysetmessages: + msg228300
2014年06月15日 09:54:51Saimadhav.Heblikarsetmessages: + msg220625
2014年05月19日 01:57:13terry.reedysetmessages: + msg218777
stage: test needed
2014年05月18日 05:54:15Saimadhav.Heblikarcreate

AltStyle によって変換されたページ (->オリジナル) /