Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-63075: IDLE editor - Auto insertion of the closers #3520

Open
wants to merge 75 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
5c57faf
parenmatch highlighting options
wohlganger Jun 29, 2017
d49170f
fixed whitespace issue
wohlganger Jun 29, 2017
8c3a966
only select from highlighting options
wohlganger Jun 30, 2017
757bb38
fixed whitespace
wohlganger Jun 30, 2017
d7e136c
whitespace
wohlganger Jun 30, 2017
b178af5
Merge remote-tracking branch 'upstream/master'
wohlganger Jul 10, 2017
2adebb9
#27099 - turn builtin extensions to reg modules
wohlganger Jul 10, 2017
86c3d4c
whitespace + fix I thought I already committed.
wohlganger Jul 10, 2017
1c246fc
warning fixes, removed old config entries
wohlganger Jul 11, 2017
0418ad0
calltip append doc fix, default paren-fore fix, help update
wohlganger Jul 11, 2017
097a0d8
fix calltips
wohlganger Jul 12, 2017
4232e77
removed circular import
wohlganger Jul 12, 2017
a7b29f5
fix warnings due to missing default config options in idlelib.config-…
wohlganger Jul 12, 2017
e30b7b4
Merge branch 'master' into master
wohlganger Jul 17, 2017
252672b
added format_pmaxw to config-main
wohlganger Jul 19, 2017
999b573
Merge remote-tracking branch 'upstream/master'
wohlganger Jul 19, 2017
1a74591
replaced help.html edit with idle.rst edit
wohlganger Jul 20, 2017
3501c45
Merge branch 'master' into master
wohlganger Jul 24, 2017
232f711
Merge remote-tracking branch 'upstream/master'
wohlganger Aug 4, 2017
f095c53
added conflict resolved var descriptions to docstring in create_page_…
wohlganger Aug 4, 2017
a661bfa
strip trailing whitespace
wohlganger Aug 4, 2017
5c90a8f
Merge branch 'master' into master
wohlganger Aug 24, 2017
5a6a007
re-introduced highlight options.
wohlganger Aug 24, 2017
709b434
News blurb.
terryjreedy Aug 24, 2017
b9b1f9a
Reinstated zoomheight as an extension. Replaced autoexpand with zoomh…
wohlganger Aug 24, 2017
8b76ccb
fixed configdialog test (wrong cases), fixed whitespace
wohlganger Aug 24, 2017
b986c12
update idle.rst: zoomheight is an extension.
wohlganger Aug 24, 2017
f86acaa
more fixes for zoomheight - trying to minimize changes from master
wohlganger Aug 24, 2017
5805462
zoomheight config fix, zoomheight should be fully back to the way it …
wohlganger Aug 24, 2017
165b548
whitespace
wohlganger Aug 24, 2017
aee5d02
fixed force-open completions missing from config-keys and config. Fix…
wohlganger Aug 24, 2017
cb161bd
Changes made per terryjreedy request.
wohlganger Aug 25, 2017
dc09a2b
re-added autocomplete wait and format paragraph max width options to …
wohlganger Aug 25, 2017
100463e
Merge branch 'master' into master
wohlganger Aug 28, 2017
3fbaa90
Merge branch 'master' into master
wohlganger Aug 30, 2017
60ff82f
changes per terryjreedy
wohlganger Aug 30, 2017
e3f6f16
fix warnings, errors, bugs
wohlganger Aug 30, 2017
a253f0c
whitespace
wohlganger Aug 30, 2017
fe8eb7a
bugfix
wohlganger Aug 30, 2017
caa7774
Make changes need to get IDLE to run
terryjreedy Aug 31, 2017
c29184e
Edit extension/feature files, making existing tests pass.
terryjreedy Aug 31, 2017
c62f726
Redo feature bindings.
terryjreedy Aug 31, 2017
d0e009f
First draft of dummy extension.
terryjreedy Aug 31, 2017
9e889c2
Fix that eliminate startup error and all but 2 test_idle errors.
terryjreedy Aug 31, 2017
6afdea3
Move widgets, load widgets, add tests, make previous tests pass.
terryjreedy Sep 2, 2017
7de27db
Add format-paragraph to core keys.
terryjreedy Sep 5, 2017
a2e76a7
Merge remote-tracking branch 'origin/master' into pr_2494
terryjreedy Sep 5, 2017
2265b53
Fix autocomplete and long lines.
terryjreedy Sep 5, 2017
03eed24
Put fixed-key event-adds in EditorWindow.__init__ for now.
terryjreedy Sep 9, 2017
207b0ca
Don't warn if new core keys not in user key config.
terryjreedy Sep 9, 2017
00acd22
Make code context inoperative in outwin, shell.
terryjreedy Sep 9, 2017
2d2bfbd
Change new bindings for MacOsx.
terryjreedy Sep 9, 2017
fe76cd3
Disable test dependent on extension Alt keys.
terryjreedy Sep 9, 2017
d2966e0
Minor edits.
terryjreedy Sep 9, 2017
6845bb0
File out news blurb explaining effect for users.
terryjreedy Sep 10, 2017
b7afebb
Don't compute ('') during integer input in entry box.
terryjreedy Sep 10, 2017
b1a4101
Merge remote-tracking branch 'upstream/master'
wohlganger Sep 12, 2017
7a53366
fix conflict
wohlganger Sep 12, 2017
d3b6848
fix conflict
wohlganger Sep 12, 2017
eb58d39
fix conflict, match upstream
wohlganger Sep 12, 2017
119bbfc
match upstream
wohlganger Sep 12, 2017
814efbe
Auto-insert-parens extension
wohlganger Sep 12, 2017
7f596b9
removed unused old options
wohlganger Sep 12, 2017
26323e2
Changes made per csabella request.
wohlganger Sep 13, 2017
7862d8a
fix stupid mistake
wohlganger Sep 13, 2017
2f19ca8
added mutual delete / backspace. minor fixes
wohlganger Sep 13, 2017
6d2fa26
PEP8 fixes.
wohlganger Sep 13, 2017
15f8d82
replace type option of bool with 'bool'
wohlganger Sep 13, 2017
d141abc
Merge branch 'master' into 18875-Auto-insert-parens
wohlganger Sep 15, 2017
caa9240
whitespace
wohlganger Sep 15, 2017
21f4482
whitespace fixed with reindent.py
wohlganger Sep 15, 2017
25ba0f8
trying to fix Tk for travis testing parenclose
wohlganger Sep 15, 2017
a4cd0e3
Set parenclose test as gui required
wohlganger Sep 18, 2017
cffa9d6
Added option to distinguish between mutual delete and mutual backspace
wohlganger Sep 20, 2017
9aaa395
Update ParenClose.py
terryjreedy Apr 6, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions Lib/idlelib/ParenClose.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add ParenClose to parenmatch.py.

