cpython icon indicating copy to clipboard operation
cpython copied to clipboard

gh-89520: IDLE - Make extentions use user's keys, not all defaults

Open CoolCat467 opened this issue 3 years ago • 4 comments

I was trying to write an extension for Idle, and was noticing extension keys defined in ~/.idlerc weren't being used, so I looked into it and found that in idlelib.config.idleConf, GetExtensionKeys, __GetRawExtensionKeys, and GetKeyBinding would never check for user keys, always assuming default keys have everything. This is not always true. In a system with multiple users, you would not want to modify the default config file for a custom extension, as that is used for everyone. This pull request is changing aforementioned functions to also check user config files, and overwrite values acquired from the default config file if the user config redefined one.

Edit: The especially interesting part is that extensions are loaded anyways if they have an entry in the user's IDLE configuration file, but their keybinds are only loaded properly if they are defined in the root configuration file at the moment. This pull request is fixing this difference and allows extension keybinds to be loaded from the user's IDLE configuration file, making user extensions work just as well as root extensions.

  • Issue: gh-89520

CoolCat467 avatar Oct 03 '21 21:10 CoolCat467

Some tests are failing; please fix those.

Fixed failing tests as of 016ce89

CoolCat467 avatar Nov 17 '23 22:11 CoolCat467

As mentioned above, I'm not able to provide a full review on this PR.

JelleZijlstra avatar Mar 11 '24 03:03 JelleZijlstra

@terryjreedy Ping, it's been 3 years and I think my newest approach will work well. I've been testing a version of what this PR does as it's own module that has to do some nasty monkeypatching stuff, but it allows IDLE to successfully load extensions from user configuration files, and I would like to see this become something standard.

CoolCat467 avatar Mar 11 '24 03:03 CoolCat467

These comments are very preliminary. And when I wrote the first 2, I had forgotten that this PR intentionally changes a config rule. But its late, so I am submitting them as they are to give you some feedback.

If I were to approve, new tests would be needed, modeled on existing tests that avoid reading/writing the user files on disk by loading the in-memory config dicts. zzdummy.py, or a copy in idle_test, might be usable.

@terryjreedy Ping, I have reviewed your feedback and have made noted changes, and I've added a few tests that try to capture the fact that and extension that is not enabled will not have it's keybinds bound but that one that is enabled will, as well as making sure extensions that are enabled entirely from user configuration files will have their keybinds found successfully. To do this I made a copy of test_zzdummy (test_zzdummy_user) and changed both of them to test aforementioned behavior.

CoolCat467 avatar Mar 12 '24 04:03 CoolCat467