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.
This commit is contained in:
parent
aef9ad82f7
commit
d4b4c00b57
|
@ -10,6 +10,7 @@ import traceback
|
||||||
import webbrowser
|
import webbrowser
|
||||||
|
|
||||||
from tkinter import *
|
from tkinter import *
|
||||||
|
from tkinter.font import Font
|
||||||
from tkinter.ttk import Scrollbar
|
from tkinter.ttk import Scrollbar
|
||||||
import tkinter.simpledialog as tkSimpleDialog
|
import tkinter.simpledialog as tkSimpleDialog
|
||||||
import tkinter.messagebox as tkMessageBox
|
import tkinter.messagebox as tkMessageBox
|
||||||
|
@ -120,14 +121,13 @@ class EditorWindow(object):
|
||||||
self.prompt_last_line = '' # Override in PyShell
|
self.prompt_last_line = '' # Override in PyShell
|
||||||
self.text_frame = text_frame = Frame(top)
|
self.text_frame = text_frame = Frame(top)
|
||||||
self.vbar = vbar = Scrollbar(text_frame, name='vbar')
|
self.vbar = vbar = Scrollbar(text_frame, name='vbar')
|
||||||
self.width = idleConf.GetOption('main', 'EditorWindow',
|
width = idleConf.GetOption('main', 'EditorWindow', 'width', type='int')
|
||||||
'width', type='int')
|
|
||||||
text_options = {
|
text_options = {
|
||||||
'name': 'text',
|
'name': 'text',
|
||||||
'padx': 5,
|
'padx': 5,
|
||||||
'wrap': 'none',
|
'wrap': 'none',
|
||||||
'highlightthickness': 0,
|
'highlightthickness': 0,
|
||||||
'width': self.width,
|
'width': width,
|
||||||
'tabstyle': 'wordprocessor', # new in 8.5
|
'tabstyle': 'wordprocessor', # new in 8.5
|
||||||
'height': idleConf.GetOption(
|
'height': idleConf.GetOption(
|
||||||
'main', 'EditorWindow', 'height', type='int'),
|
'main', 'EditorWindow', 'height', type='int'),
|
||||||
|
@ -154,6 +154,7 @@ class EditorWindow(object):
|
||||||
text.bind('<MouseWheel>', self.mousescroll)
|
text.bind('<MouseWheel>', self.mousescroll)
|
||||||
text.bind('<Button-4>', self.mousescroll)
|
text.bind('<Button-4>', self.mousescroll)
|
||||||
text.bind('<Button-5>', self.mousescroll)
|
text.bind('<Button-5>', self.mousescroll)
|
||||||
|
text.bind('<Configure>', self.handle_winconfig)
|
||||||
text.bind("<<cut>>", self.cut)
|
text.bind("<<cut>>", self.cut)
|
||||||
text.bind("<<copy>>", self.copy)
|
text.bind("<<copy>>", self.copy)
|
||||||
text.bind("<<paste>>", self.paste)
|
text.bind("<<paste>>", self.paste)
|
||||||
|
@ -211,6 +212,7 @@ class EditorWindow(object):
|
||||||
text['font'] = idleConf.GetFont(self.root, 'main', 'EditorWindow')
|
text['font'] = idleConf.GetFont(self.root, 'main', 'EditorWindow')
|
||||||
text.grid(row=1, column=1, sticky=NSEW)
|
text.grid(row=1, column=1, sticky=NSEW)
|
||||||
text.focus_set()
|
text.focus_set()
|
||||||
|
self.set_width()
|
||||||
|
|
||||||
# usetabs true -> literal tab characters are used by indent and
|
# usetabs true -> literal tab characters are used by indent and
|
||||||
# dedent cmds, possibly mixed with spaces if
|
# dedent cmds, possibly mixed with spaces if
|
||||||
|
@ -338,6 +340,22 @@ class EditorWindow(object):
|
||||||
else:
|
else:
|
||||||
self.update_menu_state('options', '*Line Numbers', 'disabled')
|
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):
|
def _filename_to_unicode(self, filename):
|
||||||
"""Return filename as BMP unicode so displayable in Tk."""
|
"""Return filename as BMP unicode so displayable in Tk."""
|
||||||
# Decode bytes to unicode.
|
# Decode bytes to unicode.
|
||||||
|
@ -830,6 +848,7 @@ class EditorWindow(object):
|
||||||
# Finally, update the main text widget.
|
# Finally, update the main text widget.
|
||||||
new_font = idleConf.GetFont(self.root, 'main', 'EditorWindow')
|
new_font = idleConf.GetFont(self.root, 'main', 'EditorWindow')
|
||||||
self.text['font'] = new_font
|
self.text['font'] = new_font
|
||||||
|
self.set_width()
|
||||||
|
|
||||||
def RemoveKeybindings(self):
|
def RemoveKeybindings(self):
|
||||||
"Remove the keybindings before they are changed."
|
"Remove the keybindings before they are changed."
|
||||||
|
|
|
@ -82,18 +82,10 @@ class CountLinesTest(unittest.TestCase):
|
||||||
|
|
||||||
class SqueezerTest(unittest.TestCase):
|
class SqueezerTest(unittest.TestCase):
|
||||||
"""Tests for the Squeezer class."""
|
"""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):
|
def make_mock_editor_window(self, with_text_widget=False):
|
||||||
"""Create a mock EditorWindow instance."""
|
"""Create a mock EditorWindow instance."""
|
||||||
editwin = NonCallableMagicMock()
|
editwin = NonCallableMagicMock()
|
||||||
# isinstance(editwin, PyShell) must be true for Squeezer to enable
|
editwin.width = 80
|
||||||
# auto-squeezing; in practice this will always be true.
|
|
||||||
editwin.__class__ = PyShell
|
|
||||||
|
|
||||||
if with_text_widget:
|
if with_text_widget:
|
||||||
editwin.root = get_test_tk_root(self)
|
editwin.root = get_test_tk_root(self)
|
||||||
|
@ -107,7 +99,6 @@ class SqueezerTest(unittest.TestCase):
|
||||||
if editor_window is None:
|
if editor_window is None:
|
||||||
editor_window = self.make_mock_editor_window()
|
editor_window = self.make_mock_editor_window()
|
||||||
squeezer = Squeezer(editor_window)
|
squeezer = Squeezer(editor_window)
|
||||||
squeezer.get_line_width = Mock(return_value=80)
|
|
||||||
return squeezer
|
return squeezer
|
||||||
|
|
||||||
def make_text_widget(self, root=None):
|
def make_text_widget(self, root=None):
|
||||||
|
@ -143,8 +134,8 @@ class SqueezerTest(unittest.TestCase):
|
||||||
line_width=line_width,
|
line_width=line_width,
|
||||||
expected=expected):
|
expected=expected):
|
||||||
text = eval(text_code)
|
text = eval(text_code)
|
||||||
squeezer.get_line_width.return_value = line_width
|
with patch.object(editwin, 'width', line_width):
|
||||||
self.assertEqual(squeezer.count_lines(text), expected)
|
self.assertEqual(squeezer.count_lines(text), expected)
|
||||||
|
|
||||||
def test_init(self):
|
def test_init(self):
|
||||||
"""Test the creation of Squeezer instances."""
|
"""Test the creation of Squeezer instances."""
|
||||||
|
@ -294,7 +285,6 @@ class SqueezerTest(unittest.TestCase):
|
||||||
"""Test the reload() class-method."""
|
"""Test the reload() class-method."""
|
||||||
editwin = self.make_mock_editor_window(with_text_widget=True)
|
editwin = self.make_mock_editor_window(with_text_widget=True)
|
||||||
squeezer = self.make_squeezer_instance(editwin)
|
squeezer = self.make_squeezer_instance(editwin)
|
||||||
squeezer.load_font = Mock()
|
|
||||||
|
|
||||||
orig_auto_squeeze_min_lines = squeezer.auto_squeeze_min_lines
|
orig_auto_squeeze_min_lines = squeezer.auto_squeeze_min_lines
|
||||||
|
|
||||||
|
@ -307,7 +297,6 @@ class SqueezerTest(unittest.TestCase):
|
||||||
Squeezer.reload()
|
Squeezer.reload()
|
||||||
self.assertEqual(squeezer.auto_squeeze_min_lines,
|
self.assertEqual(squeezer.auto_squeeze_min_lines,
|
||||||
new_auto_squeeze_min_lines)
|
new_auto_squeeze_min_lines)
|
||||||
squeezer.load_font.assert_called()
|
|
||||||
|
|
||||||
def test_reload_no_squeezer_instances(self):
|
def test_reload_no_squeezer_instances(self):
|
||||||
"""Test that Squeezer.reload() runs without any instances existing."""
|
"""Test that Squeezer.reload() runs without any instances existing."""
|
||||||
|
|
|
@ -15,10 +15,8 @@ output written to the standard error stream ("stderr"), such as exception
|
||||||
messages and their tracebacks.
|
messages and their tracebacks.
|
||||||
"""
|
"""
|
||||||
import re
|
import re
|
||||||
import weakref
|
|
||||||
|
|
||||||
import tkinter as tk
|
import tkinter as tk
|
||||||
from tkinter.font import Font
|
|
||||||
import tkinter.messagebox as tkMessageBox
|
import tkinter.messagebox as tkMessageBox
|
||||||
|
|
||||||
from idlelib.config import idleConf
|
from idlelib.config import idleConf
|
||||||
|
@ -203,8 +201,6 @@ class Squeezer:
|
||||||
This avoids IDLE's shell slowing down considerably, and even becoming
|
This avoids IDLE's shell slowing down considerably, and even becoming
|
||||||
completely unresponsive, when very long outputs are written.
|
completely unresponsive, when very long outputs are written.
|
||||||
"""
|
"""
|
||||||
_instance_weakref = None
|
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def reload(cls):
|
def reload(cls):
|
||||||
"""Load class variables from config."""
|
"""Load class variables from config."""
|
||||||
|
@ -213,14 +209,6 @@ class Squeezer:
|
||||||
type="int", default=50,
|
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):
|
def __init__(self, editwin):
|
||||||
"""Initialize settings for Squeezer.
|
"""Initialize settings for Squeezer.
|
||||||
|
|
||||||
|
@ -241,9 +229,6 @@ class Squeezer:
|
||||||
# however, needs to make such changes.
|
# however, needs to make such changes.
|
||||||
self.base_text = editwin.per.bottom
|
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;
|
# Twice the text widget's border width and internal padding;
|
||||||
# pre-calculated here for the get_line_width() method.
|
# pre-calculated here for the get_line_width() method.
|
||||||
self.window_width_delta = 2 * (
|
self.window_width_delta = 2 * (
|
||||||
|
@ -298,24 +283,7 @@ class Squeezer:
|
||||||
|
|
||||||
Tabs are considered tabwidth characters long.
|
Tabs are considered tabwidth characters long.
|
||||||
"""
|
"""
|
||||||
linewidth = self.get_line_width()
|
return count_lines_with_wrapping(s, self.editwin.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):
|
def squeeze_current_text_event(self, event):
|
||||||
"""squeeze-current-text event handler
|
"""squeeze-current-text event handler
|
||||||
|
|
|
@ -0,0 +1 @@
|
||||||
|
IDLE Settings dialog now closes properly when there is no shell window.
|
Loading…
Reference in New Issue