bpo-37628: Fix IDLE config sample sizes (GH-14958)

The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
(cherry picked from commit 3221a63c69268a9362802371a616f49d522a5c4f)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>

5 files changed, 129 insertions, 51 deletions

diff --git a/Lib/idlelib/configdialog.py b/Lib/idlelib/configdialog.py
index 217f8fd..4df6ece 100644
--- a/Lib/idlelib/configdialog.py
+++ b/Lib/idlelib/configdialog.py
@@ -33,6 +33,7 @@ from idlelib.codecontext import CodeContext
 from idlelib.parenmatch import ParenMatch
 from idlelib.format import FormatParagraph
 from idlelib.squeezer import Squeezer
+from idlelib.textview import ScrollableTextFrame
 
 changes = ConfigChanges()
 # Reload changed options in the following classes.
@@ -556,7 +557,9 @@ class FontPage(Frame):
                 frame_font_param, variable=self.font_bold,
                 onvalue=1, offvalue=0, text='Bold')
         # frame_sample.
-        self.font_sample = Text(frame_sample, width=20, height=20)
+        font_sample_frame = ScrollableTextFrame(frame_sample)
+        self.font_sample = font_sample_frame.text
+        self.font_sample.config(wrap=NONE, width=1, height=1)
         self.font_sample.insert(END, font_sample_text)
         # frame_indent.
         indent_title = Label(
@@ -568,8 +571,9 @@ class FontPage(Frame):
 
         # Grid and pack widgets:
         self.columnconfigure(1, weight=1)
+        self.rowconfigure(2, weight=1)
         frame_font.grid(row=0, column=0, padx=5, pady=5)
-        frame_sample.grid(row=0, column=1, rowspan=2, padx=5, pady=5,
+        frame_sample.grid(row=0, column=1, rowspan=3, padx=5, pady=5,
                           sticky='nsew')
         frame_indent.grid(row=1, column=0, padx=5, pady=5, sticky='ew')
         # frame_font.
@@ -582,7 +586,7 @@ class FontPage(Frame):
         self.sizelist.pack(side=LEFT, anchor=W)
         self.bold_toggle.pack(side=LEFT, anchor=W, padx=20)
         # frame_sample.
-        self.font_sample.pack(expand=TRUE, fill=BOTH)
+        font_sample_frame.pack(expand=TRUE, fill=BOTH)
         # frame_indent.
         indent_title.pack(side=TOP, anchor=W, padx=5)
         self.indent_scale.pack(side=TOP, padx=5, fill=X)
@@ -840,9 +844,11 @@ class HighPage(Frame):
         frame_theme = LabelFrame(self, borderwidth=2, relief=GROOVE,
                                  text=' Highlighting Theme ')
         # frame_custom.
-        text = self.highlight_sample = Text(
-                frame_custom, relief=SOLID, borderwidth=1,
-                font=('courier', 12, ''), cursor='hand2', width=21, height=13,
+        sample_frame = ScrollableTextFrame(
+                frame_custom, relief=SOLID, borderwidth=1)
+        text = self.highlight_sample = sample_frame.text
+        text.configure(
+                font=('courier', 12, ''), cursor='hand2', width=1, height=1,
                 takefocus=FALSE, highlightthickness=0, wrap=NONE)
         text.bind('<Double-Button-1>', lambda e: 'break')
         text.bind('<B1-Motion>', lambda e: 'break')
@@ -868,7 +874,7 @@ class HighPage(Frame):
         for texttag in text_and_tags:
             text.insert(END, texttag[0], texttag[1])
         n_lines = len(text.get('1.0', END).splitlines())
-        for lineno in range(1, n_lines + 1):
+        for lineno in range(1, n_lines):
             text.insert(f'{lineno}.0',
                         f'{lineno:{len(str(n_lines))}d} ',
                         'linenumber')
@@ -920,9 +926,9 @@ class HighPage(Frame):
         frame_custom.pack(side=LEFT, padx=5, pady=5, expand=TRUE, fill=BOTH)
         frame_theme.pack(side=TOP, padx=5, pady=5, fill=X)
         # frame_custom.
-        self.frame_color_set.pack(side=TOP, padx=5, pady=5, expand=TRUE, fill=X)
+        self.frame_color_set.pack(side=TOP, padx=5, pady=5, fill=X)
         frame_fg_bg_toggle.pack(side=TOP, padx=5, pady=0)
-        self.highlight_sample.pack(
+        sample_frame.pack(
                 side=TOP, padx=5, pady=5, expand=TRUE, fill=BOTH)
         self.button_set_color.pack(side=TOP, expand=TRUE, fill=X, padx=8, pady=4)
         self.targetlist.pack(side=TOP, expand=TRUE, fill=X, padx=8, pady=3)
diff --git a/Lib/idlelib/idle_test/htest.py b/Lib/idlelib/idle_test/htest.py
index f2f37e1..6990af5 100644
--- a/Lib/idlelib/idle_test/htest.py
+++ b/Lib/idlelib/idle_test/htest.py
@@ -349,7 +349,7 @@ _undo_delegator_spec = {
 ViewWindow_spec = {
     'file': 'textview',
     'kwds': {'title': 'Test textview',
-             'text': 'The quick brown fox jumps over the lazy dog.\n'*35,
+             'contents': 'The quick brown fox jumps over the lazy dog.\n'*35,
              '_htest': True},
     'msg': "Test for read-only property of text.\n"
            "Select text, scroll window, close"
diff --git a/Lib/idlelib/idle_test/test_textview.py b/Lib/idlelib/idle_test/test_textview.py
index 6f0c193..7189378 100644
--- a/Lib/idlelib/idle_test/test_textview.py
+++ b/Lib/idlelib/idle_test/test_textview.py
@@ -6,12 +6,12 @@ Using mock Text would not change this.  Other mocks are used to retrieve
 information about calls.
 """
 from idlelib import textview as tv
-import unittest
 from test.support import requires
 requires('gui')
 
 import os
-from tkinter import Tk
+import unittest
+from tkinter import Tk, TclError, CHAR, NONE, WORD
 from tkinter.ttk import Button
 from idlelib.idle_test.mock_idle import Func
 from idlelib.idle_test.mock_tk import Mbox_func
@@ -69,13 +69,65 @@ class ViewWindowTest(unittest.TestCase):
         view.destroy()
 
 
-class TextFrameTest(unittest.TestCase):
+class AutoHideScrollbarTest(unittest.TestCase):
+    # Method set is tested in ScrollableTextFrameTest
+    def test_forbidden_geometry(self):
+        scroll = tv.AutoHideScrollbar(root)
+        self.assertRaises(TclError, scroll.pack)
+        self.assertRaises(TclError, scroll.place)
+
+
+class ScrollableTextFrameTest(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        cls.root = root = Tk()
+        root.withdraw()
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.root.update_idletasks()
+        cls.root.destroy()
+        del cls.root
+
+    def make_frame(self, wrap=NONE, **kwargs):
+        frame = tv.ScrollableTextFrame(self.root, wrap=wrap, **kwargs)
+        def cleanup_frame():
+            frame.update_idletasks()
+            frame.destroy()
+        self.addCleanup(cleanup_frame)
+        return frame
+
+    def test_line1(self):
+        frame = self.make_frame()
+        frame.text.insert('1.0', 'test text')
+        self.assertEqual(frame.text.get('1.0', '1.end'), 'test text')
+
+    def test_horiz_scrollbar(self):
+        # The horizontal scrollbar should be shown/hidden according to
+        # the 'wrap' setting: It should only be shown when 'wrap' is
+        # set to NONE.
+
+        # wrap = NONE -> with horizontal scrolling
+        frame = self.make_frame(wrap=NONE)
+        self.assertEqual(frame.text.cget('wrap'), NONE)
+        self.assertIsNotNone(frame.xscroll)
+
+        # wrap != NONE -> no horizontal scrolling
+        for wrap in [CHAR, WORD]:
+            with self.subTest(wrap=wrap):
+                frame = self.make_frame(wrap=wrap)
+                self.assertEqual(frame.text.cget('wrap'), wrap)
+                self.assertIsNone(frame.xscroll)
+
+
+class ViewFrameTest(unittest.TestCase):
 
     @classmethod
     def setUpClass(cls):
         cls.root = root = Tk()
         root.withdraw()
-        cls.frame = tv.TextFrame(root, 'test text')
+        cls.frame = tv.ViewFrame(root, 'test text')
 
     @classmethod
     def tearDownClass(cls):
diff --git a/Lib/idlelib/textview.py b/Lib/idlelib/textview.py
index 4867a80..808a2ae 100644
--- a/Lib/idlelib/textview.py
+++ b/Lib/idlelib/textview.py
@@ -2,14 +2,15 @@
 
 """
 from tkinter import Toplevel, Text, TclError,\
-    HORIZONTAL, VERTICAL, N, S, E, W
+    HORIZONTAL, VERTICAL, NS, EW, NSEW, NONE, WORD, SUNKEN
 from tkinter.ttk import Frame, Scrollbar, Button
 from tkinter.messagebox import showerror
 
+from functools import update_wrapper
 from idlelib.colorizer import color_config
 
 
-class AutoHiddenScrollbar(Scrollbar):
+class AutoHideScrollbar(Scrollbar):
     """A scrollbar that is automatically hidden when not needed.
 
     Only the grid geometry manager is supported.
@@ -28,52 +29,70 @@ class AutoHiddenScrollbar(Scrollbar):
         raise TclError(f'{self.__class__.__name__} does not support "place"')
 
 
-class TextFrame(Frame):
-    "Display text with scrollbar."
+class ScrollableTextFrame(Frame):
+    """Display text with scrollbar(s)."""
 
-    def __init__(self, parent, rawtext, wrap='word'):
+    def __init__(self, master, wrap=NONE, **kwargs):
         """Create a frame for Textview.
 
-        parent - parent widget for this frame
-        rawtext - text to display
+        master - master widget for this frame
+        wrap - type of text wrapping to use ('word', 'char' or 'none')
+
+        All parameters except for 'wrap' are passed to Frame.__init__().
+
+        The Text widget is accessible via the 'text' attribute.
+
+        Note: Changing the wrapping mode of the text widget after
+        instantiation is not supported.
         """
-        super().__init__(parent)
-        self['relief'] = 'sunken'
-        self['height'] = 700
+        super().__init__(master, **kwargs)
 
-        self.text = text = Text(self, wrap=wrap, highlightthickness=0)
-        color_config(text)
-        text.grid(row=0, column=0, sticky=N+S+E+W)
+        text = self.text = Text(self, wrap=wrap)
+        text.grid(row=0, column=0, sticky=NSEW)
         self.grid_rowconfigure(0, weight=1)
         self.grid_columnconfigure(0, weight=1)
-        text.insert(0.0, rawtext)
-        text['state'] = 'disabled'
-        text.focus_set()
 
         # vertical scrollbar
-        self.yscroll = yscroll = AutoHiddenScrollbar(self, orient=VERTICAL,
-                                                     takefocus=False,
-                                                     command=text.yview)
-        text['yscrollcommand'] = yscroll.set
-        yscroll.grid(row=0, column=1, sticky=N+S)
-
-        if wrap == 'none':
-            # horizontal scrollbar
-            self.xscroll = xscroll = AutoHiddenScrollbar(self, orient=HORIZONTAL,
-                                                         takefocus=False,
-                                                         command=text.xview)
-            text['xscrollcommand'] = xscroll.set
-            xscroll.grid(row=1, column=0, sticky=E+W)
+        self.yscroll = AutoHideScrollbar(self, orient=VERTICAL,
+                                         takefocus=False,
+                                         command=text.yview)
+        self.yscroll.grid(row=0, column=1, sticky=NS)
+        text['yscrollcommand'] = self.yscroll.set
+
+        # horizontal scrollbar - only when wrap is set to NONE
+        if wrap == NONE:
+            self.xscroll = AutoHideScrollbar(self, orient=HORIZONTAL,
+                                             takefocus=False,
+                                             command=text.xview)
+            self.xscroll.grid(row=1, column=0, sticky=EW)
+            text['xscrollcommand'] = self.xscroll.set
+        else:
+            self.xscroll = None
 
 
 class ViewFrame(Frame):
     "Display TextFrame and Close button."
-    def __init__(self, parent, text, wrap='word'):
+    def __init__(self, parent, contents, wrap='word'):
+        """Create a frame for viewing text with a "Close" button.
+
+        parent - parent widget for this frame
+        contents - text to display
+        wrap - type of text wrapping to use ('word', 'char' or 'none')
+
+        The Text widget is accessible via the 'text' attribute.
+        """
         super().__init__(parent)
         self.parent = parent
         self.bind('<Return>', self.ok)
         self.bind('<Escape>', self.ok)
-        self.textframe = TextFrame(self, text, wrap=wrap)
+        self.textframe = ScrollableTextFrame(self, relief=SUNKEN, height=700)
+
+        text = self.text = self.textframe.text
+        text.insert('1.0', contents)
+        text.configure(wrap=wrap, highlightthickness=0, state='disabled')
+        color_config(text)
+        text.focus_set()
+
         self.button_ok = button_ok = Button(
                 self, text='Close', command=self.ok, takefocus=False)
         self.textframe.pack(side='top', expand=True, fill='both')
@@ -87,7 +106,7 @@ class ViewFrame(Frame):
 class ViewWindow(Toplevel):
     "A simple text viewer dialog for IDLE."
 
-    def __init__(self, parent, title, text, modal=True, wrap='word',
+    def __init__(self, parent, title, contents, modal=True, wrap=WORD,
                  *, _htest=False, _utest=False):
         """Show the given text in a scrollable window with a 'close' button.
 
@@ -96,7 +115,7 @@ class ViewWindow(Toplevel):
 
         parent - parent of this dialog
         title - string which is title of popup dialog
-        text - text to display in dialog
+        contents - text to display in dialog
         wrap - type of text wrapping to use ('word', 'char' or 'none')
         _htest - bool; change box location when running htest.
         _utest - bool; don't wait_window when running unittest.
@@ -109,7 +128,7 @@ class ViewWindow(Toplevel):
         self.geometry(f'=750x500+{x}+{y}')
 
         self.title(title)
-        self.viewframe = ViewFrame(self, text, wrap=wrap)
+        self.viewframe = ViewFrame(self, contents, wrap=wrap)
         self.protocol("WM_DELETE_WINDOW", self.ok)
         self.button_ok = button_ok = Button(self, text='Close',
                                             command=self.ok, takefocus=False)
@@ -129,18 +148,18 @@ class ViewWindow(Toplevel):
         self.destroy()
 
 
-def view_text(parent, title, text, modal=True, wrap='word', _utest=False):
+def view_text(parent, title, contents, modal=True, wrap='word', _utest=False):
     """Create text viewer for given text.
 
     parent - parent of this dialog
     title - string which is the title of popup dialog
-    text - text to display in this dialog
+    contents - text to display in this dialog
     wrap - type of text wrapping to use ('word', 'char' or 'none')
     modal - controls if users can interact with other windows while this
             dialog is displayed
     _utest - bool; controls wait_window on unittest
     """
-    return ViewWindow(parent, title, text, modal, wrap=wrap, _utest=_utest)
+    return ViewWindow(parent, title, contents, modal, wrap=wrap, _utest=_utest)
 
 
 def view_file(parent, title, filename, encoding, modal=True, wrap='word',
diff --git a/Misc/NEWS.d/next/IDLE/2019-07-26-17-51-13.bpo-37628.kX4AUF.rst b/Misc/NEWS.d/next/IDLE/2019-07-26-17-51-13.bpo-37628.kX4AUF.rst
new file mode 100644
index 0000000..60910c4
--- /dev/null
+++ b/Misc/NEWS.d/next/IDLE/2019-07-26-17-51-13.bpo-37628.kX4AUF.rst
@@ -0,0 +1 @@
+Settings dialog no longer expands with font size.