Skip to content

Commit

Permalink
bpo-37929: IDLE: avoid Squeezer-related config dialog crashes (GH-15452)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
taleinat committed Aug 25, 2019
1 parent aef9ad8 commit d4b4c00
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 50 deletions.
25 changes: 22 additions & 3 deletions Lib/idlelib/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'),
Expand All @@ -154,6 +154,7 @@ def __init__(self, flist=None, filename=None, key=None, root=None):
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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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."
Expand Down
17 changes: 3 additions & 14 deletions Lib/idlelib/idle_test/test_squeezer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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):
Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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

Expand All @@ -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."""
Expand Down
34 changes: 1 addition & 33 deletions Lib/idlelib/squeezer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""
Expand All @@ -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.
Expand All @@ -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 * (
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
IDLE Settings dialog now closes properly when there is no shell window.

0 comments on commit d4b4c00

Please sign in to comment.