bpo-40468: Move IDLE helplist settings to extensions page of dialog. (GH-26593)

These are the settings that extend the help menu.  Moving them shortens the dialog and will help with it being too tall for small screens.

2 files changed, 188 insertions, 139 deletions

diff --git a/Lib/idlelib/configdialog.py b/Lib/idlelib/configdialog.py
index c52a04b..6638c06 100644
--- a/Lib/idlelib/configdialog.py
+++ b/Lib/idlelib/configdialog.py
@@ -268,8 +268,6 @@ class ConfigDialog(Toplevel):
             set_extension_value: Set in userCfg['extensions'].
             save_all_changed_extensions: Call extension page Save().
         """
-        parent = self.parent
-        frame = Frame(self.note)
         self.ext_defaultCfg = idleConf.defaultCfg['extensions']
         self.ext_userCfg = idleConf.userCfg['extensions']
         self.is_int = self.register(is_int)
@@ -277,18 +275,21 @@ class ConfigDialog(Toplevel):
         # Create widgets - a listbox shows all available extensions, with the
         # controls for the extension selected in the listbox to the right.
         self.extension_names = StringVar(self)
-        frame.rowconfigure(0, weight=1)
-        frame.columnconfigure(2, weight=1)
-        self.extension_list = Listbox(frame, listvariable=self.extension_names,
+        frame = Frame(self.note)
+        frame_ext = LabelFrame(frame, borderwidth=2, relief=GROOVE,
+                               text=' Feature Extensions ')
+        frame_ext.rowconfigure(0, weight=1)
+        frame_ext.columnconfigure(2, weight=1)
+        self.extension_list = Listbox(frame_ext, listvariable=self.extension_names,
                                       selectmode='browse')
         self.extension_list.bind('<<ListboxSelect>>', self.extension_selected)
-        scroll = Scrollbar(frame, command=self.extension_list.yview)
+        scroll = Scrollbar(frame_ext, command=self.extension_list.yview)
         self.extension_list.yscrollcommand=scroll.set
-        self.details_frame = LabelFrame(frame, width=250, height=250)
+        self.details_frame = LabelFrame(frame_ext, width=250, height=250)
         self.extension_list.grid(column=0, row=0, sticky='nws')
         scroll.grid(column=1, row=0, sticky='ns')
         self.details_frame.grid(column=2, row=0, sticky='nsew', padx=[10, 0])
-        frame.configure(padding=10)
+        frame_ext.configure(padding=10)
         self.config_frame = {}
         self.current_extension = None
 
@@ -304,6 +305,13 @@ class ConfigDialog(Toplevel):
         self.extension_list.selection_set(0)
         self.extension_selected(None)
 
+
+        self.frame_help = HelpFrame(frame, borderwidth=2, relief=GROOVE,
+                                    text=' Help Menu Extensions ')
+        frame_ext.grid(row=0, column=0, sticky='nsew')
+        Label(frame).grid(row=1, column=0)
+        self.frame_help.grid(row=2, column=0, sticky='sew')
+
         return frame
 
     def load_extensions(self):
@@ -1854,14 +1862,6 @@ class GenPage(Frame):
                 frame_auto_squeeze_min_lines: Frame
                     auto_squeeze_min_lines_title: Label
                     (*)auto_squeeze_min_lines_int: Entry - auto_squeeze_min_lines
-            frame_help: LabelFrame
-                frame_helplist: Frame
-                    frame_helplist_buttons: Frame
-                        (*)button_helplist_edit
-                        (*)button_helplist_add
-                        (*)button_helplist_remove
-                    (*)helplist: ListBox
-                    scroll_helplist: Scrollbar
         """
         # Integer values need StringVar because int('') raises.
         self.startup_edit = tracers.add(
@@ -1902,8 +1902,6 @@ class GenPage(Frame):
                                   text=' Editor Preferences')
         frame_shell = LabelFrame(self, borderwidth=2, relief=GROOVE,
                                  text=' Shell Preferences')
