[Python-checkins] bpo-35196: Optimize Squeezer's write() interception (GH-10454)

Tal Einat webhook-mailer at python.org
Sun Jan 13 10:02:04 EST 2019


https://github.com/python/cpython/commit/39a33e99270848d34628cdbb1fdb727f9ede502a
commit: 39a33e99270848d34628cdbb1fdb727f9ede502a
branch: master
author: Tal Einat <taleinat+github at gmail.com>
committer: GitHub <noreply at github.com>
date: 2019-01-13T17:01:50+02:00
summary:

bpo-35196: Optimize Squeezer's write() interception (GH-10454)

The new functionality of Squeezer.reload() is also tested, along with some general
re-working of the tests in test_squeezer.py.

files:
A Misc/NEWS.d/next/IDLE/2018-12-27-17-46-42.bpo-35196.9E-xUh.rst
M Lib/idlelib/NEWS.txt
M Lib/idlelib/editor.py
M Lib/idlelib/idle_test/test_squeezer.py
M Lib/idlelib/pyshell.py
M Lib/idlelib/squeezer.py

diff --git a/Lib/idlelib/NEWS.txt b/Lib/idlelib/NEWS.txt
index 3d93e91d3147..d1748a21bce4 100644
--- a/Lib/idlelib/NEWS.txt
+++ b/Lib/idlelib/NEWS.txt
@@ -3,6 +3,8 @@ Released on 2019-10-20?
 ======================================
 
 
+bpo-35196: Speed up squeezer line counting.
+
 bpo-35208: Squeezer now counts wrapped lines before newlines.
 
 bpo-35555: Gray out Code Context menu entry when it's not applicable.
diff --git a/Lib/idlelib/editor.py b/Lib/idlelib/editor.py
index f4437668a3ed..d13ac3786da4 100644
--- a/Lib/idlelib/editor.py
+++ b/Lib/idlelib/editor.py
@@ -317,9 +317,6 @@ def __init__(self, flist=None, filename=None, key=None, root=None):
         text.bind("<<zoom-height>>", self.ZoomHeight(self).zoom_height_event)
         text.bind("<<toggle-code-context>>",
                   self.CodeContext(self).toggle_code_context_event)
-        squeezer = self.Squeezer(self)
-        text.bind("<<squeeze-current-text>>",
-                  squeezer.squeeze_current_text_event)
 
     def _filename_to_unicode(self, filename):
         """Return filename as BMP unicode so diplayable in Tk."""
diff --git a/Lib/idlelib/idle_test/test_squeezer.py b/Lib/idlelib/idle_test/test_squeezer.py
index da2c2dd50c65..7c28a107a90f 100644
--- a/Lib/idlelib/idle_test/test_squeezer.py
+++ b/Lib/idlelib/idle_test/test_squeezer.py
@@ -1,3 +1,5 @@
+"Test squeezer, coverage 95%"
+
 from collections import namedtuple
 from textwrap import dedent
 from tkinter import Text, Tk
@@ -33,10 +35,10 @@ def cleanup_root():
 
 class CountLinesTest(unittest.TestCase):
     """Tests for the count_lines_with_wrapping function."""
-    def check(self, expected, text, linewidth, tabwidth):
+    def check(self, expected, text, linewidth):
         return self.assertEqual(
             expected,
-            count_lines_with_wrapping(text, linewidth, tabwidth),
+            count_lines_with_wrapping(text, linewidth),
         )
 
     def test_count_empty(self):
@@ -55,37 +57,14 @@ def test_count_several_lines(self):
         """Test with several lines of text."""
         self.assertEqual(count_lines_with_wrapping("1\n2\n3\n"), 3)
 
-    def test_tab_width(self):
-        """Test with various tab widths and line widths."""
-        self.check(expected=1, text='\t' * 1, linewidth=8, tabwidth=4)
-        self.check(expected=1, text='\t' * 2, linewidth=8, tabwidth=4)
-        self.check(expected=2, text='\t' * 3, linewidth=8, tabwidth=4)
-        self.check(expected=2, text='\t' * 4, linewidth=8, tabwidth=4)
-        self.check(expected=3, text='\t' * 5, linewidth=8, tabwidth=4)
-
-        # test longer lines and various tab widths
-        self.check(expected=4, text='\t' * 10, linewidth=12, tabwidth=4)
-        self.check(expected=10, text='\t' * 10, linewidth=12, tabwidth=8)
-        self.check(expected=2, text='\t' * 4, linewidth=10, tabwidth=3)
-
-        # test tabwidth=1
-        self.check(expected=2, text='\t' * 9, linewidth=5, tabwidth=1)
-        self.check(expected=2, text='\t' * 10, linewidth=5, tabwidth=1)
-        self.check(expected=3, text='\t' * 11, linewidth=5, tabwidth=1)
-
-        # test for off-by-one errors
-        self.check(expected=2, text='\t' * 6, linewidth=12, tabwidth=4)
-        self.check(expected=3, text='\t' * 6, linewidth=11, tabwidth=4)
-        self.check(expected=2, text='\t' * 6, linewidth=13, tabwidth=4)
-
     def test_empty_lines(self):