Parens and Ticks Closing Extension

When you hit left paren or tick,
automatically creates the closing paren or tick.

Add to the end of config-extensions.def :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Config options will be added to config-general.def. The general tab already needs to be split for another issue. This issue, and a complete patch, will depend on how we do that, and then how to lay out the option buttons.


[ParenClose]
enable = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete.

paren_close = True
tick_close = True
skip_closures = True
mutual_delete = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think defaults should be 'off', as in no change from current behavior. I am not sure what latter 2 mean. See below.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the specification of the new feature by adding action on deletion should have been discussed on the issue. (This applies generally to changing specs.) Notepad++ does do anything special on deletion. Binding the deletion keys here prevents binding them elsewhere, and I have an idea of where it might be needed, as opposed to optional. I also think sometimes delete more and sometimes not is confusing. The deletion part of 2f19ca8 and anything later should be reverted unless and until there is a clear agreement on adding this.

[ParenClose_cfgBindings]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete section. Not needed for hard-coded features.

p-open = <Key-parenleft> <Key-bracketleft> <Key-braceleft>
t-open = <Key-quotedbl> <Key-quoteright>
p-close = <Key-parenright> <Key-bracketright> <Key-braceright>
terryjreedy marked this conversation as resolved.
Show resolved Hide resolved
back = <Key-BackSpace>
delete = <Key-Delete>
"""

from idlelib.config import idleConf


class ParenClose:
'''
When loaded as an extension to IDLE, and paren_close is True, the symbols
([{ will have their closures printed after them and the insertion cursor
moved between the two. The same is true for tick closures and the symbols
' and ". When \'\'\' or """ are typed and tick_close is True, it will also
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will consider rewording docstring, but this is secondary. Why are triples single quotes escaped?