-        frame_help = LabelFrame(self, borderwidth=2, relief=GROOVE,
-                                text=' Additional Help Sources ')
         # Frame_window.
         frame_run = Frame(frame_window, borderwidth=0)
         startup_title = Label(frame_run, text='At Startup')
@@ -1999,32 +1997,11 @@ class GenPage(Frame):
                 validatecommand=self.digits_only, validate='key',
         )
 
-        # frame_help.
-        frame_helplist = Frame(frame_help)
-        frame_helplist_buttons = Frame(frame_helplist)
-        self.helplist = Listbox(
-                frame_helplist, height=5, takefocus=True,
-                exportselection=FALSE)
-        scroll_helplist = Scrollbar(frame_helplist)
-        scroll_helplist['command'] = self.helplist.yview
-        self.helplist['yscrollcommand'] = scroll_helplist.set
-        self.helplist.bind('<ButtonRelease-1>', self.help_source_selected)
-        self.button_helplist_edit = Button(
-                frame_helplist_buttons, text='Edit', state='disabled',
-                width=8, command=self.helplist_item_edit)
-        self.button_helplist_add = Button(
-                frame_helplist_buttons, text='Add',
-                width=8, command=self.helplist_item_add)
-        self.button_helplist_remove = Button(
-                frame_helplist_buttons, text='Remove', state='disabled',
-                width=8, command=self.helplist_item_remove)
-
         # Pack widgets:
         # Body.
         frame_window.pack(side=TOP, padx=5, pady=5, expand=TRUE, fill=BOTH)
         frame_editor.pack(side=TOP, padx=5, pady=5, expand=TRUE, fill=BOTH)
         frame_shell.pack(side=TOP, padx=5, pady=5, expand=TRUE, fill=BOTH)
-        frame_help.pack(side=TOP, padx=5, pady=5, expand=TRUE, fill=BOTH)
         # frame_run.
         frame_run.pack(side=TOP, padx=5, pady=0, fill=X)
         startup_title.pack(side=LEFT, anchor=W, padx=5, pady=5)
@@ -2077,17 +2054,12 @@ class GenPage(Frame):
         auto_squeeze_min_lines_title.pack(side=LEFT, anchor=W, padx=5, pady=5)
         self.auto_squeeze_min_lines_int.pack(side=TOP, padx=5, pady=5)
 
-        # frame_help.
-        frame_helplist_buttons.pack(side=RIGHT, padx=5, pady=5, fill=Y)
-        frame_helplist.pack(side=TOP, padx=5, pady=5, expand=TRUE, fill=BOTH)
-        scroll_helplist.pack(side=RIGHT, anchor=W, fill=Y)
-        self.helplist.pack(side=LEFT, anchor=E, expand=TRUE, fill=BOTH)
-        self.button_helplist_edit.pack(side=TOP, anchor=W, pady=5)
-        self.button_helplist_add.pack(side=TOP, anchor=W)
-        self.button_helplist_remove.pack(side=TOP, anchor=W, pady=5)
-
     def load_general_cfg(self):
         "Load current configuration settings for the general options."
+        self.load_windows_cfg()
+        self.load_shelled_cfg()
+
+    def load_windows_cfg(self):
         # Set variables for all windows.
         self.startup_edit.set(idleConf.GetOption(
                 'main', 'General', 'editor-on-startup', type='bool'))
@@ -2106,6 +2078,7 @@ class GenPage(Frame):
         self.paren_bell.set(idleConf.GetOption(
                 'extensions', 'ParenMatch', 'bell'))
 
+    def load_shelled_cfg(self):
         # Set variables for editor windows.
         self.autosave.set(idleConf.GetOption(
                 'main', 'General', 'autosave', default=0, type='bool'))