-        self.check(expected=1, text='\n', linewidth=80, tabwidth=8)
-        self.check(expected=2, text='\n\n', linewidth=80, tabwidth=8)
-        self.check(expected=10, text='\n' * 10, linewidth=80, tabwidth=8)
+        self.check(expected=1, text='\n', linewidth=80)
+        self.check(expected=2, text='\n\n', linewidth=80)
+        self.check(expected=10, text='\n' * 10, linewidth=80)
 
     def test_long_line(self):
-        self.check(expected=3, text='a' * 200, linewidth=80, tabwidth=8)
-        self.check(expected=3, text='a' * 200 + '\n', linewidth=80, tabwidth=8)
+        self.check(expected=3, text='a' * 200, linewidth=80)
+        self.check(expected=3, text='a' * 200 + '\n', linewidth=80)
 
     def test_several_lines_different_lengths(self):
         text = dedent("""\
@@ -94,82 +73,78 @@ def test_several_lines_different_lengths(self):
 
             7 chars
             13 characters""")
-        self.check(expected=5, text=text, linewidth=80, tabwidth=8)
-        self.check(expected=5, text=text + '\n', linewidth=80, tabwidth=8)
-        self.check(expected=6, text=text, linewidth=40, tabwidth=8)
-        self.check(expected=7, text=text, linewidth=20, tabwidth=8)
-        self.check(expected=11, text=text, linewidth=10, tabwidth=8)
+        self.check(expected=5, text=text, linewidth=80)
+        self.check(expected=5, text=text + '\n', linewidth=80)
+        self.check(expected=6, text=text, linewidth=40)
+        self.check(expected=7, text=text, linewidth=20)
+        self.check(expected=11, text=text, linewidth=10)
 
 
 class SqueezerTest(unittest.TestCase):
     """Tests for the Squeezer class."""
-    def make_mock_editor_window(self):
+    def tearDown(self):
+        # Clean up the Squeezer class's reference to its instance,
+        # to avoid side-effects from one test case upon another.
+        if Squeezer._instance_weakref is not None:
+            Squeezer._instance_weakref = None
+
+    def make_mock_editor_window(self, with_text_widget=False):
         """Create a mock EditorWindow instance."""
         editwin = NonCallableMagicMock()
         # isinstance(editwin, PyShell) must be true for Squeezer to enable
-        # auto-squeezing; in practice this will always be true
+        # auto-squeezing; in practice this will always be true.
         editwin.__class__ = PyShell
+
+        if with_text_widget:
+            editwin.root = get_test_tk_root(self)
+            text_widget = self.make_text_widget(root=editwin.root)
+            editwin.text = editwin.per.bottom = text_widget
+
         return editwin
 
     def make_squeezer_instance(self, editor_window=None):
         """Create an actual Squeezer instance with a mock EditorWindow."""
         if editor_window is None:
             editor_window = self.make_mock_editor_window()
-        return Squeezer(editor_window)
+        squeezer = Squeezer(editor_window)
+        squeezer.get_line_width = Mock(return_value=80)
+        return squeezer
+
+    def make_text_widget(self, root=None):
+        if root is None:
+            root = get_test_tk_root(self)
+        text_widget = Text(root)
+        text_widget["font"] = ('Courier', 10)
+        text_widget.mark_set("iomark", "1.0")
+        return text_widget
+
+    def set_idleconf_option_with_cleanup(self, configType, section, option, value):
+        prev_val = idleConf.GetOption(configType, section, option)
+        idleConf.SetOption(configType, section, option, value)
+        self.addCleanup(idleConf.SetOption,
+                        configType, section, option, prev_val)
 
     def test_count_lines(self):
-        """Test Squeezer.count_lines() with various inputs.
-
-        This checks that Squeezer.count_lines() calls the
-        count_lines_with_wrapping() function with the appropriate parameters.
-        """
-        for tabwidth, linewidth in [(4, 80), (1, 79), (8, 80), (3, 120)]:
-            self._test_count_lines_helper(linewidth=linewidth,
-                                          tabwidth=tabwidth)
-
-    def _prepare_mock_editwin_for_count_lines(self, editwin,
-                                              linewidth, tabwidth):
-        """Prepare a mock EditorWindow object for Squeezer.count_lines."""
-        CHAR_WIDTH = 10
-        BORDER_WIDTH = 2
-        PADDING_WIDTH = 1
-
-        # Prepare all the required functionality on the mock EditorWindow object
-        # so that the calculations in Squeezer.count_lines() can run.
-        editwin.get_tk_tabwidth.return_value = tabwidth
-        editwin.text.winfo_width.return_value = \
-            linewidth * CHAR_WIDTH + 2 * (BORDER_WIDTH + PADDING_WIDTH)
-        text_opts = {
-            'border': BORDER_WIDTH,
-            'padx': PADDING_WIDTH,
-            'font': None,
-        }
-        editwin.text.cget = lambda opt: text_opts[opt]
-
-        # monkey-path tkinter.font.Font with a mock object, so that
-        # Font.measure('0') returns CHAR_WIDTH
-        mock_font = Mock()
-        def measure(char):
-            if char == '0':
-                return CHAR_WIDTH
-            raise ValueError("measure should only be called on '0'!")
-        mock_font.return_value.measure = measure
-        patcher = patch('idlelib.squeezer.Font', mock_font)
-        patcher.start()
-        self.addCleanup(patcher.stop)
-
-    def _test_count_lines_helper(self, linewidth, tabwidth):
-        """Helper for test_count_lines."""
+        """Test Squeezer.count_lines() with various inputs."""
         editwin = self.make_mock_editor_window()
