From 44344386172653a5ab49e58866f7c01de71b4d5f Mon Sep 17 00:00:00 2001 From: Tal Einat Date: Sun, 25 Aug 2019 08:52:58 +0300 Subject: [PATCH] bpo-37929: IDLE: avoid Squeezer-related config dialog crashes (GH-15452) 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. (cherry picked from commit d4b4c00b57d24f6ee2cf3a96213406bb09953df3) Co-authored-by: Tal Einat --- Lib/idlelib/editor.py | 25 ++++++++++++-- Lib/idlelib/idle_test/test_squeezer.py | 17 ++-------- Lib/idlelib/squeezer.py | 34 +------------------ .../2019-08-24-22-00-33.bpo-37929.jb7523.rst | 1 + 4 files changed, 27 insertions(+), 50 deletions(-) create mode 100644 Misc/NEWS.d/next/IDLE/2019-08-24-22-00-33.bpo-37929.jb7523.rst diff --git a/Lib/idlelib/editor.py b/Lib/idlelib/editor.py index 35027da76bce2f..793ed3afaed0ff 100644 --- a/Lib/idlelib/editor.py +++ b/Lib/idlelib/editor.py @@ -10,6 +10,7 @@ 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 @@ def __init__(self, flist=None, filename=None, key=None, root=None): 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 @@ def __init__(self, flist=None, filename=None, key=None, root=None): text.bind('', self.mousescroll) text.bind('', self.mousescroll) text.bind('', self.mousescroll) + text.bind('', self.handle_winconfig) text.bind("<>", self.cut) text.bind("<>", self.copy) text.bind("<>", self.paste) @@ -211,6 +212,7 @@ def __init__(self, flist=None, filename=None, key=None, root=None): 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 @@ def __init__(self, flist=None, filename=None, key=None, root=None): 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 @@ def ResetFont(self): # 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." diff --git a/Lib/idlelib/idle_test/test_squeezer.py b/Lib/idlelib/idle_test/test_squeezer.py index 4e3da030a3adce..1af2ce832845cd 100644 --- a/Lib/idlelib/idle_test/test_squeezer.py +++ b/Lib/idlelib/idle_test/test_squeezer.py @@ -82,18 +82,10 @@ def test_several_lines_different_lengths(self): 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 @@ def make_squeezer_instance(self, editor_window=None): 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 @@ def test_count_lines(self): 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 @@ def test_reload(self): """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 @@ def test_reload(self): 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.""" diff --git a/Lib/idlelib/squeezer.py b/Lib/idlelib/squeezer.py index 032401f2abc738..be1538a25fdedf 100644 --- a/Lib/idlelib/squeezer.py +++ b/Lib/idlelib/squeezer.py @@ -15,10 +15,8 @@ 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 @@ 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. @@ -241,9 +229,6 @@ def __init__(self, editwin): # 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 @@ def count_lines(self, s): 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 index 00000000000000..d627b2de2a7c64 --- /dev/null +++ b/Misc/NEWS.d/next/IDLE/2019-08-24-22-00-33.bpo-37929.jb7523.rst @@ -0,0 +1 @@ +IDLE Settings dialog now closes properly when there is no shell window.