@@ -2120,12 +2093,63 @@ class GenPage(Frame):
         self.auto_squeeze_min_lines.set(idleConf.GetOption(
                 'main', 'PyShell', 'auto-squeeze-min-lines', type='int'))
 
-        # Set additional help sources.
-        self.user_helplist = idleConf.GetAllExtraHelpSourcesList()
-        self.helplist.delete(0, 'end')
-        for help_item in self.user_helplist:
-            self.helplist.insert(END, help_item[0])
-        self.set_add_delete_state()
+
+class HelpFrame(LabelFrame):
+
+    def __init__(self, master, **cfg):
+        super().__init__(master, **cfg)
+        self.create_frame_help()
+        self.load_helplist()
+
+    def create_frame_help(self):
+        """Create LabelFrame for additional help menu sources.
+
+        load_helplist loads list user_helplist with
+        name, position pairs and copies names to listbox helplist.
+        Clicking a name invokes help_source selected. Clicking
+        button_helplist_name invokes helplist_item_name, which also
+        changes user_helplist.  These functions all call
+        set_add_delete_state. All but load call update_help_changes to
+        rewrite changes['main']['HelpFiles'].
+
+        Widgets for HelpFrame(LabelFrame):  (*) widgets bound to self
+            frame_helplist: Frame
+                (*)helplist: ListBox
+                scroll_helplist: Scrollbar
+            frame_buttons: Frame
+                (*)button_helplist_edit
+                (*)button_helplist_add
+                (*)button_helplist_remove
+        """
+        # self = frame_help in dialog (until ExtPage class).
+        frame_helplist = Frame(self)
+        self.helplist = Listbox(
+                frame_helplist, height=5, takefocus=True,
+                exportselection=FALSE)
+        scroll_helplist = Scrollbar(frame_helplist)
+        scroll_helplist['command'] = self.helplist.yview
+        self.helplist['yscrollcommand'] = scroll_helplist.set
+        self.helplist.bind('<ButtonRelease-1>', self.help_source_selected)
+
+        frame_buttons = Frame(self)
+        self.button_helplist_edit = Button(
+                frame_buttons, text='Edit', state='disabled',
+                width=8, command=self.helplist_item_edit)
+        self.button_helplist_add = Button(
+                frame_buttons, text='Add',
+                width=8, command=self.helplist_item_add)
+        self.button_helplist_remove = Button(
+                frame_buttons, text='Remove', state='disabled',
+                width=8, command=self.helplist_item_remove)
+
+        # Pack frame_help.
+        frame_helplist.pack(side=LEFT, padx=5, pady=5, expand=TRUE, fill=BOTH)
+        self.helplist.pack(side=LEFT, anchor=E, expand=TRUE, fill=BOTH)
+        scroll_helplist.pack(side=RIGHT, anchor=W, fill=Y)
+        frame_buttons.pack(side=RIGHT, padx=5, pady=5, fill=Y)
+        self.button_helplist_edit.pack(side=TOP, anchor=W, pady=5)
+        self.button_helplist_add.pack(side=TOP, anchor=W)
+        self.button_helplist_remove.pack(side=TOP, anchor=W, pady=5)
 
     def help_source_selected(self, event):
         "Handle event for selecting additional help."
@@ -2195,6 +2219,14 @@ class GenPage(Frame):
                     'main', 'HelpFiles', str(num),
                     ';'.join(self.user_helplist[num-1][:2]))
 
+    def load_helplist(self):
+        # Set additional help sources.
+        self.user_helplist = idleConf.GetAllExtraHelpSourcesList()
+        self.helplist.delete(0, 'end')
+        for help_item in self.user_helplist:
+            self.helplist.insert(END, help_item[0])
+        self.set_add_delete_state()
+
 
 class VarTrace:
     """Maintain Tk variables trace state."""