-        self._prepare_mock_editwin_for_count_lines(editwin, linewidth, tabwidth)
         squeezer = self.make_squeezer_instance(editwin)
 
-        mock_count_lines = Mock(return_value=SENTINEL_VALUE)
-        text = 'TEXT'
-        with patch('idlelib.squeezer.count_lines_with_wrapping',
-                   mock_count_lines):
-            self.assertIs(squeezer.count_lines(text), SENTINEL_VALUE)
-            mock_count_lines.assert_called_with(text, linewidth, tabwidth)
+        for text_code, line_width, expected in [
+            (r"'\n'", 80, 1),
+            (r"'\n' * 3", 80, 3),
+            (r"'a' * 40 + '\n'", 80, 1),
+            (r"'a' * 80 + '\n'", 80, 1),
+            (r"'a' * 200 + '\n'", 80, 3),
+            (r"'aa\t' * 20", 80, 2),
+            (r"'aa\t' * 21", 80, 3),
+            (r"'aa\t' * 20", 40, 4),
+        ]:
+            with self.subTest(text_code=text_code,
+                              line_width=line_width,
+                              expected=expected):
+                text = eval(text_code)
+                squeezer.get_line_width.return_value = line_width
+                self.assertEqual(squeezer.count_lines(text), expected)
 
     def test_init(self):
         """Test the creation of Squeezer instances."""
@@ -207,8 +182,6 @@ def test_write_not_stdout(self):
     def test_write_stdout(self):
         """Test Squeezer's overriding of the EditorWindow's write() method."""
         editwin = self.make_mock_editor_window()
-        self._prepare_mock_editwin_for_count_lines(editwin,
-                                                   linewidth=80, tabwidth=8)
 
         for text in ['', 'TEXT']:
             editwin.write = orig_write = Mock(return_value=SENTINEL_VALUE)
@@ -232,12 +205,8 @@ def test_write_stdout(self):
 
     def test_auto_squeeze(self):
         """Test that the auto-squeezing creates an ExpandingButton properly."""
-        root = get_test_tk_root(self)
-        text_widget = Text(root)
-        text_widget.mark_set("iomark", "1.0")
-
-        editwin = self.make_mock_editor_window()
-        editwin.text = text_widget
+        editwin = self.make_mock_editor_window(with_text_widget=True)
+        text_widget = editwin.text
         squeezer = self.make_squeezer_instance(editwin)
         squeezer.auto_squeeze_min_lines = 5
         squeezer.count_lines = Mock(return_value=6)
@@ -248,58 +217,48 @@ def test_auto_squeeze(self):
 
     def test_squeeze_current_text_event(self):
         """Test the squeeze_current_text event."""
-        root = get_test_tk_root(self)
-
-        # squeezing text should work for both stdout and stderr
+        # Squeezing text should work for both stdout and stderr.
         for tag_name in ["stdout", "stderr"]:
-            text_widget = Text(root)
-            text_widget.mark_set("iomark", "1.0")
-
-            editwin = self.make_mock_editor_window()
-            editwin.text = editwin.per.bottom = text_widget
+            editwin = self.make_mock_editor_window(with_text_widget=True)
+            text_widget = editwin.text
             squeezer = self.make_squeezer_instance(editwin)
             squeezer.count_lines = Mock(return_value=6)
 
-            # prepare some text in the Text widget
+            # Prepare some text in the Text widget.
             text_widget.insert("1.0", "SOME\nTEXT\n", tag_name)
             text_widget.mark_set("insert", "1.0")
             self.assertEqual(text_widget.get('1.0', 'end'), 'SOME\nTEXT\n\n')
 
             self.assertEqual(len(squeezer.expandingbuttons), 0)
 
-            # test squeezing the current text
+            # Test squeezing the current text.
             retval = squeezer.squeeze_current_text_event(event=Mock())
             self.assertEqual(retval, "break")
             self.assertEqual(text_widget.get('1.0', 'end'), '\n\n')
             self.assertEqual(len(squeezer.expandingbuttons), 1)
             self.assertEqual(squeezer.expandingbuttons[0].s, 'SOME\nTEXT')
 
-            # test that expanding the squeezed text works and afterwards the
-            # Text widget contains the original text
+            # Test that expanding the squeezed text works and afterwards
+            # the Text widget contains the original text.
             squeezer.expandingbuttons[0].expand(event=Mock())
             self.assertEqual(text_widget.get('1.0', 'end'), 'SOME\nTEXT\n\n')
             self.assertEqual(len(squeezer.expandingbuttons), 0)
 
     def test_squeeze_current_text_event_no_allowed_tags(self):
         """Test that the event doesn't squeeze text without a relevant tag."""
