gh-89520: Load extension settings and keybindings from user config (GH-28713)

Extension keybindings defined in ~/.idlerc/config-extensions.cfg were silently ignored because GetExtensionKeys, __GetRawExtensionKeys, and GetExtensionBindings only checked default config. Fix these to check user config as well, and update the extensions config dialog to handle user-only extensions correctly.

---------

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>

Author: 4 people

Lib/idlelib/config.py

@@ -476,34 +476,58 @@ def GetExtensionKeys(self, extensionName):
         Keybindings come from GetCurrentKeySet() active key dict,
         where previously used bindings are disabled.
         """
-        keysName = extensionName + '_cfgBindings'
-        activeKeys = self.GetCurrentKeySet()
-        extKeys = {}
-        if self.defaultCfg['extensions'].has_section(keysName):
-            eventNames = self.defaultCfg['extensions'].GetOptionList(keysName)
-            for eventName in eventNames:
-                event = '<<' + eventName + '>>'
-                binding = activeKeys[event]
-                extKeys[event] = binding
-        return extKeys
-
-    def __GetRawExtensionKeys(self,extensionName):
+        bindings_section = f'{extensionName}_cfgBindings'
+        current_keyset = self.GetCurrentKeySet()
+        extension_keys = {}
+
+        event_names = set()
+        if self.userCfg['extensions'].has_section(bindings_section):
+            event_names |= set(
+                self.userCfg['extensions'].GetOptionList(bindings_section)
+            )
+        if self.defaultCfg['extensions'].has_section(bindings_section):
+            event_names |= set(
+                self.defaultCfg['extensions'].GetOptionList(bindings_section)
+            )
+
+        for event_name in event_names:
+            event = f'<<{event_name}>>'
+            binding = current_keyset.get(event, None)
+            if binding is None:
+                continue
+            extension_keys[event] = binding
+        return extension_keys
+
+    def __GetRawExtensionKeys(self, extension_name):
         """Return dict {configurable extensionName event : keybinding list}.
 
         Events come from default config extension_cfgBindings section.
         Keybindings list come from the splitting of GetOption, which
         tries user config before default config.
         """
-        keysName = extensionName+'_cfgBindings'
-        extKeys = {}
-        if self.defaultCfg['extensions'].has_section(keysName):
-            eventNames = self.defaultCfg['extensions'].GetOptionList(keysName)
-            for eventName in eventNames:
-                binding = self.GetOption(
-                        'extensions', keysName, eventName, default='').split()
-                event = '<<' + eventName + '>>'
-                extKeys[event] = binding
-        return extKeys
+        bindings_section = f'{extension_name}_cfgBindings'
+        extension_keys = {}
+
+        event_names = set()
+        if self.userCfg['extensions'].has_section(bindings_section):
+            event_names |= set(
+                self.userCfg['extensions'].GetOptionList(bindings_section)
+            )
+        if self.defaultCfg['extensions'].has_section(bindings_section):
+            event_names |= set(
+                self.defaultCfg['extensions'].GetOptionList(bindings_section)
+            )
+
+        for event_name in event_names:
+            binding = self.GetOption(
+                'extensions',
+                bindings_section,
+                event_name,
+                default='',
+            ).split()
+            event = f'<<{event_name}>>'
+            extension_keys[event] = binding
+        return extension_keys
 
     def GetExtensionBindings(self, extensionName):
         """Return dict {extensionName event : active or defined keybinding}.
