[Python-checkins] cpython: Issue #27621: Put query response validation error messages in query box
terry.reedy
python-checkins at python.org
Wed Aug 10 12:50:29 EDT 2016
https://hg.python.org/cpython/rev/f0e86b60de5f
changeset: 102606:f0e86b60de5f
user: Terry Jan Reedy <tjreedy at udel.edu>
date: Wed Aug 10 12:50:16 2016 -0400
summary:
Issue #27621: Put query response validation error messages in query box
instead of in separate massagebox. Redo tests to match.
Add Mac OSX refinements. Original patch by Mark Roseman.
files:
Lib/idlelib/idle_test/test_query.py | 235 ++++++---------
Lib/idlelib/query.py | 122 ++++---
2 files changed, 164 insertions(+), 193 deletions(-)
diff --git a/Lib/idlelib/idle_test/test_query.py b/Lib/idlelib/idle_test/test_query.py
--- a/Lib/idlelib/idle_test/test_query.py
+++ b/Lib/idlelib/idle_test/test_query.py
@@ -16,21 +16,9 @@
from tkinter import Tk
import unittest
from unittest import mock
-from idlelib.idle_test.mock_tk import Var, Mbox_func
+from idlelib.idle_test.mock_tk import Var
from idlelib import query
-# Mock entry.showerror messagebox so don't need click to continue
-# when entry_ok and path_ok methods call it to display errors.
-
-orig_showerror = query.showerror
-showerror = Mbox_func() # Instance has __call__ method.
-
-def setUpModule():
- query.showerror = showerror
-
-def tearDownModule():
- query.showerror = orig_showerror
-
# NON-GUI TESTS
@@ -42,59 +30,49 @@
entry_ok = query.Query.entry_ok
ok = query.Query.ok
cancel = query.Query.cancel
- # Add attributes needed for the tests.
+ # Add attributes and initialization needed for tests.
entry = Var()
- result = None
- destroyed = False
+ entry_error = {}
+ def __init__(self, dummy_entry):
+ self.entry.set(dummy_entry)
+ self.entry_error['text'] = ''
+ self.result = None
+ self.destroyed = False
+ def showerror(self, message):
+ self.entry_error['text'] = message
def destroy(self):
self.destroyed = True
- dialog = Dummy_Query()
-
- def setUp(self):
- showerror.title = None
- self.dialog.result = None
- self.dialog.destroyed = False
-
def test_entry_ok_blank(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set(' ')
- Equal(dialog.entry_ok(), None)
- Equal((dialog.result, dialog.destroyed), (None, False))
- Equal(showerror.title, 'Entry Error')
- self.assertIn('Blank', showerror.message)
+ dialog = self.Dummy_Query(' ')
+ self.assertEqual(dialog.entry_ok(), None)
+ self.assertEqual((dialog.result, dialog.destroyed), (None, False))
+ self.assertIn('blank line', dialog.entry_error['text'])
def test_entry_ok_good(self):
- dialog = self.dialog
+ dialog = self.Dummy_Query(' good ')
Equal = self.assertEqual
- dialog.entry.set(' good ')
Equal(dialog.entry_ok(), 'good')
Equal((dialog.result, dialog.destroyed), (None, False))
- Equal(showerror.title, None)
+ Equal(dialog.entry_error['text'], '')
def test_ok_blank(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set('')
+ dialog = self.Dummy_Query('')
dialog.entry.focus_set = mock.Mock()
- Equal(dialog.ok(), None)
+ self.assertEqual(dialog.ok(), None)
self.assertTrue(dialog.entry.focus_set.called)
del dialog.entry.focus_set
- Equal((dialog.result, dialog.destroyed), (None, False))
+ self.assertEqual((dialog.result, dialog.destroyed), (None, False))
def test_ok_good(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set('good')
- Equal(dialog.ok(), None)
- Equal((dialog.result, dialog.destroyed), ('good', True))
+ dialog = self.Dummy_Query('good')
+ self.assertEqual(dialog.ok(), None)
+ self.assertEqual((dialog.result, dialog.destroyed), ('good', True))
def test_cancel(self):
- dialog = self.dialog
- Equal = self.assertEqual
- Equal(self.dialog.cancel(), None)
- Equal((dialog.result, dialog.destroyed), (None, True))
+ dialog = self.Dummy_Query('does not matter')
+ self.assertEqual(dialog.cancel(), None)
+ self.assertEqual((dialog.result, dialog.destroyed), (None, True))
class SectionNameTest(unittest.TestCase):
@@ -104,42 +82,32 @@
entry_ok = query.SectionName.entry_ok # Function being tested.
used_names = ['used']
entry = Var()
-
- dialog = Dummy_SectionName()
-
- def setUp(self):
- showerror.title = None
+ entry_error = {}
+ def __init__(self, dummy_entry):
+ self.entry.set(dummy_entry)
+ self.entry_error['text'] = ''
+ def showerror(self, message):
+ self.entry_error['text'] = message
def test_blank_section_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set(' ')
- Equal(dialog.entry_ok(), None)
- Equal(showerror.title, 'Name Error')
- self.assertIn('No', showerror.message)
+ dialog = self.Dummy_SectionName(' ')
+ self.assertEqual(dialog.entry_ok(), None)
+ self.assertIn('no name', dialog.entry_error['text'])
def test_used_section_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set('used')
- Equal(self.dialog.entry_ok(), None)
- Equal(showerror.title, 'Name Error')
- self.assertIn('use', showerror.message)
+ dialog = self.Dummy_SectionName('used')
+ self.assertEqual(dialog.entry_ok(), None)
+ self.assertIn('use', dialog.entry_error['text'])
def test_long_section_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set('good'*8)
- Equal(self.dialog.entry_ok(), None)
- Equal(showerror.title, 'Name Error')
- self.assertIn('too long', showerror.message)
+ dialog = self.Dummy_SectionName('good'*8)
+ self.assertEqual(dialog.entry_ok(), None)
+ self.assertIn('longer than 30', dialog.entry_error['text'])
def test_good_section_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set(' good ')
- Equal(dialog.entry_ok(), 'good')
- Equal(showerror.title, None)
+ dialog = self.Dummy_SectionName(' good ')
+ self.assertEqual(dialog.entry_ok(), 'good')
+ self.assertEqual(dialog.entry_error['text'], '')
class ModuleNameTest(unittest.TestCase):
@@ -149,42 +117,32 @@
entry_ok = query.ModuleName.entry_ok # Function being tested.
text0 = ''
entry = Var()
-
- dialog = Dummy_ModuleName()
-
- def setUp(self):
- showerror.title = None
+ entry_error = {}
+ def __init__(self, dummy_entry):
+ self.entry.set(dummy_entry)
+ self.entry_error['text'] = ''
+ def showerror(self, message):
+ self.entry_error['text'] = message
def test_blank_module_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set(' ')
- Equal(dialog.entry_ok(), None)
- Equal(showerror.title, 'Name Error')
- self.assertIn('No', showerror.message)
+ dialog = self.Dummy_ModuleName(' ')
+ self.assertEqual(dialog.entry_ok(), None)
+ self.assertIn('no name', dialog.entry_error['text'])
def test_bogus_module_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set('__name_xyz123_should_not_exist__')
- Equal(self.dialog.entry_ok(), None)
- Equal(showerror.title, 'Import Error')
- self.assertIn('not found', showerror.message)
+ dialog = self.Dummy_ModuleName('__name_xyz123_should_not_exist__')
+ self.assertEqual(dialog.entry_ok(), None)
+ self.assertIn('not found', dialog.entry_error['text'])
def test_c_source_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set('itertools')
- Equal(self.dialog.entry_ok(), None)
- Equal(showerror.title, 'Import Error')
- self.assertIn('source-based', showerror.message)
+ dialog = self.Dummy_ModuleName('itertools')
+ self.assertEqual(dialog.entry_ok(), None)
+ self.assertIn('source-based', dialog.entry_error['text'])
def test_good_module_name(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.entry.set('idlelib')
+ dialog = self.Dummy_ModuleName('idlelib')
self.assertTrue(dialog.entry_ok().endswith('__init__.py'))
- Equal(showerror.title, None)
+ self.assertEqual(dialog.entry_error['text'], '')
# 3 HelpSource test classes each test one function.
@@ -198,13 +156,13 @@
browse_file = query.HelpSource.browse_file
pathvar = Var()
- dialog = Dummy_HelpSource()
-
def test_file_replaces_path(self):
- # Path is widget entry, file is file dialog return.
- dialog = self.dialog
+ dialog = self.Dummy_HelpSource()
+ # Path is widget entry, either '' or something.
+ # Func return is file dialog return, either '' or something.
+ # Func return should override widget entry.
+ # We need all 4 combination to test all (most) code paths.
for path, func, result in (
- # We need all combination to test all (most) code paths.
('', lambda a,b,c:'', ''),
('', lambda a,b,c: __file__, __file__),
('htest', lambda a,b,c:'', 'htest'),
@@ -217,78 +175,72 @@
class HelpsourcePathokTest(unittest.TestCase):
- "Test path_ok method of ModuleName subclass of Query."
+ "Test path_ok method of HelpSource subclass of Query."
class Dummy_HelpSource:
path_ok = query.HelpSource.path_ok
path = Var()
-
- dialog = Dummy_HelpSource()
+ path_error = {}
+ def __init__(self, dummy_path):
+ self.path.set(dummy_path)
+ self.path_error['text'] = ''
+ def showerror(self, message, widget=None):
+ self.path_error['text'] = message
@classmethod
def tearDownClass(cls):
query.platform = orig_platform
- def setUp(self):
- showerror.title = None
-
def test_path_ok_blank(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.path.set(' ')
- Equal(dialog.path_ok(), None)
- Equal(showerror.title, 'File Path Error')
- self.assertIn('No help', showerror.message)
+ dialog = self.Dummy_HelpSource(' ')
+ self.assertEqual(dialog.path_ok(), None)
+ self.assertIn('no help file', dialog.path_error['text'])
def test_path_ok_bad(self):
- dialog = self.dialog
- Equal = self.assertEqual
- dialog.path.set(__file__ + 'bad-bad-bad')
- Equal(dialog.path_ok(), None)
- Equal(showerror.title, 'File Path Error')
- self.assertIn('not exist', showerror.message)
+ dialog = self.Dummy_HelpSource(__file__ + 'bad-bad-bad')
+ self.assertEqual(dialog.path_ok(), None)
+ self.assertIn('not exist', dialog.path_error['text'])
def test_path_ok_web(self):
- dialog = self.dialog
+ dialog = self.Dummy_HelpSource('')
Equal = self.assertEqual
for url in 'www.py.org', 'http://py.org':
with self.subTest():
dialog.path.set(url)
- Equal(dialog.path_ok(), url)
- Equal(showerror.title, None)
+ self.assertEqual(dialog.path_ok(), url)
+ self.assertEqual(dialog.path_error['text'], '')
def test_path_ok_file(self):
- dialog = self.dialog
- Equal = self.assertEqual
+ dialog = self.Dummy_HelpSource('')
for platform, prefix in ('darwin', 'file://'), ('other', ''):
with self.subTest():
query.platform = platform
dialog.path.set(__file__)
- Equal(dialog.path_ok(), prefix + __file__)
- Equal(showerror.title, None)
+ self.assertEqual(dialog.path_ok(), prefix + __file__)
+ self.assertEqual(dialog.path_error['text'], '')
class HelpsourceEntryokTest(unittest.TestCase):
- "Test entry_ok method of ModuleName subclass of Query."
+ "Test entry_ok method of HelpSource subclass of Query."
class Dummy_HelpSource:
entry_ok = query.HelpSource.entry_ok
+ entry_error = {}
+ path_error = {}
def item_ok(self):
return self.name
def path_ok(self):
return self.path
- dialog = Dummy_HelpSource()
-
def test_entry_ok_helpsource(self):
- dialog = self.dialog
+ dialog = self.Dummy_HelpSource()
for name, path, result in ((None, None, None),
(None, 'doc.txt', None),
('doc', None, None),
('doc', 'doc.txt', ('doc', 'doc.txt'))):
with self.subTest():
dialog.name, dialog.path = name, path
- self.assertEqual(self.dialog.entry_ok(), result)
+ self.assertEqual(dialog.entry_ok(), result)
# GUI TESTS
@@ -344,10 +296,10 @@
root = Tk()
dialog = query.SectionName(root, 'T', 't', {'abc'}, _utest=True)
Equal = self.assertEqual
- Equal(dialog.used_names, {'abc'})
+ self.assertEqual(dialog.used_names, {'abc'})
dialog.entry.insert(0, 'okay')
dialog.button_ok.invoke()
- Equal(dialog.result, 'okay')
+ self.assertEqual(dialog.result, 'okay')
del dialog
root.destroy()
del root
@@ -362,9 +314,8 @@
def test_click_module_name(self):
root = Tk()
dialog = query.ModuleName(root, 'T', 't', 'idlelib', _utest=True)
- Equal = self.assertEqual
- Equal(dialog.text0, 'idlelib')
- Equal(dialog.entry.get(), 'idlelib')
+ self.assertEqual(dialog.text0, 'idlelib')
+ self.assertEqual(dialog.entry.get(), 'idlelib')
dialog.button_ok.invoke()
self.assertTrue(dialog.result.endswith('__init__.py'))
del dialog
diff --git a/Lib/idlelib/query.py b/Lib/idlelib/query.py
--- a/Lib/idlelib/query.py
+++ b/Lib/idlelib/query.py
@@ -10,6 +10,8 @@
Subclass SectionName gets a name for a new config file section.
Configdialog uses it for new highlight theme and keybinding set names.
+Subclass ModuleName gets a name for File => Open Module.
+Subclass HelpSource gets menu item and path for additions to Help menu.
"""
# Query and Section name result from splitting GetCfgSectionNameDialog
# of configSectionNameDialog.py (temporarily config_sec.py) into
@@ -21,10 +23,10 @@
import importlib
import os
from sys import executable, platform # Platform is set for one test.
-from tkinter import Toplevel, StringVar
+from tkinter import Toplevel, StringVar, W, E, N, S
from tkinter import filedialog
-from tkinter.messagebox import showerror
from tkinter.ttk import Frame, Button, Entry, Label
+from tkinter.font import Font
class Query(Toplevel):
"""Base class for getting verified answer from a user.
@@ -47,18 +49,26 @@
"""
Toplevel.__init__(self, parent)
self.withdraw() # Hide while configuring, especially geometry.
- self.configure(borderwidth=5)
- self.resizable(height=False, width=False)
+ self.parent = parent
self.title(title)
- self.transient(parent)
- self.grab_set()
- self.bind('<Key-Return>', self.ok)
- self.bind('<Key-Escape>', self.cancel)
- self.protocol("WM_DELETE_WINDOW", self.cancel)
- self.parent = parent
self.message = message
self.text0 = text0
self.used_names = used_names
+ self.transient(parent)
+ self.grab_set()
+ windowingsystem = self.tk.call('tk', 'windowingsystem')
+ if windowingsystem == 'aqua':
+ try:
+ self.tk.call('::tk::unsupported::MacWindowStyle', 'style',
+ self._w, 'moveableModal', '')
+ except:
+ pass
+ self.bind("<Command-.>", self.cancel)
+ self.bind('<Key-Escape>', self.cancel)
+ self.protocol("WM_DELETE_WINDOW", self.cancel)
+ self.bind('<Key-Return>', self.ok)
+ self.bind("<KP_Enter>", self.ok)
+ self.resizable(height=False, width=False)
self.create_widgets()
self.update_idletasks() # Needed here for winfo_reqwidth below.
self.geometry( # Center dialog over parent (or below htest box).
@@ -75,32 +85,42 @@
def create_widgets(self): # Call from override, if any.
# Bind to self widgets needed for entry_ok or unittest.
- self.frame = frame = Frame(self, borderwidth=2, relief='sunken', )
+ self.frame = frame = Frame(self, padding=10)
+ frame.grid(column=0, row=0, sticky='news')
+ frame.grid_columnconfigure(0, weight=1)
+
entrylabel = Label(frame, anchor='w', justify='left',
text=self.message)
self.entryvar = StringVar(self, self.text0)
self.entry = Entry(frame, width=30, textvariable=self.entryvar)
self.entry.focus_set()
+ self.error_font = Font(name='TkCaptionFont',
+ exists=True, root=self.parent)
+ self.entry_error = Label(frame, text=' ', foreground='red',
+ font=self.error_font)
+ self.button_ok = Button(
+ frame, text='OK', default='active', command=self.ok)
+ self.button_cancel = Button(
+ frame, text='Cancel', command=self.cancel)
- buttons = Frame(self)
- self.button_ok = Button(buttons, text='Ok', default='active',
- width=8, command=self.ok)
- self.button_cancel = Button(buttons, text='Cancel',
- width=8, command=self.cancel)
+ entrylabel.grid(column=0, row=0, columnspan=3, padx=5, sticky=W)
+ self.entry.grid(column=0, row=1, columnspan=3, padx=5, sticky=W+E,
+ pady=[10,0])
+ self.entry_error.grid(column=0, row=2, columnspan=3, padx=5,
+ sticky=W+E)
+ self.button_ok.grid(column=1, row=99, padx=5)
+ self.button_cancel.grid(column=2, row=99, padx=5)
- frame.pack(side='top', expand=True, fill='both')
- entrylabel.pack(padx=5, pady=5)
- self.entry.pack(padx=5, pady=5)
- buttons.pack(side='bottom')
- self.button_ok.pack(side='left', padx=5)
- self.button_cancel.pack(side='right', padx=5)
+ def showerror(self, message, widget=None):
+ #self.bell(displayof=self)
+ (widget or self.entry_error)['text'] = 'ERROR: ' + message
def entry_ok(self): # Example: usually replace.
"Return non-blank entry or None."
+ self.entry_error['text'] = ''
entry = self.entry.get().strip()
if not entry:
- showerror(title='Entry Error',
- message='Blank line.', parent=self)
+ self.showerror('blank line.')
return None
return entry
@@ -134,19 +154,16 @@
def entry_ok(self):
"Return sensible ConfigParser section name or None."
+ self.entry_error['text'] = ''
name = self.entry.get().strip()
if not name:
- showerror(title='Name Error',
- message='No name specified.', parent=self)
+ self.showerror('no name specified.')
return None
elif len(name)>30:
- showerror(title='Name Error',
- message='Name too long. It should be no more than '+
- '30 characters.', parent=self)
+ self.showerror('name is longer than 30 characters.')
return None
elif name in self.used_names:
- showerror(title='Name Error',
- message='This name is already in use.', parent=self)
+ self.showerror('name is already in use.')
return None
return name
@@ -162,30 +179,27 @@
def entry_ok(self):
"Return entered module name as file path or None."
+ self.entry_error['text'] = ''
name = self.entry.get().strip()
if not name:
- showerror(title='Name Error',
- message='No name specified.', parent=self)
+ self.showerror('no name specified.')
return None
# XXX Ought to insert current file's directory in front of path.
try:
spec = importlib.util.find_spec(name)
except (ValueError, ImportError) as msg:
- showerror("Import Error", str(msg), parent=self)
+ self.showerror(str(msg))
return None
if spec is None:
- showerror("Import Error", "module not found",
- parent=self)
+ self.showerror("module not found")
return None
if not isinstance(spec.loader, importlib.abc.SourceLoader):
- showerror("Import Error", "not a source-based module",
- parent=self)
+ self.showerror("not a source-based module")
return None
try:
file_path = spec.loader.get_filename(name)
except AttributeError:
- showerror("Import Error",
- "loader does not support get_filename",
+ self.showerror("loader does not support get_filename",
parent=self)
return None
return file_path
@@ -204,8 +218,9 @@
"""
self.filepath = filepath
message = 'Name for item on Help menu:'
- super().__init__(parent, title, message, text0=menuitem,
- used_names=used_names, _htest=_htest, _utest=_utest)
+ super().__init__(
+ parent, title, message, text0=menuitem,
+ used_names=used_names, _htest=_htest, _utest=_utest)
def create_widgets(self):
super().create_widgets()
@@ -216,10 +231,16 @@
self.path = Entry(frame, textvariable=self.pathvar, width=40)
browse = Button(frame, text='Browse', width=8,
command=self.browse_file)
+ self.path_error = Label(frame, text=' ', foreground='red',
+ font=self.error_font)
- pathlabel.pack(anchor='w', padx=5, pady=3)
- self.path.pack(anchor='w', padx=5, pady=3)
- browse.pack(pady=3)
+ pathlabel.grid(column=0, row=10, columnspan=3, padx=5, pady=[10,0],
+ sticky=W)
+ self.path.grid(column=0, row=11, columnspan=2, padx=5, sticky=W+E,
+ pady=[10,0])
+ browse.grid(column=2, row=11, padx=5, sticky=W+S)
+ self.path_error.grid(column=0, row=12, columnspan=3, padx=5,
+ sticky=W+E)
def askfilename(self, filetypes, initdir, initfile): # htest #
# Extracted from browse_file so can mock for unittests.
@@ -256,17 +277,14 @@
"Simple validity check for menu file path"
path = self.path.get().strip()
if not path: #no path specified
- showerror(title='File Path Error',
- message='No help file path specified.',
- parent=self)
+ self.showerror('no help file path specified.', self.path_error)
return None
elif not path.startswith(('www.', 'http')):
if path[:5] == 'file:':
path = path[5:]
if not os.path.exists(path):
- showerror(title='File Path Error',
- message='Help file path does not exist.',
- parent=self)
+ self.showerror('help file path does not exist.',
+ self.path_error)
return None
if platform == 'darwin': # for Mac Safari
path = "file://" + path
@@ -274,6 +292,8 @@
def entry_ok(self):
"Return apparently valid (name, path) or None"
+ self.entry_error['text'] = ''
+ self.path_error['text'] = ''
name = self.item_ok()
path = self.path_ok()
return None if name is None or path is None else (name, path)
--
Repository URL: https://hg.python.org/cpython
More information about the Python-checkins
mailing list