-        root = get_test_tk_root(self)
-
-        text_widget = Text(root)
-        text_widget.mark_set("iomark", "1.0")
-
-        editwin = self.make_mock_editor_window()
-        editwin.text = editwin.per.bottom = text_widget
+        editwin = self.make_mock_editor_window(with_text_widget=True)
+        text_widget = editwin.text
         squeezer = self.make_squeezer_instance(editwin)
         squeezer.count_lines = Mock(return_value=6)
 
-        # prepare some text in the Text widget
+        # Prepare some text in the Text widget.
         text_widget.insert("1.0", "SOME\nTEXT\n", "TAG")
         text_widget.mark_set("insert", "1.0")
         self.assertEqual(text_widget.get('1.0', 'end'), 'SOME\nTEXT\n\n')
 
         self.assertEqual(len(squeezer.expandingbuttons), 0)
 
-        # test squeezing the current text
+        # Test squeezing the current text.
         retval = squeezer.squeeze_current_text_event(event=Mock())
         self.assertEqual(retval, "break")
         self.assertEqual(text_widget.get('1.0', 'end'), 'SOME\nTEXT\n\n')
@@ -307,23 +266,18 @@ def test_squeeze_current_text_event_no_allowed_tags(self):
 
     def test_squeeze_text_before_existing_squeezed_text(self):
         """Test squeezing text before existing squeezed text."""
-        root = get_test_tk_root(self)
-
-        text_widget = Text(root)
-        text_widget.mark_set("iomark", "1.0")
-
-        editwin = self.make_mock_editor_window()
-        editwin.text = editwin.per.bottom = text_widget
+        editwin = self.make_mock_editor_window(with_text_widget=True)
+        text_widget = editwin.text
         squeezer = self.make_squeezer_instance(editwin)
         squeezer.count_lines = Mock(return_value=6)
 
-        # prepare some text in the Text widget and squeeze it
+        # Prepare some text in the Text widget and squeeze it.
         text_widget.insert("1.0", "SOME\nTEXT\n", "stdout")
         text_widget.mark_set("insert", "1.0")
         squeezer.squeeze_current_text_event(event=Mock())
         self.assertEqual(len(squeezer.expandingbuttons), 1)
 
-        # test squeezing the current text
+        # Test squeezing the current text.
         text_widget.insert("1.0", "MORE\nSTUFF\n", "stdout")
         text_widget.mark_set("insert", "1.0")
         retval = squeezer.squeeze_current_text_event(event=Mock())
@@ -336,27 +290,30 @@ def test_squeeze_text_before_existing_squeezed_text(self):
             squeezer.expandingbuttons[1],
         ))
 
-    GetOptionSignature = namedtuple('GetOptionSignature',
-        'configType section option default type warn_on_default raw')
-    @classmethod
-    def _make_sig(cls, configType, section, option, default=sentinel.NOT_GIVEN,
-                  type=sentinel.NOT_GIVEN,
-                  warn_on_default=sentinel.NOT_GIVEN,
-                  raw=sentinel.NOT_GIVEN):
-        return cls.GetOptionSignature(configType, section, option, default,
-                                      type, warn_on_default, raw)
-
-    @classmethod
-    def get_GetOption_signature(cls, mock_call_obj):
-        args, kwargs = mock_call_obj[-2:]
-        return cls._make_sig(*args, **kwargs)
-
     def test_reload(self):
         """Test the reload() class-method."""
-        self.assertIsInstance(Squeezer.auto_squeeze_min_lines, int)
-        idleConf.SetOption('main', 'PyShell', 'auto-squeeze-min-lines', '42')
+        editwin = self.make_mock_editor_window(with_text_widget=True)
+        text_widget = editwin.text
+        squeezer = self.make_squeezer_instance(editwin)
+
+        orig_zero_char_width = squeezer.zero_char_width
+        orig_auto_squeeze_min_lines = squeezer.auto_squeeze_min_lines
+
+        # Increase both font size and auto-squeeze-min-lines.
+        text_widget["font"] = ('Courier', 20)
+        new_auto_squeeze_min_lines = orig_auto_squeeze_min_lines + 10
+        self.set_idleconf_option_with_cleanup(
+            'main', 'PyShell', 'auto-squeeze-min-lines',
+            str(new_auto_squeeze_min_lines))
+
+        Squeezer.reload()
+        self.assertGreater(squeezer.zero_char_width, orig_zero_char_width)
+        self.assertEqual(squeezer.auto_squeeze_min_lines,
+                         new_auto_squeeze_min_lines)
+
+    def test_reload_no_squeezer_instances(self):
+        """Test that Squeezer.reload() runs without any instances existing."""
         Squeezer.reload()
-        self.assertEqual(Squeezer.auto_squeeze_min_lines, 42)
 
 
 class ExpandingButtonTest(unittest.TestCase):
@@ -369,7 +326,7 @@ def make_mock_squeezer(self):
         squeezer = Mock()
         squeezer.editwin.text = Text(root)
 
-        # Set default values for the configuration settings
+        # Set default values for the configuration settings.
         squeezer.auto_squeeze_min_lines = 50
         return squeezer
 
@@ -382,23 +339,23 @@ def test_init(self, MockHovertip):
         expandingbutton = ExpandingButton('TEXT', 'TAGS', 50, squeezer)
         self.assertEqual(expandingbutton.s, 'TEXT')
 
