bpo-37929: IDLE: avoid Squeezer-related config dialog crashes (GH-15452)
authorTal Einat <taleinat+github@gmail.com>
Sun, 25 Aug 2019 05:52:58 +0000 (08:52 +0300)
committerGitHub <noreply@github.com>
Sun, 25 Aug 2019 05:52:58 +0000 (08:52 +0300)
These were caused by keeping around a reference to the Squeezer
instance and calling it's load_font() upon config changes, which
sometimes happened even if the shell window no longer existed.

This change completely removes that mechanism, instead having the
editor window properly update its width attribute, which can then
be used by Squeezer.

Lib/idlelib/editor.py
Lib/idlelib/idle_test/test_squeezer.py
Lib/idlelib/squeezer.py
Misc/NEWS.d/next/IDLE/2019-08-24-22-00-33.bpo-37929.jb7523.rst [new file with mode: 0644]

index 35027da76bce2fc8a339f6094b8e503fe8446999..793ed3afaed0ff5f8aa186091f678640bfaeb21a 100644 (file)
@@ -10,6 +10,7 @@ import traceback
 import webbrowser
 
 from tkinter import *
+from tkinter.font import Font
 from tkinter.ttk import Scrollbar
 import tkinter.simpledialog as tkSimpleDialog
 import tkinter.messagebox as tkMessageBox
@@ -120,14 +121,13 @@ class EditorWindow(object):
         self.prompt_last_line = ''  # Override in PyShell
         self.text_frame = text_frame = Frame(top)
         self.vbar = vbar = Scrollbar(text_frame, name='vbar')
-        self.width = idleConf.GetOption('main', 'EditorWindow',
-                                        'width', type='int')
+        width = idleConf.GetOption('main', 'EditorWindow', 'width', type='int')
         text_options = {
                 'name': 'text',
                 'padx': 5,
                 'wrap': 'none',
                 'highlightthickness': 0,
-                'width': self.width,
+                'width': width,
                 'tabstyle': 'wordprocessor',  # new in 8.5
                 'height': idleConf.GetOption(
                         'main', 'EditorWindow', 'height', type='int'),
@@ -154,6 +154,7 @@ class EditorWindow(object):
         text.bind('<MouseWheel>', self.mousescroll)
         text.bind('<Button-4>', self.mousescroll)
         text.bind('<Button-5>', self.mousescroll)
+        text.bind('<Configure>', self.handle_winconfig)
         text.bind("<<cut>>", self.cut)
         text.bind("<<copy>>", self.copy)
         text.bind("<<paste>>", self.paste)
@@ -211,6 +212,7 @@ class EditorWindow(object):
         text['font'] = idleConf.GetFont(self.root, 'main', 'EditorWindow')
         text.grid(row=1, column=1, sticky=NSEW)
         text.focus_set()
+        self.set_width()
 
         # usetabs true  -> literal tab characters are used by indent and
         #                  dedent cmds, possibly mixed with spaces if
@@ -338,6 +340,22 @@ class EditorWindow(object):
         else:
             self.update_menu_state('options', '*Line Numbers', 'disabled')
 
+    def handle_winconfig(self, event=None):
+        self.set_width()
+
+    def set_width(self):
+        text = self.text
+        inner_padding = sum(map(text.tk.getint, [text.cget('border'),
+                                                 text.cget('padx')]))
+        pixel_width = text.winfo_width() - 2 * inner_padding
+
+        # 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
+        zero_char_width = \
+            Font(text, font=text.cget('font')).measure('0')
+        self.width = pixel_width // zero_char_width
+
     def _filename_to_unicode(self, filename):
         """Return filename as BMP unicode so displayable in Tk."""
         # Decode bytes to unicode.
@@ -830,6 +848,7 @@ class EditorWindow(object):
         # Finally, update the main text widget.
         new_font = idleConf.GetFont(self.root, 'main', 'EditorWindow')
         self.text['font'] = new_font
+        self.set_width()
 
     def RemoveKeybindings(self):
         "Remove the keybindings before they are changed."
index 4e3da030a3adce048ba52ebdf99b9305fc53bf06..1af2ce832845cdaca1a70e96adf4b2f1364e80cd 100644 (file)
@@ -82,18 +82,10 @@ class CountLinesTest(unittest.TestCase):
 
 class SqueezerTest(unittest.TestCase):
     """Tests for the Squeezer class."""
-    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.
-        editwin.__class__ = PyShell
+        editwin.width = 80
 
         if with_text_widget:
             editwin.root = get_test_tk_root(self)
@@ -107,7 +99,6 @@ class SqueezerTest(unittest.TestCase):
         if editor_window is None:
             editor_window = self.make_mock_editor_window()
         squeezer = Squeezer(editor_window)
-        squeezer.get_line_width = Mock(return_value=80)
         return squeezer
 
     def make_text_widget(self, root=None):
@@ -143,8 +134,8 @@ class SqueezerTest(unittest.TestCase):
                               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)
+                with patch.object(editwin, 'width', line_width):
+                    self.assertEqual(squeezer.count_lines(text), expected)
 
     def test_init(self):
         """Test the creation of Squeezer instances."""
@@ -294,7 +285,6 @@ class SqueezerTest(unittest.TestCase):
         """Test the reload() class-method."""
         editwin = self.make_mock_editor_window(with_text_widget=True)
         squeezer = self.make_squeezer_instance(editwin)
-        squeezer.load_font = Mock()
 
         orig_auto_squeeze_min_lines = squeezer.auto_squeeze_min_lines
 
@@ -307,7 +297,6 @@ class SqueezerTest(unittest.TestCase):
         Squeezer.reload()
         self.assertEqual(squeezer.auto_squeeze_min_lines,
                          new_auto_squeeze_min_lines)
-        squeezer.load_font.assert_called()
 
     def test_reload_no_squeezer_instances(self):
         """Test that Squeezer.reload() runs without any instances existing."""
index 032401f2abc738cb1ba3525b71682a2154074f5c..be1538a25fdedfc53281985398606dc427c67713 100644 (file)
@@ -15,10 +15,8 @@ output written to the standard error stream ("stderr"), such as exception
 messages and their tracebacks.
 """
 import re
-import weakref
 
 import tkinter as tk
-from tkinter.font import Font
 import tkinter.messagebox as tkMessageBox
 
 from idlelib.config import idleConf
@@ -203,8 +201,6 @@ 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."""
@@ -213,14 +209,6 @@ class Squeezer:
             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.
 
@@ -241,9 +229,6 @@ class 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 * (
@@ -298,24 +283,7 @@ class Squeezer:
 
         Tabs are considered tabwidth characters long.
         """
-        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')
+        return count_lines_with_wrapping(s, self.editwin.width)
 
     def squeeze_current_text_event(self, event):
         """squeeze-current-text event handler
diff --git a/Misc/NEWS.d/next/IDLE/2019-08-24-22-00-33.bpo-37929.jb7523.rst b/Misc/NEWS.d/next/IDLE/2019-08-24-22-00-33.bpo-37929.jb7523.rst
new file mode 100644 (file)
index 0000000..d627b2d
--- /dev/null
@@ -0,0 +1 @@
+IDLE Settings dialog now closes properly when there is no shell window.