produce the closing symbols. If skip_closures is True, then when a closure
symbol is typed and the same one is to the right of it, that symbols is
deleted before the new one is typed, effectively skipping over the closure.
If the cursor is between two closures and mutual_delete or mutual_backspace
is True, when the respective command is given, both closures are deleted.
'''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand these.


closers = {'(': ')', '[': ']', '{': '}', "'": "'", '"': '"'}

def __init__(self, editwin=None):
# Setting default to none makes testing easier.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I an not sure that this will work for a non-extension feature.

if editwin:
self.text = editwin.text
else:
self.text = None
self.paren_close = idleConf.GetOption(
'extensions', 'ParenClose', 'paren_close',
type='bool', default=True)
self.tick_close = idleConf.GetOption(
'extensions', 'ParenClose', 'tick_close',
type='bool', default=True)
self.skip_closures = idleConf.GetOption(
'extensions', 'ParenClose', 'skip_closures',
type='bool', default=True)
self.mutual_delete = idleConf.GetOption(
'extensions', 'ParenClose', 'mutual_delete',
type='bool', default=True)
self.mutual_backspace = idleConf.GetOption(
'extensions', 'ParenClose', 'mutual_backspace',
type='bool', default=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting the options should be done in a def reload(cls), the same as with ParenMatch.
ParenClose.reload() must be added after ParenMatch.reload() at the bottom of the file.
A call to ParenClose.reload must be added to configdialog wherever ParenMatch.reload is called, to make user changes take immediate effect.


def delcheck(self, pos):
symbol1 = self.text.get(pos + '-1c', pos)
symbol2 = self.text.get(pos, pos + '+1c')
return (symbol1 in self.closers
and self.closers[symbol1] == symbol2)

def back_event(self, event):
if self.mutual_backspace:
pos = self.text.index('insert')
if self.delcheck(pos):
self.text.delete(pos, pos + '+1c')

def delete_event(self, event):
if self.mutual_delete:
pos = self.text.index('insert')
if self.delcheck(pos):
self.text.delete(pos + '-1c', pos)

def p_open_event(self, event):
if self.paren_close:
pos = self.text.index('insert')
self.text.insert(pos, self.closers[event.char])
self.text.mark_set('insert', pos)

def p_close_event(self, event):
pos = self.text.index('insert')
if self.skip_closures \
and self.text.get(pos, pos + ' +1c') == event.char:
self.text.delete(pos, pos + '+1c')
return True
return False

def t_open_event(self, event):
if self.tick_close:
pos = self.text.index('insert')
if self.text.get(pos + ' -2c', pos) == event.char * 2 \
and self.text.get(pos, pos + ' +1c') != event.char:
# Instead of one tick, add three ticks if there are two;
# user wants to make docstring or multiline.
self.text.insert(pos, event.char * 3)
self.text.mark_set('insert', pos)
elif not self.p_close_event(event):
self.p_open_event(event)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not yet looked at the instance method code.



if __name__ == '__main__':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already in parenmatch.

import unittest
unittest.main('idlelib.idle_test.test_parenclose', verbosity=2)
14 changes: 14 additions & 0 deletions Lib/idlelib/config-extensions.def
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,17 @@ z-text= Z
z-in= <Control-Shift-KeyRelease-Insert>
[ZzDummy_bindings]
z-out= <Control-Shift-KeyRelease-Delete>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said above, put in config-general.

[ParenClose]
enable = True
paren_close = True
tick_close = True
skip_closures = True
mutual_delete = True
mutual_backspace = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the configuration options should be discussed more on the issue tracker.

[ParenClose_bindings]
p-open = <Key-parenleft> <Key-bracketleft> <Key-braceleft>
t-open = <Key-quotedbl> <Key-quoteright>
p-close = <Key-parenright> <Key-bracketright> <Key-braceright>
back = <Key-BackSpace>
delete = <Key-Delete>
Copy link
Member

@terryjreedy terryjreedy Apr 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, non-configurable bindings are not needed here. On the other hand, the bindings will have to be added elsewhere. Currently, these are done in EditorWindow.init.

In the future, I want to think about and maybe try putting feature bindings in the feature class. Also, I believe they could be class rather than instance bindings, as they are the same for all instances.

EDIT: Key bindings could, maybe should, be done in a loop iterating through (key, event_handler) pairs, and in a separate method.

14 changes: 7 additions & 7 deletions Lib/idlelib/idle_test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,21 +439,21 @@ def test_get_extensions(self):
''')
eq = self.assertEqual
iGE = idleConf.GetExtensions
eq(iGE(shell_only=True), [])
eq(iGE(), ['ZzDummy'])
eq(iGE(editor_only=True), ['ZzDummy'])
eq(iGE(active_only=False), ['ZzDummy', 'DISABLE'])
eq(iGE(active_only=False, editor_only=True), ['ZzDummy', 'DISABLE'])
eq(iGE(shell_only=True), ['ParenClose'])
eq(iGE(), ['ZzDummy', 'ParenClose'])
eq(iGE(editor_only=True), ['ZzDummy', 'ParenClose'])
eq(iGE(active_only=False), ['ZzDummy', 'ParenClose', 'DISABLE'])
eq(iGE(active_only=False, editor_only=True), ['ZzDummy', 'ParenClose', 'DISABLE'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that all these changes to test_config only apply to extensions, and are therefore not needed.

userextn.remove_section('ZzDummy')
userextn.remove_section('DISABLE')


def test_remove_key_bind_names(self):
conf = self.mock_config()

self.assertCountEqual(
conf.RemoveKeyBindNames(conf.GetSectionList('default', 'extensions')),
['AutoComplete', 'CodeContext', 'FormatParagraph', 'ParenMatch','ZzDummy'])
['AutoComplete', 'CodeContext', 'FormatParagraph', 'ParenClose',
'ParenMatch','ZzDummy'])

def test_get_extn_name_for_event(self):
userextn.read_string('''
Expand Down
97 changes: 97 additions & 0 deletions Lib/idlelib/idle_test/test_parenclose.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
'''Test idlelib.parenclose.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add MockEvent and ParenCloseTest to test_parenmatch. I have not reviewed in detail, but have a couple of comments.

'''
from idlelib.ParenClose import ParenClose
import unittest
# Can't use mock text; it doesn't handle positioning correctly.
from tkinter import Text
from test.support import requires
requires('gui')

class MockEvent(object):
def __init__(self, char):
self.char = char


class ParenCloseTest(unittest.TestCase):
def test_parenClose(self):
p = ParenClose()
p.text = Text()
p_open = p.p_open_event
p_close = p.p_close_event
t_open = p.t_open_event
back = p.back_event
delete = p.delete_event
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like testing all 5 methods in one method. On the other hand, I don't like repeating code, and using subtests allows multiple failures in one method. A helper function might allow separate test functions without duplication, but this is a lower priority than getting good coverage to start with.

for paren_close_set, tick_close_set, skip_closures_set, mutual_b_set, \
mutual_d_set, insert, func, pos, char, result, posend in [
(1, 1, 1, 1, 1, 'def abuse', p_open, '1.9', '(',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for test_tuple in [<list of tuples>] would allow test_tuple to be passed to a helper function.

'def abuse()', '1.10'),
(1, 1, 1, 1, 1, 'spam = ', p_open, '1.7', '[',
'spam = []', '1.8'),
(1, 1, 1, 1, 1, 'eggs = ', p_open, '1.7', '{',
'eggs = {}', '1.8'),
(1, 1, 1, 1, 1, 'def abuse2()', p_close, '1.11', ')',
'def abuse2()', '1.12'),
(1, 1, 1, 1, 1, 'spam2 = []', p_close, '1.9', ']',
'spam2 = []', '1.10'),
(1, 1, 1, 1, 1, 'eggs2 = {}', p_close, '1.9', '}',
'eggs2 = {}', '1.10'),
(1, 1, 1, 1, 1, "color = ", t_open, '1.8', "'",
"color = ''", '1.9'),
(1, 1, 1, 1, 1, "velocity = ", t_open, '1.11', '"',
'velocity = ""', '1.12'),
(1, 1, 1, 1, 1, "more_spam = ''", t_open, '1.14', "'",
"more_spam = ''''''", '1.15'),
(1, 1, 1, 1, 1, 'more_eggs = ""', t_open, '1.14', '"',
'more_eggs = """"""', '1.15'),
(1, 1, 1, 1, 1, "more_spam2 = ''''''", t_open, '1.16', "'",
"more_spam2 = ''''''", '1.17'),
(1, 1, 1, 1, 1, 'more_eggs2 = """"""', t_open, '1.16', '"',
'more_eggs2 = """"""', '1.17'),
(0, 1, 1, 1, 1, 'no_spam = ', p_open, '1.10', '(',
'no_spam = (', '1.11'),
(1, 0, 1, 1, 1, 'no_velocity = ', t_open, '1.14', "'",
"no_velocity = '", '1.15'),
(1, 0, 1, 1, 1, 'no_more_eggs = ""', t_open, '1.17', '"',
'no_more_eggs = """', '1.18'),
(1, 1, 0, 1, 1, 'tinny = []', p_close, '1.9', ']',
'tinny = []]', '1.10'),
(1, 1, 0, 1, 1, 'gone = ""', t_open, '1.8', '"',
'gone = """"', '1.9'),
(0, 0, 0, 0, 1, "honk = []", back, '1.8', '',
"honk = []", '1.8'),
(0, 0, 0, 1, 0, "henk = ()", delete, '1.8', '',
"henk = ()", '1.8'),
(0, 0, 0, 1, 1, 'nurb = ""', back, '1.8', '',
'nurb = "', '1.8'),
(0, 0, 0, 1, 1, 'chiz = []', delete, '1.8', '',
'chiz = ]', '1.7'),
(0, 0, 0, 1, 1, 'pink = {}', back, '1.8', '',
'pink = {', '1.8')
]:
# The actual delete and backspace events do not occur in
# this test. We are checking to make sure everything is
# correct before they would delete or backspace.

# Reset text and self.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with self.subTest ... is needed to make asserts independent.

p.paren_close = paren_close_set
p.tick_close = tick_close_set
p.skip_closures = skip_closures_set
p.mutual_delete = mutual_d_set
p.mutual_backspace = mutual_b_set
p.text.delete('1.0', 'end')
# Write text, move to current position, write character.
p.text.insert('1.0', insert)
p.text.mark_set('insert', pos)
event = MockEvent(char)
func(event)
pos = p.text.index('insert')
p.text.insert(pos, char)
# Checking position and text at the same time
# makes it easier to spot where errors occur.
pos = p.text.index('insert')
actual = p.text.get('1.0', 'end')
self.assertTupleEqual((actual, pos), (result+'\n', posend))


if __name__ == '__main__':
unittest.main(verbosity=2)
Empty file modified Tools/ssl/multissltests.py
100755 → 100644
Empty file.