-        # check that the underlying tkinter.Button is properly configured
+        # Check that the underlying tkinter.Button is properly configured.
         self.assertEqual(expandingbutton.master, text_widget)
         self.assertTrue('50 lines' in expandingbutton.cget('text'))
 
-        # check that the text widget still contains no text
+        # Check that the text widget still contains no text.
         self.assertEqual(text_widget.get('1.0', 'end'), '\n')
 
-        # check that the mouse events are bound
+        # Check that the mouse events are bound.
         self.assertIn('<Double-Button-1>', expandingbutton.bind())
         right_button_code = '<Button-%s>' % ('2' if macosx.isAquaTk() else '3')
         self.assertIn(right_button_code, expandingbutton.bind())
 
-        # check that ToolTip was called once, with appropriate values
+        # Check that ToolTip was called once, with appropriate values.
         self.assertEqual(MockHovertip.call_count, 1)
         MockHovertip.assert_called_with(expandingbutton, ANY, hover_delay=ANY)
 
-        # check that 'right-click' appears in the tooltip text
+        # Check that 'right-click' appears in the tooltip text.
         tooltip_text = MockHovertip.call_args[0][1]
         self.assertIn('right-click', tooltip_text.lower())
 
@@ -407,29 +364,30 @@ def test_expand(self):
         squeezer = self.make_mock_squeezer()
         expandingbutton = ExpandingButton('TEXT', 'TAGS', 50, squeezer)
 
-        # insert the button into the text widget
-        # (this is normally done by the Squeezer class)
+        # Insert the button into the text widget
+        # (this is normally done by the Squeezer class).
         text_widget = expandingbutton.text
         text_widget.window_create("1.0", window=expandingbutton)
 
-        # set base_text to the text widget, so that changes are actually made
-        # to it (by ExpandingButton) and we can inspect these changes afterwards
+        # Set base_text to the text widget, so that changes are actually
+        # made to it (by ExpandingButton) and we can inspect these
+        # changes afterwards.
         expandingbutton.base_text = expandingbutton.text
 
         # trigger the expand event
         retval = expandingbutton.expand(event=Mock())
         self.assertEqual(retval, None)
 
-        # check that the text was inserted into the text widget
+        # Check that the text was inserted into the text widget.
         self.assertEqual(text_widget.get('1.0', 'end'), 'TEXT\n')
 
-        # check that the 'TAGS' tag was set on the inserted text
+        # Check that the 'TAGS' tag was set on the inserted text.
         text_end_index = text_widget.index('end-1c')
         self.assertEqual(text_widget.get('1.0', text_end_index), 'TEXT')
         self.assertEqual(text_widget.tag_nextrange('TAGS', '1.0'),
                           ('1.0', text_end_index))
 
-        # check that the button removed itself from squeezer.expandingbuttons
+        # Check that the button removed itself from squeezer.expandingbuttons.
         self.assertEqual(squeezer.expandingbuttons.remove.call_count, 1)
         squeezer.expandingbuttons.remove.assert_called_with(expandingbutton)
 
@@ -441,55 +399,54 @@ def test_expand_dangerous_oupput(self):
         expandingbutton.set_is_dangerous()
         self.assertTrue(expandingbutton.is_dangerous)
 
-        # insert the button into the text widget
-        # (this is normally done by the Squeezer class)
+        # Insert the button into the text widget
+        # (this is normally done by the Squeezer class).
         text_widget = expandingbutton.text
         text_widget.window_create("1.0", window=expandingbutton)
 
-        # set base_text to the text widget, so that changes are actually made
-        # to it (by ExpandingButton) and we can inspect these changes afterwards
+        # Set base_text to the text widget, so that changes are actually
+        # made to it (by ExpandingButton) and we can inspect these
+        # changes afterwards.
         expandingbutton.base_text = expandingbutton.text
 
-        # patch the message box module to always return False
+        # Patch the message box module to always return False.
         with patch('idlelib.squeezer.tkMessageBox') as mock_msgbox:
             mock_msgbox.askokcancel.return_value = False
             mock_msgbox.askyesno.return_value = False
-
-            # trigger the expand event
+            # Trigger the expand event.
             retval = expandingbutton.expand(event=Mock())
 
-        # check that the event chain was broken and no text was inserted
+        # Check that the event chain was broken and no text was inserted.
         self.assertEqual(retval, 'break')
         self.assertEqual(expandingbutton.text.get('1.0', 'end-1c'), '')
 
-        # patch the message box module to always return True
+        # Patch the message box module to always return True.
         with patch('idlelib.squeezer.tkMessageBox') as mock_msgbox:
             mock_msgbox.askokcancel.return_value = True
             mock_msgbox.askyesno.return_value = True
-
-            # trigger the expand event
+            # Trigger the expand event.
             retval = expandingbutton.expand(event=Mock())
 
-        # check that the event chain wasn't broken and the text was inserted
+        # Check that the event chain wasn't broken and the text was inserted.
         self.assertEqual(retval, None)
         self.assertEqual(expandingbutton.text.get('1.0', 'end-1c'), text)
 
     def test_copy(self):
         """Test the copy event."""
