bpo-30495: IDLE: improve textview with docstrings, PEP8 names, more tests. (#2283)

Split TextViewer class into ViewWindow, ViewFrame, and TextFrame classes so that instances
of the latter two can be placed with other widgets within a multiframe window.
Patch by Cheryl Sabella.

6 files changed, 141 insertions, 100 deletions

diff --git a/Lib/idlelib/idle_test/htest.py b/Lib/idlelib/idle_test/htest.py
index 6f676ae..e483bbc 100644
--- a/Lib/idlelib/idle_test/htest.py
+++ b/Lib/idlelib/idle_test/htest.py
@@ -8,7 +8,7 @@ callable in the module named in the spec.  Close the window to skip or
 end the test.
 
 In a tested module, let X be a global name bound to a callable (class
-or function) whose .__name__ attrubute is also X (the usual situation).
+or function) whose .__name__ attribute is also X (the usual situation).
 The first parameter of X must be 'parent'.  When called, the parent
 argument will be the root window.  X must create a child Toplevel
 window (or subclass thereof).  The Toplevel may be a test widget or
@@ -306,15 +306,6 @@ _tabbed_pages_spec = {
            "<nothing> is an invalid add page and remove page name.\n"
     }
 
-TextViewer_spec = {
-    'file': 'textview',
-    'kwds': {'title': 'Test textview',
-             'text':'The quick brown fox jumps over the lazy dog.\n'*35,
-             '_htest': True},
-    'msg': "Test for read-only property of text.\n"
-           "Text is selectable. Window is scrollable.",
-     }
-
 _tooltip_spec = {
     'file': 'tooltip',
     'kwds': {},
@@ -338,6 +329,15 @@ _undo_delegator_spec = {
            "by printing to the console or the IDLE shell.\n"
     }
 
+ViewWindow_spec = {
+    'file': 'textview',
+    'kwds': {'title': 'Test textview',
+             'text': '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"
+     }
+
 _widget_redirector_spec = {
     'file': 'redirector',
     'kwds': {},
diff --git a/Lib/idlelib/idle_test/test_help_about.py b/Lib/idlelib/idle_test/test_help_about.py
index 08c38a5..1f67aad 100644
--- a/Lib/idlelib/idle_test/test_help_about.py
+++ b/Lib/idlelib/idle_test/test_help_about.py
@@ -50,19 +50,17 @@ class LiveDialogTest(unittest.TestCase):
     def test_printer_buttons(self):
         """Test buttons whose commands use printer function."""
         dialog = self.dialog
-        button_sources = [(self.dialog.py_license, license),
-                          (self.dialog.py_copyright, copyright),
-                          (self.dialog.py_credits, credits)]
+        button_sources = [(dialog.py_license, license),
+                          (dialog.py_copyright, copyright),
+                          (dialog.py_credits, credits)]
 
         for button, printer in button_sources:
             printer._Printer__setup()
             button.invoke()
+            get = dialog._current_textview.viewframe.textframe.text.get
+            self.assertEqual(printer._Printer__lines[0], get('1.0', '1.end'))
             self.assertEqual(
-                    printer._Printer__lines[0],
-                    dialog._current_textview.text.get('1.0', '1.end'))
-            self.assertEqual(
-                    printer._Printer__lines[1],
-                    dialog._current_textview.text.get('2.0', '2.end'))
+                    printer._Printer__lines[1], get('2.0', '2.end'))
             dialog._current_textview.destroy()
 
     def test_file_buttons(self):
@@ -75,14 +73,11 @@ class LiveDialogTest(unittest.TestCase):
         for button, filename in button_sources:
             button.invoke()
             fn = findfile(filename, subdir='idlelib')
+            get = dialog._current_textview.viewframe.textframe.text.get
             with open(fn) as f:
-                self.assertEqual(
-                        f.readline().strip(),
-                        dialog._current_textview.text.get('1.0', '1.end'))
+                self.assertEqual(f.readline().strip(), get('1.0', '1.end'))
                 f.readline()
-                self.assertEqual(
-                    f.readline().strip(),
-                    dialog._current_textview.text.get('3.0', '3.end'))
+                self.assertEqual(f.readline().strip(), get('3.0', '3.end'))
             dialog._current_textview.destroy()
 
 
diff --git a/Lib/idlelib/idle_test/test_textview.py b/Lib/idlelib/idle_test/test_textview.py
index 7a2f7e4..c129c2f 100644
--- a/Lib/idlelib/idle_test/test_textview.py
+++ b/Lib/idlelib/idle_test/test_textview.py
@@ -1,7 +1,7 @@
 '''Test idlelib.textview.
 
-Since all methods and functions create (or destroy) a TextViewer, which
-is a widget containing multiple widgets, all tests must be gui tests.
+Since all methods and functions create (or destroy) a ViewWindow, which
+is a widget containing a widget, etcetera, all tests must be gui tests.
 Using mock Text would not change this.  Other mocks are used to retrieve
 information about calls.
 
@@ -13,7 +13,8 @@ requires('gui')
 
 import unittest
 import os
-from tkinter import Tk, Button
+from tkinter import Tk
+from tkinter.ttk import Button
 from idlelib.idle_test.mock_idle import Func
 from idlelib.idle_test.mock_tk import Mbox_func
 
@@ -25,44 +26,44 @@ def setUpModule():
 def tearDownModule():
     global root
     root.update_idletasks()
-    root.destroy()  # Pyflakes falsely sees root as undefined.
+    root.destroy()
     del root
 
-# If we call TextViewer or wrapper functions with defaults
+# If we call ViewWindow or wrapper functions with defaults
 # modal=True, _utest=False, test hangs on call to wait_window.
 # Have also gotten tk error 'can't invoke "event" command'.
 
 
-class TV(tv.TextViewer):  # Used in TextViewTest.
+class VW(tv.ViewWindow):  # Used in ViewWindowTest.
     transient = Func()
     grab_set = Func()
     wait_window = Func()
 
 
-# Call wrapper class with mock wait_window.
-class TextViewTest(unittest.TestCase):
+# Call wrapper class VW with mock wait_window.
+class ViewWindowTest(unittest.TestCase):
 
     def setUp(self):
-        TV.transient.__init__()
-        TV.grab_set.__init__()
-        TV.wait_window.__init__()
+        VW.transient.__init__()
+        VW.grab_set.__init__()
+        VW.wait_window.__init__()
 
     def test_init_modal(self):
-        view = TV(root, 'Title', 'test text')
-        self.assertTrue(TV.transient.called)
-        self.assertTrue(TV.grab_set.called)
-        self.assertTrue(TV.wait_window.called)
+        view = VW(root, 'Title', 'test text')
+        self.assertTrue(VW.transient.called)
+        self.assertTrue(VW.grab_set.called)
+        self.assertTrue(VW.wait_window.called)
         view.ok()
 
     def test_init_nonmodal(self):
-        view = TV(root, 'Title', 'test text', modal=False)
-        self.assertFalse(TV.transient.called)
-        self.assertFalse(TV.grab_set.called)
-        self.assertFalse(TV.wait_window.called)
+        view = VW(root, 'Title', 'test text', modal=False)
+        self.assertFalse(VW.transient.called)
+        self.assertFalse(VW.grab_set.called)
+        self.assertFalse(VW.wait_window.called)
         view.ok()
 
     def test_ok(self):
-        view = TV(root, 'Title', 'test text', modal=False)
+        view = VW(root, 'Title', 'test text', modal=False)
         view.destroy = Func()
         view.ok()
         self.assertTrue(view.destroy.called)
@@ -70,7 +71,28 @@ class TextViewTest(unittest.TestCase):
         view.destroy()
 
 
-# Call TextViewer with modal=False.
+class TextFrameTest(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        "By itself, this tests that file parsed without exception."
+        cls.root = root = Tk()
+        root.withdraw()
+        cls.frame = tv.TextFrame(root, 'test text')
+
+    @classmethod
+    def tearDownClass(cls):
+        del cls.frame
+        cls.root.update_idletasks()
+        cls.root.destroy()
+        del cls.root
+
+    def test_line1(self):
+        get = self.frame.text.get
+        self.assertEqual(get('1.0', '1.end'), 'test text')
+
+
+# Call ViewWindow with modal=False.
 class ViewFunctionTest(unittest.TestCase):
 
     @classmethod
@@ -85,13 +107,16 @@ class ViewFunctionTest(unittest.TestCase):
 
     def test_view_text(self):
         view = tv.view_text(root, 'Title', 'test text', modal=False)
-        self.assertIsInstance(view, tv.TextViewer)
+        self.assertIsInstance(view, tv.ViewWindow)
+        self.assertIsInstance(view.viewframe, tv.ViewFrame)
         view.ok()
 
     def test_view_file(self):
         view = tv.view_file(root, 'Title', __file__, modal=False)
-        self.assertIsInstance(view, tv.TextViewer)
-        self.assertIn('Test', view.text.get('1.0', '1.end'))
+        self.assertIsInstance(view, tv.ViewWindow)
+        self.assertIsInstance(view.viewframe, tv.ViewFrame)
+        get = view.viewframe.textframe.text.get
+        self.assertIn('Test', get('1.0', '1.end'))
         view.ok()
 
     def test_bad_file(self):
@@ -109,8 +134,7 @@ class ViewFunctionTest(unittest.TestCase):
         self.assertEqual(tv.showerror.title, 'Unicode Decode Error')
 
 
-
-# Call TextViewer with _utest=True.
+# Call ViewWindow with _utest=True.
 class ButtonClickTest(unittest.TestCase):
 
     def setUp(self):
@@ -131,7 +155,8 @@ class ButtonClickTest(unittest.TestCase):
 
         self.assertEqual(self.called, True)
         self.assertEqual(self.view.title(), 'TITLE_TEXT')
-        self.assertEqual(self.view.text.get('1.0', '1.end'), 'COMMAND')
+        self.assertEqual(self.view.viewframe.textframe.text.get('1.0', '1.end'),
+                         'COMMAND')
 
     def test_view_file_bind_with_button(self):
         def _command():
@@ -143,12 +168,11 @@ class ButtonClickTest(unittest.TestCase):
 
         self.assertEqual(self.called, True)
         self.assertEqual(self.view.title(), 'TITLE_FILE')
+        get = self.view.viewframe.textframe.text.get
         with open(__file__) as f:
-            self.assertEqual(self.view.text.get('1.0', '1.end'),
-                             f.readline().strip())
+            self.assertEqual(get('1.0', '1.end'), f.readline().strip())
             f.readline()
-            self.assertEqual(self.view.text.get('3.0', '3.end'),
-                             f.readline().strip())
+            self.assertEqual(get('3.0', '3.end'), f.readline().strip())
 
 
 if __name__ == '__main__':
diff --git a/Lib/idlelib/pyshell.py b/Lib/idlelib/pyshell.py
index 5b0e5b2..be7bd3d 100755
--- a/Lib/idlelib/pyshell.py
+++ b/Lib/idlelib/pyshell.py
@@ -892,7 +892,7 @@ class PyShell(OutputWindow):
         try:
             # page help() text to shell.
             import pydoc # import must be done here to capture i/o rebinding.
-            # XXX KBK 27Dec07 use TextViewer someday, but must work w/o subproc
+            # XXX KBK 27Dec07 use text viewer someday, but must work w/o subproc
             pydoc.pager = pydoc.plainpager
         except:
             sys.stderr = sys.__stderr__
diff --git a/Lib/idlelib/textview.py b/Lib/idlelib/textview.py
index ab653a9..de4b190 100644
--- a/Lib/idlelib/textview.py
+++ b/Lib/idlelib/textview.py
@@ -1,14 +1,59 @@
 """Simple text browser for IDLE
 
 """
-from tkinter import Toplevel, Frame, Button, Text
-from tkinter import DISABLED, SUNKEN, VERTICAL, WORD
-from tkinter import RIGHT, LEFT, TOP, BOTTOM, BOTH, X, Y
-from tkinter.ttk import Scrollbar
+from tkinter import Toplevel, Text
+from tkinter.ttk import Frame, Scrollbar, Button
 from tkinter.messagebox import showerror
 
 
-class TextViewer(Toplevel):
+class TextFrame(Frame):
+    "Display text with scrollbar."
+
+    def __init__(self, parent, rawtext):
+        """Create a frame for Textview.
+
+        parent - parent widget for this frame
+        rawtext - text to display
+        """
+        super().__init__(parent)
+        self['relief'] = 'sunken'
+        self['height'] = 700
+        # TODO: get fg/bg from theme.
+        self.bg = '#ffffff'
+        self.fg = '#000000'
+
+        self.text = text = Text(self, wrap='word', highlightthickness=0,
+                                fg=self.fg, bg=self.bg)
+        self.scroll = scroll = Scrollbar(self, orient='vertical',
+                                         takefocus=False, command=text.yview)
+        text['yscrollcommand'] = scroll.set
+        text.insert(0.0, rawtext)
+        text['state'] = 'disabled'
+        text.focus_set()
+
+        scroll.pack(side='right', fill='y')
+        text.pack(side='left', expand=True, fill='both')
+
+
+class ViewFrame(Frame):
+    "Display TextFrame and Close button."
+    def __init__(self, parent, text):
+        super().__init__(parent)
+        self.parent = parent
+        self.bind('<Return>', self.ok)
+        self.bind('<Escape>', self.ok)
+        self.textframe = TextFrame(self, text)
+        self.button_ok = button_ok = Button(
+                self, text='Close', command=self.ok, takefocus=False)
+        self.textframe.pack(side='top', expand=True, fill='both')
+        button_ok.pack(side='bottom')
+
+    def ok(self, event=None):
+        """Dismiss text viewer dialog."""
+        self.parent.destroy()
+
+
+class ViewWindow(Toplevel):
     "A simple text viewer dialog for IDLE."
 
     def __init__(self, parent, title, text, modal=True,
@@ -24,26 +69,19 @@ class TextViewer(Toplevel):
         _htest - bool; change box location when running htest.
         _utest - bool; don't wait_window when running unittest.
         """
-        Toplevel.__init__(self, parent)
-        self.configure(borderwidth=5)
+        super().__init__(parent)
+        self['borderwidth'] = 5
         # Place dialog below parent if running htest.
-        self.geometry("=%dx%d+%d+%d" % (750, 500,
-                           parent.winfo_rootx() + 10,
-                           parent.winfo_rooty() + (10 if not _htest else 100)))
-        # TODO: get fg/bg from theme.
-        self.bg = '#ffffff'
-        self.fg = '#000000'
+        x = parent.winfo_rootx() + 10
+        y = parent.winfo_rooty() + (10 if not _htest else 100)
+        self.geometry(f'=750x500+{x}+{y}')
 
-        self.create_widgets()
         self.title(title)
+        self.viewframe = ViewFrame(self, text)
         self.protocol("WM_DELETE_WINDOW", self.ok)
-        self.parent = parent
-        self.text.focus_set()
-        # Bind keys for closing this dialog.
-        self.bind('<Return>', self.ok)
-        self.bind('<Escape>', self.ok)
-        self.text.insert(0.0, text)
-        self.text.config(state=DISABLED)
+        self.button_ok = button_ok = Button(self, text='Close',
+                                            command=self.ok, takefocus=False)
+        self.viewframe.pack(side='top', expand=True, fill='both')
 
         if modal:
             self.transient(parent)
@@ -51,31 +89,13 @@ class TextViewer(Toplevel):
             if not _utest:
                 self.wait_window()
 
-    def create_widgets(self):
-        "Create Frame with Text (with vertical Scrollbar) and Button."
-        frame = Frame(self, relief=SUNKEN, height=700)
-        frame_buttons = Frame(self)
-        self.button_ok = Button(frame_buttons, text='Close',
-                                command=self.ok, takefocus=False)
-        self.scrollbar = Scrollbar(frame, orient=VERTICAL, takefocus=False)
-        self.text = Text(frame, wrap=WORD, highlightthickness=0,
-                             fg=self.fg, bg=self.bg)
-        self.scrollbar.config(command=self.text.yview)
-        self.text.config(yscrollcommand=self.scrollbar.set)
-        
-        self.button_ok.pack()
-        self.scrollbar.pack(side=RIGHT, fill=Y)
-        self.text.pack(side=LEFT, expand=True, fill=BOTH)
-        frame_buttons.pack(side=BOTTOM, fill=X)
-        frame.pack(side=TOP, expand=True, fill=BOTH)
-
     def ok(self, event=None):
         """Dismiss text viewer dialog."""
         self.destroy()
 
 
 def view_text(parent, title, text, modal=True, _utest=False):
-    """Create TextViewer for given text.
+    """Create text viewer for given text.
 
     parent - parent of this dialog
     title - string which is the title of popup dialog
@@ -84,11 +104,11 @@ def view_text(parent, title, text, modal=True, _utest=False):
             dialog is displayed
     _utest - bool; controls wait_window on unittest
     """
-    return TextViewer(parent, title, text, modal, _utest=_utest)
+    return ViewWindow(parent, title, text, modal, _utest=_utest)
 
 
 def view_file(parent, title, filename, encoding=None, modal=True, _utest=False):
-    """Create TextViewer for text in filename.
+    """Create text viewer for text in filename.
 
     Return error message if file cannot be read.  Otherwise calls view_text
     with contents of the file.
@@ -98,7 +118,7 @@ def view_file(parent, title, filename, encoding=None, modal=True, _utest=False):
             contents = file.read()
     except OSError:
         showerror(title='File Load Error',
-                  message='Unable to load file %r .' % filename,
+                  message=f'Unable to load file {filename!r} .',
                   parent=parent)
     except UnicodeDecodeError as err:
         showerror(title='Unicode Decode Error',
@@ -113,4 +133,4 @@ if __name__ == '__main__':
     import unittest
     unittest.main('idlelib.idle_test.test_textview', verbosity=2, exit=False)
     from idlelib.idle_test.htest import run
-    run(TextViewer)
+    run(ViewWindow)
diff --git a/Misc/NEWS.d/next/IDLE/2017-06-29-18-23-06.bpo-30495.qIWgc4.rst b/Misc/NEWS.d/next/IDLE/2017-06-29-18-23-06.bpo-30495.qIWgc4.rst
new file mode 100644
index 0000000..5e046e0
--- /dev/null
+++ b/Misc/NEWS.d/next/IDLE/2017-06-29-18-23-06.bpo-30495.qIWgc4.rst
@@ -0,0 +1,2 @@
+IDLE: Improve textview with docstrings, PEP8 names, and more tests. Patch by
+Cheryl Sabella.