diff --git a/Lib/idlelib/idle_test/test_configdialog.py b/Lib/idlelib/idle_test/test_configdialog.py
index 98ddc67..214d1b3 100644
--- a/Lib/idlelib/idle_test/test_configdialog.py
+++ b/Lib/idlelib/idle_test/test_configdialog.py
@@ -1207,24 +1207,14 @@ class GenPageTest(unittest.TestCase):
     """Test that general tab widgets enable users to make changes.
 
     Test that widget actions set vars, that var changes add
-    options to changes and that helplist works correctly.
+    options to changes.
     """
     @classmethod
     def setUpClass(cls):
         page = cls.page = dialog.genpage
         dialog.note.select(page)
-        page.set = page.set_add_delete_state = Func()
-        page.upc = page.update_help_changes = Func()
         page.update()
 
-    @classmethod
-    def tearDownClass(cls):
-        page = cls.page
-        del page.set, page.set_add_delete_state
-        del page.upc, page.update_help_changes
-        page.helplist.delete(0, 'end')
-        page.user_helplist.clear()
-
     def setUp(self):
         changes.clear()
 
@@ -1236,16 +1226,11 @@ class GenPageTest(unittest.TestCase):
         d.autosave.set(1)
         d.win_width.set(1)
         d.win_height.set(1)
-        d.helplist.insert('end', 'bad')
-        d.user_helplist = ['bad', 'worse']
-        idleConf.SetOption('main', 'HelpFiles', '1', 'name;file')
         d.load_general_cfg()
         eq(d.startup_edit.get(), 0)
         eq(d.autosave.get(), 0)
         eq(d.win_width.get(), '80')
         eq(d.win_height.get(), '40')
-        eq(d.helplist.get(0, 'end'), ('name',))
-        eq(d.user_helplist, [('name', 'file', '1')])
 
     def test_startup(self):
         d = self.page
@@ -1306,11 +1291,43 @@ class GenPageTest(unittest.TestCase):
         self.page.context_int.insert(0, '1')
         self.assertEqual(extpage, {'CodeContext': {'maxlines': '1'}})
 
+
+class HelpSourceTest(unittest.TestCase):
+    """Test that the help source list works correctly."""
+    @classmethod
+    def setUpClass(cls):
+        dialog.note.select(dialog.extpage)
+        frame = cls.frame = dialog.frame_help
+        frame.set = frame.set_add_delete_state = Func()
+        frame.upc = frame.update_help_changes = Func()
+        frame.update()
+
+    @classmethod
+    def tearDownClass(cls):
+        frame = cls.frame
+        del frame.set, frame.set_add_delete_state
+        del frame.upc, frame.update_help_changes
+        frame.helplist.delete(0, 'end')
+        frame.user_helplist.clear()
+
+    def setUp(self):
+        changes.clear()
+
+    def test_load_helplist(self):
+        eq = self.assertEqual
+        fr = self.frame
+        fr.helplist.insert('end', 'bad')
+        fr.user_helplist = ['bad', 'worse']
+        idleConf.SetOption('main', 'HelpFiles', '1', 'name;file')
+        fr.load_helplist()
+        eq(fr.helplist.get(0, 'end'), ('name',))
+        eq(fr.user_helplist, [('name', 'file', '1')])
+
     def test_source_selected(self):
-        d = self.page
-        d.set = d.set_add_delete_state
-        d.upc = d.update_help_changes
-        helplist = d.helplist
+        fr = self.frame
+        fr.set = fr.set_add_delete_state
+        fr.upc = fr.update_help_changes
+        helplist = fr.helplist
         dex = 'end'
         helplist.insert(dex, 'source')
         helplist.activate(dex)
@@ -1321,38 +1338,38 @@ class GenPageTest(unittest.TestCase):
         x, y, dx, dy = helplist.bbox(dex)
         x += dx // 2
         y += dy // 2
-        d.set.called = d.upc.called = 0
+        fr.set.called = fr.upc.called = 0
         helplist.event_generate('<Enter>', x=0, y=0)
         helplist.event_generate('<Motion>', x=x, y=y)
         helplist.event_generate('<Button-1>', x=x, y=y)
         helplist.event_generate('<ButtonRelease-1>', x=x, y=y)
         self.assertEqual(helplist.get('anchor'), 'source')