@@ -512,18 +536,30 @@ def GetExtensionBindings(self, extensionName):
         configurable events (from default config) to GetOption splits,
         as in self.__GetRawExtensionKeys.
         """
-        bindsName = extensionName + '_bindings'
-        extBinds = self.GetExtensionKeys(extensionName)
-        #add the non-configurable bindings
-        if self.defaultCfg['extensions'].has_section(bindsName):
-            eventNames = self.defaultCfg['extensions'].GetOptionList(bindsName)
-            for eventName in eventNames:
-                binding = self.GetOption(
-                        'extensions', bindsName, eventName, default='').split()
-                event = '<<' + eventName + '>>'
-                extBinds[event] = binding
-
-        return extBinds
+        bindings_section = f'{extensionName}_bindings'
+        extension_keys = self.GetExtensionKeys(extensionName)
+
+        # add the non-configurable bindings
+        event_names = set()
+        if self.userCfg['extensions'].has_section(bindings_section):
+            event_names |= set(
+                self.userCfg['extensions'].GetOptionList(bindings_section)
+            )
+        if self.defaultCfg['extensions'].has_section(bindings_section):
+            event_names |= set(
+                self.defaultCfg['extensions'].GetOptionList(bindings_section)
+            )
+
+        for event_name in event_names:
+            binding = self.GetOption(
+                'extensions',
+                bindings_section,
+                event_name,
+                default=''
+            ).split()
+            event = f'<<{event_name}>>'
+            extension_keys[event] = binding
+        return extension_keys
 
     def GetKeyBinding(self, keySetName, eventStr):
         """Return the keybinding list for keySetName eventStr.

Lib/idlelib/configdialog.py

@@ -1960,12 +1960,15 @@ def create_page_extensions(self):
     def load_extensions(self):
         "Fill self.extensions with data from the default and user configs."
         self.extensions = {}
+
         for ext_name in idleConf.GetExtensions(active_only=False):
             # Former built-in extensions are already filtered out.
             self.extensions[ext_name] = []
 
         for ext_name in self.extensions:
-            opt_list = sorted(self.ext_defaultCfg.GetOptionList(ext_name))
+            default = set(self.ext_defaultCfg.GetOptionList(ext_name))
+            user = set(self.ext_userCfg.GetOptionList(ext_name))
+            opt_list = sorted(default | user)
 
             # Bring 'enable' options to the beginning of the list.
             enables = [opt_name for opt_name in opt_list
@@ -1975,8 +1978,12 @@ def load_extensions(self):
             opt_list = enables + opt_list
 
             for opt_name in opt_list:
-                def_str = self.ext_defaultCfg.Get(
-                        ext_name, opt_name, raw=True)
+                if opt_name in default:
+                    def_str = self.ext_defaultCfg.Get(
+                            ext_name, opt_name, raw=True)
+                else:
+                    def_str = self.ext_userCfg.Get(
+                            ext_name, opt_name, raw=True)
                 try:
                     def_obj = {'True':True, 'False':False}[def_str]
                     opt_type = 'bool'
@@ -2054,10 +2061,11 @@ def set_extension_value(self, section, opt):
         default = opt['default']
         value = opt['var'].get().strip() or default
         opt['var'].set(value)
-        # if self.defaultCfg.has_section(section):
-        # Currently, always true; if not, indent to return.
-        if (value == default):
+
+        # Only save option in user config if it differs from the default
+        if self.ext_defaultCfg.has_section(section) and value == default:
             return self.ext_userCfg.RemoveOption(section, name)
+
         # Set the option.
         return self.ext_userCfg.SetOption(section, name, value)
 

Lib/idlelib/editor.py

@@ -860,9 +860,8 @@ def RemoveKeybindings(self):
             self.text.event_delete(event, *keylist)
         for extensionName in self.get_standard_extension_names():
             xkeydefs = idleConf.GetExtensionBindings(extensionName)
-            if xkeydefs:
-                for event, keylist in xkeydefs.items():
-                    self.text.event_delete(event, *keylist)
+            for event, keylist in xkeydefs.items():
+                self.text.event_delete(event, *keylist)
 
     def ApplyKeybindings(self):
         """Apply the virtual, configurable keybindings.

Lib/idlelib/idle_test/test_zzdummy.py

@@ -38,38 +38,8 @@ def __init__(self, root, text):
         self.text.undo_block_stop = mock.Mock()
 
 
-class ZZDummyTest(unittest.TestCase):
-
-    @classmethod
-    def setUpClass(cls):
-        requires('gui')
-        root = cls.root = Tk()
-        root.withdraw()
-        text = cls.text = Text(cls.root)
-        cls.editor = DummyEditwin(root, text)
-        zzdummy.idleConf.userCfg = testcfg
-
-    @classmethod
-    def tearDownClass(cls):
-        zzdummy.idleConf.userCfg = usercfg
-        del cls.editor, cls.text
-        cls.root.update_idletasks()
-        for id in cls.root.tk.call('after', 'info'):
-            cls.root.after_cancel(id)  # Need for EditorWindow.
-        cls.root.destroy()
-        del cls.root
-
-    def setUp(self):
-        text = self.text
-        text.insert('1.0', code_sample)
-        text.undo_block_start.reset_mock()
-        text.undo_block_stop.reset_mock()
-        zz = self.zz = zzdummy.ZzDummy(self.editor)
-        zzdummy.ZzDummy.ztext = '# ignore #'
-
-    def tearDown(self):
-        self.text.delete('1.0', 'end')
-        del self.zz
+class ZZDummyMixin:
+    """Shared tests for ZzDummy with default and user configs."""
 
     def checklines(self, text, value):
         # Verify that there are lines being checked.
@@ -89,7 +59,8 @@ def test_init(self):
 
     def test_reload(self):
         self.assertEqual(self.zz.ztext, '# ignore #')
-        testcfg['extensions'].SetOption('ZzDummy', 'z-text', 'spam')
+        zzdummy.idleConf.userCfg['extensions'].SetOption(
+            'ZzDummy', 'z-text', 'spam')
         zzdummy.ZzDummy.reload()
         self.assertEqual(self.zz.ztext, 'spam')
 
@@ -148,5 +119,75 @@ def test_roundtrip(self):
         self.assertEqual(text.get('1.0', 'end-1c'), code_sample)
 
 
+class ZZDummyTest(ZZDummyMixin, unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        requires('gui')
+        root = cls.root = Tk()
+        root.withdraw()
+        text = cls.text = Text(cls.root)
+        cls.editor = DummyEditwin(root, text)
+        zzdummy.idleConf.userCfg = testcfg
+
+    @classmethod
+    def tearDownClass(cls):
+        zzdummy.idleConf.userCfg = usercfg
+        del cls.editor, cls.text
+        cls.root.update_idletasks()
+        for id in cls.root.tk.call('after', 'info'):
+            cls.root.after_cancel(id)  # Need for EditorWindow.
+        cls.root.destroy()
+        del cls.root
+
+    def setUp(self):
+        text = self.text
+        text.insert('1.0', code_sample)
+        text.undo_block_start.reset_mock()
+        text.undo_block_stop.reset_mock()
+        zz = self.zz = zzdummy.ZzDummy(self.editor)
+        zzdummy.ZzDummy.ztext = '# ignore #'
+
+    def tearDown(self):
+        self.text.delete('1.0', 'end')
+        del self.zz
+
+    def test_exists(self):
+        conf = zzdummy.idleConf
+        self.assertEqual(
+            conf.GetSectionList('user', 'extensions'), [])
+        self.assertEqual(
+            conf.GetSectionList('default', 'extensions'),
+            ['AutoComplete', 'CodeContext', 'FormatParagraph',
+             'ParenMatch', 'ZzDummy', 'ZzDummy_cfgBindings',
+             'ZzDummy_bindings'])
+        self.assertIn("ZzDummy", conf.GetExtensions(False))
+        self.assertNotIn("ZzDummy", conf.GetExtensions())
+        self.assertEqual(
+            conf.GetExtensionKeys("ZzDummy"), {})
+        self.assertEqual(
+            conf.GetExtensionBindings("ZzDummy"),
+            {'<<z-out>>': ['<Control-Shift-KeyRelease-Delete>']})
+
+    def test_exists_user(self):
+        conf = zzdummy.idleConf
+        conf.userCfg["extensions"].read_dict({
+            "ZzDummy": {'enable': 'True'}
+        })
+        self.assertEqual(
+            conf.GetSectionList('user', 'extensions'),
+            ["ZzDummy"])
+        self.assertIn("ZzDummy", conf.GetExtensions())
+        self.assertEqual(
+            conf.GetExtensionKeys("ZzDummy"),
+            {'<<z-in>>': ['<Control-Shift-KeyRelease-Insert>']})
+        self.assertEqual(
+            conf.GetExtensionBindings("ZzDummy"),
+            {'<<z-in>>': ['<Control-Shift-KeyRelease-Insert>'],
+             '<<z-out>>': ['<Control-Shift-KeyRelease-Delete>']})
+        # Restore
+        conf.userCfg["extensions"].remove_section("ZzDummy")
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)