-        # testing with the actual clipboard proved problematic, so this test
-        # replaces the clipboard manipulation functions with mocks and checks
-        # that they are called appropriately
+        # Testing with the actual clipboard proved problematic, so this
+        # test replaces the clipboard manipulation functions with mocks
+        # and checks that they are called appropriately.
         squeezer = self.make_mock_squeezer()
         expandingbutton = ExpandingButton('TEXT', 'TAGS', 50, squeezer)
         expandingbutton.clipboard_clear = Mock()
         expandingbutton.clipboard_append = Mock()
 
-        # trigger the copy event
+        # Trigger the copy event.
         retval = expandingbutton.copy(event=Mock())
         self.assertEqual(retval, None)
 
-        # check that the expanding button called clipboard_clear() and
-        # clipboard_append('TEXT') once each
+        # Vheck that the expanding button called clipboard_clear() and
+        # clipboard_append('TEXT') once each.
         self.assertEqual(expandingbutton.clipboard_clear.call_count, 1)
         self.assertEqual(expandingbutton.clipboard_append.call_count, 1)
         expandingbutton.clipboard_append.assert_called_with('TEXT')
@@ -502,13 +459,13 @@ def test_view(self):
 
         with patch('idlelib.squeezer.view_text', autospec=view_text)\
                 as mock_view_text:
-            # trigger the view event
+            # Trigger the view event.
             expandingbutton.view(event=Mock())
 
-            # check that the expanding button called view_text
+            # Check that the expanding button called view_text.
             self.assertEqual(mock_view_text.call_count, 1)
 
-            # check that the proper text was passed
+            # Check that the proper text was passed.
             self.assertEqual(mock_view_text.call_args[0][2], 'TEXT')
 
     def test_rmenu(self):
diff --git a/Lib/idlelib/pyshell.py b/Lib/idlelib/pyshell.py
index b6172fd6b9bb..ea49aff08b43 100755
--- a/Lib/idlelib/pyshell.py
+++ b/Lib/idlelib/pyshell.py
@@ -899,6 +899,9 @@ def __init__(self, flist=None):
         if use_subprocess:
             text.bind("<<view-restart>>", self.view_restart_mark)
             text.bind("<<restart-shell>>", self.restart_shell)
+        squeezer = self.Squeezer(self)
+        text.bind("<<squeeze-current-text>>",
+                  squeezer.squeeze_current_text_event)
 
         self.save_stdout = sys.stdout
         self.save_stderr = sys.stderr
diff --git a/Lib/idlelib/squeezer.py b/Lib/idlelib/squeezer.py
index 8960356799a4..869498d753a2 100644
--- a/Lib/idlelib/squeezer.py
+++ b/Lib/idlelib/squeezer.py
@@ -15,6 +15,7 @@
 messages and their tracebacks.
 """
 import re
+import weakref
 
 import tkinter as tk
 from tkinter.font import Font
@@ -26,7 +27,7 @@
 from idlelib import macosx
 
 
-def count_lines_with_wrapping(s, linewidth=80, tabwidth=8):
+def count_lines_with_wrapping(s, linewidth=80):
     """Count the number of lines in a given string.
 
     Lines are counted as if the string was wrapped so that lines are never over
@@ -34,25 +35,27 @@ def count_lines_with_wrapping(s, linewidth=80, tabwidth=8):
 
     Tabs are considered tabwidth characters long.
     """
+    tabwidth = 8  # Currently always true in Shell.
     pos = 0
     linecount = 1
     current_column = 0
 
     for m in re.finditer(r"[\t\n]", s):
-        # process the normal chars up to tab or newline
+        # Process the normal chars up to tab or newline.
         numchars = m.start() - pos
         pos += numchars
         current_column += numchars
 
-        # deal with tab or newline
+        # Deal with tab or newline.
         if s[pos] == '\n':
-            # Avoid the `current_column == 0` edge-case, and while we're at it,
-            # don't bother adding 0.
+            # Avoid the `current_column == 0` edge-case, and while we're
+            # at it, don't bother adding 0.
             if current_column > linewidth:
-                # If the current column was exactly linewidth, divmod would give
-                # (1,0), even though a new line hadn't yet been started. The same
-                # is true if length is any exact multiple of linewidth. Therefore,
-                # subtract 1 before dividing a non-empty line.
+                # If the current column was exactly linewidth, divmod
+                # would give (1,0), even though a new line hadn't yet
+                # been started. The same is true if length is any exact
+                # multiple of linewidth. Therefore, subtract 1 before
+                # dividing a non-empty line.
                 linecount += (current_column - 1) // linewidth
             linecount += 1
             current_column = 0
@@ -60,21 +63,21 @@ def count_lines_with_wrapping(s, linewidth=80, tabwidth=8):
             assert s[pos] == '\t'
             current_column += tabwidth - (current_column % tabwidth)
 
-            # if a tab passes the end of the line, consider the entire tab as
-            # being on the next line
+            # If a tab passes the end of the line, consider the entire
+            # tab as being on the next line.
             if current_column > linewidth:
                 linecount += 1
                 current_column = tabwidth
 
-        pos += 1 # after the tab or newline
+        pos += 1 # After the tab or newline.
 
-    # process remaining chars (no more tabs or newlines)
+    # Process remaining chars (no more tabs or newlines).
     current_column += len(s) - pos
-    # avoid divmod(-1, linewidth)
+    # Avoid divmod(-1, linewidth).
     if current_column > 0:
         linecount += (current_column - 1) // linewidth
     else:
-        # the text ended with a newline; don't count an extra line after it
+        # Text ended with newline; don't count an extra line after it.
         linecount -= 1
 
     return linecount
@@ -98,9 +101,7 @@ def __init__(self, s, tags, numoflines, squeezer):
         self.squeezer = squeezer
         self.editwin = editwin = squeezer.editwin
         self.text = text = editwin.text
-
-        # the base Text widget of the PyShell object, used to change text
-        # before the iomark
+        # The base Text widget is needed to change text before iomark.
         self.base_text = editwin.per.bottom
 
         line_plurality = "lines" if numoflines != 1 else "line"
@@ -119,7 +120,7 @@ def __init__(self, s, tags, numoflines, squeezer):
             self.bind("<Button-2>", self.context_menu_event)
         else:
             self.bind("<Button-3>", self.context_menu_event)
-        self.selection_handle(
+        self.selection_handle(  # X windows only.
             lambda offset, length: s[int(offset):int(offset) + int(length)])
 
         self.is_dangerous = None
@@ -182,7 +183,7 @@ def view(self, event=None):
                   modal=False, wrap='none')
 
     rmenu_specs = (
-        # item structure: (label, method_name)
+        # Item structure: (label, method_name).
         ('copy', 'copy'),
         ('view', 'view'),
     )
@@ -202,6 +203,8 @@ class Squeezer:
     This avoids IDLE's shell slowing down considerably, and even becoming
     completely unresponsive, when very long outputs are written.
     """