-        self.assertTrue(d.set.called)
-        self.assertFalse(d.upc.called)
+        self.assertTrue(fr.set.called)
+        self.assertFalse(fr.upc.called)
 
     def test_set_add_delete_state(self):
         # Call with 0 items, 1 unselected item, 1 selected item.
         eq = self.assertEqual
-        d = self.page
-        del d.set_add_delete_state  # Unmask method.
-        sad = d.set_add_delete_state
-        h = d.helplist
+        fr = self.frame
+        del fr.set_add_delete_state  # Unmask method.
+        sad = fr.set_add_delete_state
+        h = fr.helplist
 
         h.delete(0, 'end')
         sad()
-        eq(d.button_helplist_edit.state(), ('disabled',))
-        eq(d.button_helplist_remove.state(), ('disabled',))
+        eq(fr.button_helplist_edit.state(), ('disabled',))
+        eq(fr.button_helplist_remove.state(), ('disabled',))
 
         h.insert(0, 'source')
         sad()
-        eq(d.button_helplist_edit.state(), ('disabled',))
-        eq(d.button_helplist_remove.state(), ('disabled',))
+        eq(fr.button_helplist_edit.state(), ('disabled',))
+        eq(fr.button_helplist_remove.state(), ('disabled',))
 
         h.selection_set(0)
         sad()
-        eq(d.button_helplist_edit.state(), ())
-        eq(d.button_helplist_remove.state(), ())
-        d.set_add_delete_state = Func()  # Mask method.
+        eq(fr.button_helplist_edit.state(), ())
+        eq(fr.button_helplist_remove.state(), ())
+        fr.set_add_delete_state = Func()  # Mask method.
 
     def test_helplist_item_add(self):
         # Call without and twice with HelpSource result.
@@ -1360,25 +1377,25 @@ class GenPageTest(unittest.TestCase):
         eq = self.assertEqual
         orig_helpsource = configdialog.HelpSource
         hs = configdialog.HelpSource = Func(return_self=True)
-        d = self.page
-        d.helplist.delete(0, 'end')
-        d.user_helplist.clear()
-        d.set.called = d.upc.called = 0
+        fr = self.frame
+        fr.helplist.delete(0, 'end')
+        fr.user_helplist.clear()
+        fr.set.called = fr.upc.called = 0
 
         hs.result = ''
-        d.helplist_item_add()
-        self.assertTrue(list(d.helplist.get(0, 'end')) ==
-                        d.user_helplist == [])
-        self.assertFalse(d.upc.called)
+        fr.helplist_item_add()
+        self.assertTrue(list(fr.helplist.get(0, 'end')) ==
+                        fr.user_helplist == [])
+        self.assertFalse(fr.upc.called)
 
         hs.result = ('name1', 'file1')
-        d.helplist_item_add()
+        fr.helplist_item_add()
         hs.result = ('name2', 'file2')
-        d.helplist_item_add()
-        eq(d.helplist.get(0, 'end'), ('name1', 'name2'))
-        eq(d.user_helplist, [('name1', 'file1'), ('name2', 'file2')])
-        eq(d.upc.called, 2)
-        self.assertFalse(d.set.called)
+        fr.helplist_item_add()
+        eq(fr.helplist.get(0, 'end'), ('name1', 'name2'))
+        eq(fr.user_helplist, [('name1', 'file1'), ('name2', 'file2')])
+        eq(fr.upc.called, 2)
+        self.assertFalse(fr.set.called)
 
         configdialog.HelpSource = orig_helpsource
 
@@ -1387,58 +1404,58 @@ class GenPageTest(unittest.TestCase):
         eq = self.assertEqual
         orig_helpsource = configdialog.HelpSource
         hs = configdialog.HelpSource = Func(return_self=True)
