bpo-35675: IDLE - separate config_key window and frame (#11427)

bpo-35598: IDLE: Refactor window and frame class

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

Lib/idlelib/config_key.py

@@ -41,32 +41,22 @@ def translate_key(key, modifiers):
     return f'Key-{key}'
 
 
-class GetKeysDialog(Toplevel):
+class GetKeysFrame(Frame):
 
     # Dialog title for invalid key sequence
     keyerror_title = 'Key Sequence Error'
 
-    def __init__(self, parent, title, action, current_key_sequences,
+    def __init__(self, parent, action, current_key_sequences):
-                 *, _htest=False, _utest=False):
         """
         parent - parent of this dialog
-        title - string which is the title of the popup dialog
+        action - the name of the virtual event these keys will be
-        action - string, the name of the virtual event these keys will be
                  mapped to
-        current_key_sequences - list, a list of all key sequence lists
+        current_key_sequences - a list of all key sequence lists
                  currently mapped to virtual events, for overlap checking
-        _htest - bool, change box location when running htest
-        _utest - bool, do not wait when running unittest
         """
-        Toplevel.__init__(self, parent)
+        super().__init__(parent)
-        self.withdraw()  # Hide while setting geometry.
+        self['borderwidth'] = 2
-        self.configure(borderwidth=5)
+        self['relief'] = 'sunken'
-        self.resizable(height=False, width=False)
-        self.title(title)
-        self.transient(parent)
-        _setup_dialog(self)
-        self.grab_set()
-        self.protocol("WM_DELETE_WINDOW", self.cancel)
         self.parent = parent
         self.action = action
         self.current_key_sequences = current_key_sequences
@@ -82,39 +72,14 @@ def __init__(self, parent, title, action, current_key_sequences,
             self.modifier_vars.append(variable)
         self.advanced = False
         self.create_widgets()
-        self.update_idletasks()
-        self.geometry(
-                "+%d+%d" % (
-                    parent.winfo_rootx() +
-                    (parent.winfo_width()/2 - self.winfo_reqwidth()/2),
-                    parent.winfo_rooty() +
-                    ((parent.winfo_height()/2 - self.winfo_reqheight()/2)
-                    if not _htest else 150)
-                ) )  # Center dialog over parent (or below htest box).
-        if not _utest:
-            self.deiconify()  # Geometry set, unhide.
-            self.wait_window()
 
     def showerror(self, *args, **kwargs):
         # Make testing easier.  Replace in #30751.
         messagebox.showerror(*args, **kwargs)
 
     def create_widgets(self):
-        self.frame = frame = Frame(self, borderwidth=2, relief='sunken')
-        frame.pack(side='top', expand=True, fill='both')
-
-        frame_buttons = Frame(self)
-        frame_buttons.pack(side='bottom', fill='x')
-
-        self.button_ok = Button(frame_buttons, text='OK',
-                                width=8, command=self.ok)
-        self.button_ok.grid(row=0, column=0, padx=5, pady=5)
-        self.button_cancel = Button(frame_buttons, text='Cancel',
-                                   width=8, command=self.cancel)
-        self.button_cancel.grid(row=0, column=1, padx=5, pady=5)
-
         # Basic entry key sequence.
-        self.frame_keyseq_basic = Frame(frame, name='keyseq_basic')
+        self.frame_keyseq_basic = Frame(self, name='keyseq_basic')
         self.frame_keyseq_basic.grid(row=0, column=0, sticky='nsew',
                                       padx=5, pady=5)
         basic_title = Label(self.frame_keyseq_basic,
@@ -127,7 +92,7 @@ def create_widgets(self):
         basic_keys.pack(ipadx=5, ipady=5, fill='x')
 
         # Basic entry controls.
-        self.frame_controls_basic = Frame(frame)
+        self.frame_controls_basic = Frame(self)
         self.frame_controls_basic.grid(row=1, column=0, sticky='nsew', padx=5)
 
         # Basic entry modifiers.
@@ -169,7 +134,7 @@ def create_widgets(self):
         self.button_clear.grid(row=2, column=0, columnspan=4)
 
         # Advanced entry key sequence.
-        self.frame_keyseq_advanced = Frame(frame, name='keyseq_advanced')
+        self.frame_keyseq_advanced = Frame(self, name='keyseq_advanced')
         self.frame_keyseq_advanced.grid(row=0, column=0, sticky='nsew',
                                          padx=5, pady=5)
         advanced_title = Label(self.frame_keyseq_advanced, justify='left',
@@ -181,7 +146,7 @@ def create_widgets(self):
         self.advanced_keys.pack(fill='x')
 
         # Advanced entry help text.
-        self.frame_help_advanced = Frame(frame)
+        self.frame_help_advanced = Frame(self)
         self.frame_help_advanced.grid(row=1, column=0, sticky='nsew', padx=5)
         help_advanced = Label(self.frame_help_advanced, justify='left',
             text="Key bindings are specified using Tkinter keysyms as\n"+
@@ -196,7 +161,7 @@ def create_widgets(self):
         help_advanced.grid(row=0, column=0, sticky='nsew')
 
         # Switch between basic and advanced.
-        self.button_level = Button(frame, command=self.toggle_level,
+        self.button_level = Button(self, command=self.toggle_level,
                                   text='<< Basic Key Binding Entry')
         self.button_level.grid(row=2, column=0, stick='ew', padx=5, pady=5)
         self.toggle_level()
@@ -257,7 +222,8 @@ def clear_key_seq(self):
             variable.set('')
         self.key_string.set('')
 
-    def ok(self, event=None):
+    def ok(self):
+        self.result = ''
         keys = self.key_string.get().strip()
         if not keys:
             self.showerror(title=self.keyerror_title, parent=self,
@@ -265,13 +231,7 @@ def ok(self, event=None):
             return
         if (self.advanced or self.keys_ok(keys)) and self.bind_ok(keys):
             self.result = keys
-        self.grab_release()
+        return
-        self.destroy()
-
-    def cancel(self, event=None):
-        self.result = ''
-        self.grab_release()
-        self.destroy()
 
     def keys_ok(self, keys):
         """Validity check on user's 'basic' keybinding selection.
@@ -319,6 +279,73 @@ def bind_ok(self, keys):
             return True
 
 
+class GetKeysWindow(Toplevel):
+
+    def __init__(self, parent, title, action, current_key_sequences,
+                 *, _htest=False, _utest=False):
+        """
+        parent - parent of this dialog
+        title - string which is the title of the popup dialog
+        action - string, the name of the virtual event these keys will be
+                 mapped to
+        current_key_sequences - list, a list of all key sequence lists
+                 currently mapped to virtual events, for overlap checking
+        _htest - bool, change box location when running htest
+        _utest - bool, do not wait when running unittest
+        """
+        super().__init__(parent)
+        self.withdraw()  # Hide while setting geometry.
+        self['borderwidth'] = 5
+        self.resizable(height=False, width=False)
+        # Needed for winfo_reqwidth().
+        self.update_idletasks()
+        # Center dialog over parent (or below htest box).
+        x = (parent.winfo_rootx() +
+             (parent.winfo_width()//2 - self.winfo_reqwidth()//2))
+        y = (parent.winfo_rooty() +
+             ((parent.winfo_height()//2 - self.winfo_reqheight()//2)
+              if not _htest else 150))
+        self.geometry(f"+{x}+{y}")
+
+        self.title(title)
+        self.frame = frame = GetKeysFrame(self, action, current_key_sequences)
+        self.protocol("WM_DELETE_WINDOW", self.cancel)
+        frame_buttons = Frame(self)
+        self.button_ok = Button(frame_buttons, text='OK',
+                                width=8, command=self.ok)
+        self.button_cancel = Button(frame_buttons, text='Cancel',
+                                   width=8, command=self.cancel)
+        self.button_ok.grid(row=0, column=0, padx=5, pady=5)
+        self.button_cancel.grid(row=0, column=1, padx=5, pady=5)
+        frame.pack(side='top', expand=True, fill='both')
+        frame_buttons.pack(side='bottom', fill='x')
+
+        self.transient(parent)
+        _setup_dialog(self)
+        self.grab_set()
+        if not _utest:
+            self.deiconify()  # Geometry set, unhide.
+            self.wait_window()
+
+    @property
+    def result(self):
+        return self.frame.result
+
+    @result.setter
+    def result(self, value):
+        self.frame.result = value
+
+    def ok(self, event=None):
+        self.frame.ok()
+        self.grab_release()
+        self.destroy()
+
+    def cancel(self, event=None):
+        self.result = ''
+        self.grab_release()
+        self.destroy()
+
+
 if __name__ == '__main__':
     from unittest import main
     main('idlelib.idle_test.test_config_key', verbosity=2, exit=False)

Lib/idlelib/configdialog.py

@@ -24,7 +24,7 @@
 from tkinter import messagebox
 
 from idlelib.config import idleConf, ConfigChanges
-from idlelib.config_key import GetKeysDialog
+from idlelib.config_key import GetKeysWindow
 from idlelib.dynoption import DynOptionMenu
 from idlelib import macosx
 from idlelib.query import SectionName, HelpSource
@@ -1397,7 +1397,7 @@ def get_new_keys(self):
             for event in key_set_changes:
                 current_bindings[event] = key_set_changes[event].split()
         current_key_sequences = list(current_bindings.values())
-        new_keys = GetKeysDialog(self, 'Get New Keys', bind_name,
+        new_keys = GetKeysWindow(self, 'Get New Keys', bind_name,
                 current_key_sequences).result
         if new_keys:
             if self.keyset_source.get():  # Current key set is a built-in.

Lib/idlelib/idle_test/test_config_key.py

@@ -13,15 +13,13 @@
 from idlelib.idle_test.mock_idle import Func
 from idlelib.idle_test.mock_tk import Mbox_func
 
-gkd = config_key.GetKeysDialog
-
 
 class ValidationTest(unittest.TestCase):
     "Test validation methods: ok, keys_ok, bind_ok."
 
-    class Validator(gkd):
+    class Validator(config_key.GetKeysFrame):
         def __init__(self, *args, **kwargs):
-            config_key.GetKeysDialog.__init__(self, *args, **kwargs)
+            super().__init__(*args, **kwargs)
             class list_keys_final:
                 get = Func()
             self.list_keys_final = list_keys_final
@@ -34,15 +32,14 @@ def setUpClass(cls):
         cls.root = Tk()
         cls.root.withdraw()
         keylist = [['<Key-F12>'], ['<Control-Key-x>', '<Control-Key-X>']]
-        cls.dialog = cls.Validator(
+        cls.dialog = cls.Validator(cls.root, '<<Test>>', keylist)
-            cls.root, 'Title', '<<Test>>', keylist, _utest=True)
 
     @classmethod
     def tearDownClass(cls):
-        cls.dialog.cancel()
+        del cls.dialog
         cls.root.update_idletasks()
         cls.root.destroy()
-        del cls.dialog, cls.root
+        del cls.root
 
     def setUp(self):
         self.dialog.showerror.message = ''
@@ -111,14 +108,14 @@ def setUpClass(cls):
         requires('gui')
         cls.root = Tk()
         cls.root.withdraw()
-        cls.dialog = gkd(cls.root, 'Title', '<<Test>>', [], _utest=True)
+        cls.dialog = config_key.GetKeysFrame(cls.root, '<<Test>>', [])
 
     @classmethod
     def tearDownClass(cls):
-        cls.dialog.cancel()
+        del cls.dialog
         cls.root.update_idletasks()
         cls.root.destroy()
-        del cls.dialog, cls.root
+        del cls.root
 
     def test_toggle_level(self):
         dialog = self.dialog
@@ -130,7 +127,7 @@ def stackorder():
             this can be used to check whether a frame is above or
             below another one.
             """
-            for index, child in enumerate(dialog.frame.winfo_children()):
+            for index, child in enumerate(dialog.winfo_children()):
                 if child._name == 'keyseq_basic':
                     basic = index
                 if child._name == 'keyseq_advanced':
@@ -161,7 +158,7 @@ def stackorder():
 class KeySelectionTest(unittest.TestCase):
     "Test selecting key on Basic frames."
 
-    class Basic(gkd):
+    class Basic(config_key.GetKeysFrame):
         def __init__(self, *args, **kwargs):
             super().__init__(*args, **kwargs)
             class list_keys_final:
@@ -179,14 +176,14 @@ def setUpClass(cls):
         requires('gui')
         cls.root = Tk()
         cls.root.withdraw()
-        cls.dialog = cls.Basic(cls.root, 'Title', '<<Test>>', [], _utest=True)
+        cls.dialog = cls.Basic(cls.root, '<<Test>>', [])
 
     @classmethod
     def tearDownClass(cls):
-        cls.dialog.cancel()
+        del cls.dialog
         cls.root.update_idletasks()
         cls.root.destroy()
-        del cls.dialog, cls.root
+        del cls.root
 
     def setUp(self):
         self.dialog.clear_key_seq()
@@ -206,7 +203,7 @@ def test_get_modifiers(self):
         dialog.modifier_checkbuttons['foo'].invoke()
         eq(gm(), ['BAZ'])
 
-    @mock.patch.object(gkd, 'get_modifiers')
+    @mock.patch.object(config_key.GetKeysFrame, 'get_modifiers')
     def test_build_key_string(self, mock_modifiers):
         dialog = self.dialog
         key = dialog.list_keys_final
@@ -227,7 +224,7 @@ def test_build_key_string(self, mock_modifiers):
         dialog.build_key_string()
         eq(string(), '<mymod-test>')
 
-    @mock.patch.object(gkd, 'get_modifiers')
+    @mock.patch.object(config_key.GetKeysFrame, 'get_modifiers')
     def test_final_key_selected(self, mock_modifiers):
         dialog = self.dialog
         key = dialog.list_keys_final
@@ -240,7 +237,7 @@ def test_final_key_selected(self, mock_modifiers):
         eq(string(), '<Shift-Key-braceleft>')
 
 
-class CancelTest(unittest.TestCase):
+class CancelWindowTest(unittest.TestCase):
     "Simulate user clicking [Cancel] button."
 
     @classmethod
@@ -248,21 +245,89 @@ def setUpClass(cls):
         requires('gui')
         cls.root = Tk()
         cls.root.withdraw()
-        cls.dialog = gkd(cls.root, 'Title', '<<Test>>', [], _utest=True)
+        cls.dialog = config_key.GetKeysWindow(
+            cls.root, 'Title', '<<Test>>', [], _utest=True)
 
     @classmethod
     def tearDownClass(cls):
         cls.dialog.cancel()
+        del cls.dialog
         cls.root.update_idletasks()
         cls.root.destroy()
-        del cls.dialog, cls.root
+        del cls.root
 
-    def test_cancel(self):
+    @mock.patch.object(config_key.GetKeysFrame, 'ok')
+    def test_cancel(self, mock_frame_ok):
         self.assertEqual(self.dialog.winfo_class(), 'Toplevel')
         self.dialog.button_cancel.invoke()
         with self.assertRaises(TclError):
             self.dialog.winfo_class()
         self.assertEqual(self.dialog.result, '')
+        mock_frame_ok.assert_not_called()
+
+
+class OKWindowTest(unittest.TestCase):
+    "Simulate user clicking [OK] button."
+
+    @classmethod
+    def setUpClass(cls):
+        requires('gui')
+        cls.root = Tk()
+        cls.root.withdraw()
+        cls.dialog = config_key.GetKeysWindow(
+            cls.root, 'Title', '<<Test>>', [], _utest=True)
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.dialog.cancel()
+        del cls.dialog
+        cls.root.update_idletasks()
+        cls.root.destroy()
+        del cls.root
+
+    @mock.patch.object(config_key.GetKeysFrame, 'ok')
+    def test_ok(self, mock_frame_ok):
+        self.assertEqual(self.dialog.winfo_class(), 'Toplevel')
+        self.dialog.button_ok.invoke()
+        with self.assertRaises(TclError):
+            self.dialog.winfo_class()
+        mock_frame_ok.assert_called()
+
+
+class WindowResultTest(unittest.TestCase):
+    "Test window result get and set."
+
+    @classmethod
+    def setUpClass(cls):
+        requires('gui')
+        cls.root = Tk()
+        cls.root.withdraw()
+        cls.dialog = config_key.GetKeysWindow(
+            cls.root, 'Title', '<<Test>>', [], _utest=True)
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.dialog.cancel()
+        del cls.dialog
+        cls.root.update_idletasks()
+        cls.root.destroy()
+        del cls.root
+
+    def test_result(self):
+        dialog = self.dialog
+        eq = self.assertEqual
+
+        dialog.result = ''
+        eq(dialog.result, '')
+        eq(dialog.frame.result,'')
+
+        dialog.result = 'bar'
+        eq(dialog.result,'bar')
+        eq(dialog.frame.result,'bar')
+
+        dialog.frame.result = 'foo'
+        eq(dialog.result, 'foo')
+        eq(dialog.frame.result,'foo')
 
 
 class HelperTest(unittest.TestCase):

Lib/idlelib/idle_test/test_configdialog.py

@@ -954,8 +954,8 @@ def test_set_keys_type(self):
     def test_get_new_keys(self):
         eq = self.assertEqual
         d = self.page
-        orig_getkeysdialog = configdialog.GetKeysDialog
+        orig_getkeysdialog = configdialog.GetKeysWindow
-        gkd = configdialog.GetKeysDialog = Func(return_self=True)
+        gkd = configdialog.GetKeysWindow = Func(return_self=True)
         gnkn = d.get_new_keys_name = Func()
 
         d.button_new_keys.state(('!disabled',))
@@ -997,7 +997,7 @@ def test_get_new_keys(self):
         eq(d.keybinding.get(), '<Key-p>')
 
         del d.get_new_keys_name
-        configdialog.GetKeysDialog = orig_getkeysdialog
+        configdialog.GetKeysWindow = orig_getkeysdialog
 
     def test_get_new_keys_name(self):
         orig_sectionname = configdialog.SectionName