+    _instance_weakref = None
+
     @classmethod
     def reload(cls):
         """Load class variables from config."""
@@ -210,6 +213,14 @@ def reload(cls):
             type="int", default=50,
         )
 
+        # Loading the font info requires a Tk root. IDLE doesn't rely
+        # on Tkinter's "default root", so the instance will reload
+        # font info using its editor windows's Tk root.
+        if cls._instance_weakref is not None:
+            instance = cls._instance_weakref()
+            if instance is not None:
+                instance.load_font()
+
     def __init__(self, editwin):
         """Initialize settings for Squeezer.
 
@@ -223,44 +234,58 @@ def __init__(self, editwin):
         self.editwin = editwin
         self.text = text = editwin.text
 
-        # Get the base Text widget of the PyShell object, used to change text
-        # before the iomark. PyShell deliberately disables changing text before
-        # the iomark via its 'text' attribute, which is actually a wrapper for
-        # the actual Text widget. Squeezer, however, needs to make such changes.
+        # Get the base Text widget of the PyShell object, used to change
+        # text before the iomark. PyShell deliberately disables changing
+        # text before the iomark via its 'text' attribute, which is
+        # actually a wrapper for the actual Text widget. Squeezer,
+        # however, needs to make such changes.
         self.base_text = editwin.per.bottom
 
+        Squeezer._instance_weakref = weakref.ref(self)
+        self.load_font()
+
+        # Twice the text widget's border width and internal padding;
+        # pre-calculated here for the get_line_width() method.
+        self.window_width_delta = 2 * (
+            int(text.cget('border')) +
+            int(text.cget('padx'))
+        )
+
         self.expandingbuttons = []
-        from idlelib.pyshell import PyShell  # done here to avoid import cycle
-        if isinstance(editwin, PyShell):
-            # If we get a PyShell instance, replace its write method with a
-            # wrapper, which inserts an ExpandingButton instead of a long text.
-            def mywrite(s, tags=(), write=editwin.write):
-                # only auto-squeeze text which has just the "stdout" tag
-                if tags != "stdout":
-                    return write(s, tags)
-
-                # only auto-squeeze text with at least the minimum
-                # configured number of lines
-                numoflines = self.count_lines(s)
-                if numoflines < self.auto_squeeze_min_lines:
-                    return write(s, tags)
-
-                # create an ExpandingButton instance
-                expandingbutton = ExpandingButton(s, tags, numoflines,
-                                                  self)
-
-                # insert the ExpandingButton into the Text widget
-                text.mark_gravity("iomark", tk.RIGHT)
-                text.window_create("iomark", window=expandingbutton,
-                                   padx=3, pady=5)
-                text.see("iomark")
-                text.update()
-                text.mark_gravity("iomark", tk.LEFT)
-
-                # add the ExpandingButton to the Squeezer's list
-                self.expandingbuttons.append(expandingbutton)
-
-            editwin.write = mywrite
+
+        # Replace the PyShell instance's write method with a wrapper,
+        # which inserts an ExpandingButton instead of a long text.
+        def mywrite(s, tags=(), write=editwin.write):
+            # Only auto-squeeze text which has just the "stdout" tag.
+            if tags != "stdout":
+                return write(s, tags)
+
+            # Only auto-squeeze text with at least the minimum
+            # configured number of lines.
+            auto_squeeze_min_lines = self.auto_squeeze_min_lines
+            # First, a very quick check to skip very short texts.
+            if len(s) < auto_squeeze_min_lines:
+                return write(s, tags)
+            # Now the full line-count check.
+            numoflines = self.count_lines(s)
+            if numoflines < auto_squeeze_min_lines:
+                return write(s, tags)
+
+            # Create an ExpandingButton instance.
+            expandingbutton = ExpandingButton(s, tags, numoflines, self)
+
+            # Insert the ExpandingButton into the Text widget.
+            text.mark_gravity("iomark", tk.RIGHT)
+            text.window_create("iomark", window=expandingbutton,
+                               padx=3, pady=5)
+            text.see("iomark")
+            text.update()
+            text.mark_gravity("iomark", tk.LEFT)
+
+            # Add the ExpandingButton to the Squeezer's list.
+            self.expandingbuttons.append(expandingbutton)
+
+        editwin.write = mywrite
 
     def count_lines(self, s):
         """Count the number of lines in a given text.