-        d = self.page
-        d.helplist.delete(0, 'end')
-        d.helplist.insert(0, 'name1')
-        d.helplist.selection_set(0)
-        d.helplist.selection_anchor(0)
-        d.user_helplist.clear()
-        d.user_helplist.append(('name1', 'file1'))
-        d.set.called = d.upc.called = 0
+        fr = self.frame
+        fr.helplist.delete(0, 'end')
+        fr.helplist.insert(0, 'name1')
+        fr.helplist.selection_set(0)
+        fr.helplist.selection_anchor(0)
+        fr.user_helplist.clear()
+        fr.user_helplist.append(('name1', 'file1'))
+        fr.set.called = fr.upc.called = 0
 
         hs.result = ''
-        d.helplist_item_edit()
+        fr.helplist_item_edit()
         hs.result = ('name1', 'file1')
-        d.helplist_item_edit()
-        eq(d.helplist.get(0, 'end'), ('name1',))
-        eq(d.user_helplist, [('name1', 'file1')])
-        self.assertFalse(d.upc.called)
+        fr.helplist_item_edit()
+        eq(fr.helplist.get(0, 'end'), ('name1',))
+        eq(fr.user_helplist, [('name1', 'file1')])
+        self.assertFalse(fr.upc.called)
 
         hs.result = ('name2', 'file2')
-        d.helplist_item_edit()
-        eq(d.helplist.get(0, 'end'), ('name2',))
-        eq(d.user_helplist, [('name2', 'file2')])
-        self.assertTrue(d.upc.called == d.set.called == 1)
+        fr.helplist_item_edit()
+        eq(fr.helplist.get(0, 'end'), ('name2',))
+        eq(fr.user_helplist, [('name2', 'file2')])
+        self.assertTrue(fr.upc.called == fr.set.called == 1)
 
         configdialog.HelpSource = orig_helpsource
 
     def test_helplist_item_remove(self):
         eq = self.assertEqual
-        d = self.page
-        d.helplist.delete(0, 'end')
-        d.helplist.insert(0, 'name1')
-        d.helplist.selection_set(0)
-        d.helplist.selection_anchor(0)
-        d.user_helplist.clear()
-        d.user_helplist.append(('name1', 'file1'))
-        d.set.called = d.upc.called = 0
-
-        d.helplist_item_remove()
-        eq(d.helplist.get(0, 'end'), ())
-        eq(d.user_helplist, [])
-        self.assertTrue(d.upc.called == d.set.called == 1)
+        fr = self.frame
+        fr.helplist.delete(0, 'end')
+        fr.helplist.insert(0, 'name1')
+        fr.helplist.selection_set(0)
+        fr.helplist.selection_anchor(0)
+        fr.user_helplist.clear()
+        fr.user_helplist.append(('name1', 'file1'))
+        fr.set.called = fr.upc.called = 0
+
+        fr.helplist_item_remove()
+        eq(fr.helplist.get(0, 'end'), ())
+        eq(fr.user_helplist, [])
+        self.assertTrue(fr.upc.called == fr.set.called == 1)
 
     def test_update_help_changes(self):
-        d = self.page
-        del d.update_help_changes
-        d.user_helplist.clear()
-        d.user_helplist.append(('name1', 'file1'))
-        d.user_helplist.append(('name2', 'file2'))
+        fr = self.frame
+        del fr.update_help_changes
+        fr.user_helplist.clear()
+        fr.user_helplist.append(('name1', 'file1'))
+        fr.user_helplist.append(('name2', 'file2'))
 
-        d.update_help_changes()
+        fr.update_help_changes()
         self.assertEqual(mainpage['HelpFiles'],
                          {'1': 'name1;file1', '2': 'name2;file2'})
-        d.update_help_changes = Func()
+        fr.update_help_changes = Func()
 
 
 class VarTraceTest(unittest.TestCase):