@@ -273,25 +298,24 @@ def count_lines(self, s):
 
         Tabs are considered tabwidth characters long.
         """
-        # Tab width is configurable
-        tabwidth = self.editwin.get_tk_tabwidth()
-
-        # Get the Text widget's size
-        linewidth = self.editwin.text.winfo_width()
-        # Deduct the border and padding
-        linewidth -= 2*sum([int(self.editwin.text.cget(opt))
-                            for opt in ('border', 'padx')])
-
-        # Get the Text widget's font
-        font = Font(self.editwin.text, name=self.editwin.text.cget('font'))
-        # Divide the size of the Text widget by the font's width.
-        # According to Tk8.5 docs, the Text widget's width is set
-        # according to the width of its font's '0' (zero) character,
-        # so we will use this as an approximation.
-        # see: http://www.tcl.tk/man/tcl8.5/TkCmd/text.htm#M-width
-        linewidth //= font.measure('0')
-
-        return count_lines_with_wrapping(s, linewidth, tabwidth)
+        linewidth = self.get_line_width()
+        return count_lines_with_wrapping(s, linewidth)
+
+    def get_line_width(self):
+        # The maximum line length in pixels: The width of the text
+        # widget, minus twice the border width and internal padding.
+        linewidth_pixels = \
+            self.base_text.winfo_width() - self.window_width_delta
+
+        # Divide the width of the Text widget by the font width,
+        # which is taken to be the width of '0' (zero).
+        # http://www.tcl.tk/man/tcl8.6/TkCmd/text.htm#M21
+        return linewidth_pixels // self.zero_char_width
+
+    def load_font(self):
+        text = self.base_text
+        self.zero_char_width = \
+            Font(text, font=text.cget('font')).measure('0')
 
     def squeeze_current_text_event(self, event):
         """squeeze-current-text event handler
@@ -301,29 +325,29 @@ def squeeze_current_text_event(self, event):
         If the insert cursor is not in a squeezable block of text, give the
         user a small warning and do nothing.
         """
-        # set tag_name to the first valid tag found on the "insert" cursor
+        # Set tag_name to the first valid tag found on the "insert" cursor.
         tag_names = self.text.tag_names(tk.INSERT)
         for tag_name in ("stdout", "stderr"):
             if tag_name in tag_names:
                 break
         else:
-            # the insert cursor doesn't have a "stdout" or "stderr" tag
+            # The insert cursor doesn't have a "stdout" or "stderr" tag.
             self.text.bell()
             return "break"
 
-        # find the range to squeeze
+        # Find the range to squeeze.
         start, end = self.text.tag_prevrange(tag_name, tk.INSERT + "+1c")
         s = self.text.get(start, end)
 
-        # if the last char is a newline, remove it from the range
+        # If the last char is a newline, remove it from the range.
         if len(s) > 0 and s[-1] == '\n':
             end = self.text.index("%s-1c" % end)
             s = s[:-1]
 
-        # delete the text
+        # Delete the text.
         self.base_text.delete(start, end)
 
-        # prepare an ExpandingButton
+        # Prepare an ExpandingButton.
         numoflines = self.count_lines(s)
         expandingbutton = ExpandingButton(s, tag_name, numoflines, self)
 
@@ -331,9 +355,9 @@ def squeeze_current_text_event(self, event):
         self.text.window_create(start, window=expandingbutton,
                                 padx=3, pady=5)
 
-        # insert the ExpandingButton to the list of ExpandingButtons, while
-        # keeping the list ordered according to the position of the buttons in
-        # the Text widget
+        # Insert the ExpandingButton to the list of ExpandingButtons,
+        # while keeping the list ordered according to the position of
+        # the buttons in the Text widget.
         i = len(self.expandingbuttons)
         while i > 0 and self.text.compare(self.expandingbuttons[i-1],
                                           ">", expandingbutton):
diff --git a/Misc/NEWS.d/next/IDLE/2018-12-27-17-46-42.bpo-35196.9E-xUh.rst b/Misc/NEWS.d/next/IDLE/2018-12-27-17-46-42.bpo-35196.9E-xUh.rst
new file mode 100644
index 000000000000..ee90d76010d9
--- /dev/null
+++ b/Misc/NEWS.d/next/IDLE/2018-12-27-17-46-42.bpo-35196.9E-xUh.rst
@@ -0,0 +1 @@
+Speed up squeezer line counting.



More information about the Python-checkins mailing list