bpo-21261: IDLE - add completion of dict sting keys#15169

taleinat

This is a new implementation, created from scratch. I intentionally didn't base this on PR GH-1511 after reviewing it thoroughly.

Note that completion is intentionally limited to keys of type str and bytes only.

Additional changes made in this PR:

The Escape key now closes the completion window. [already done separately]
Completing a string or bytes dict key adds a closing square bracket.
Some of the auto-completion tests have been changed a bit to run without a GUI.
Significant refactoring and cleanup of some existing tests, with better use of mocking.
The fake Text widget used for tests has had its index handling greatly improved.
Added a very functional fake Listbox widget class.

https://bugs.python.org/issue21261

aeros

Tested using the IDLE shell from the PR branch. OS: Arch Linux 5.2.3.

Test for str
Before tab:

After tab:

Test for bytes
Before tab:

After tab:

Looks like it's working as intended, nicely done! I also tested to ensure that excessively_long_key and didn't auto-complete when using d[b'e and the same for d['t -> d['thanks taleinat'. I intentionally used a space instead of an underscore for the second one (even if it goes against naming convention) to ensure there weren't any issues there. LGTM.

taleinat

@aeros167, I've addressed all of the issues you brought up, plus your suggestions. I've also completed the implementation and added a ton of tests. If you have the time to give this another go, that would be great!

taleinat

@terryjreedy, in order to have many of the tests not require a GUI, I've expanded the existing mock_tk module quite a bit, most notably adding a rather full-featured Listbox class and expanding the index handling of the Text widget to be comprehensive. Please take a look at this!

I'm a bit worried about the long-term maintenance of this, though Tkinter changes little enough that it may be negligible.

If you prefer that I roll back these additions and just have the relevant tests require having a working GUI environment, I'm fine with that too.

aeros

Thanks @taleinat, I'll run through the tests again later tonight and report back with the results.

terryjreedy

I believe the change to What's New 3.8 will prevent the auto backport to 3.7, where the file does not exist. I have usually changed What's New in separate issues, currently bpos 33821 and 33821. (That is partly because What's New has been an after thought.) Since the change to the earliest version (now WN 3.7) will backport all the way, just the change to WN 3.8 needs to be pulled to a separate PR, also attached to 21261, that is only backported to 3.8.

After 3.8.0, when What's New 3.9 is added, I will add an issue for that and 3 separate PRs will be needed to avoid manual backport.

taleinat

I believe the change to What's New 3.8 will prevent the auto backport to 3.7, where the file does not exist.

I'm happy to execute that manual backport. It's simpler for me than having separate PRs.

Also added tests for this case, as well as for typing an opening bracket in a string, which should not trigger dict key completion.

taleinat

I've fixed completions after opening brackets when there are no available dict key completions, e.g. [, globals()[[, {}[ and {1: 1}[. I've also ensure things work properly after typing an opening bracket ([) in a string.

At the same time, I've removed all cases where dict keys were mixed with global variables in the completion list, as requested by @terryjreedy.

@terryjreedy, that addresses all outstanding issues with this PR.

taleinat

I have made the requested changes; please review again.

bedevere-bot

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

# Conflicts: # Doc/whatsnew/3.8.rst

taleinat

I've merged in the updated master branch and removed the outdated "What's New" entries.

This is still waiting for @terryjreedy's review.

# Conflicts: # Doc/library/idle.rst # Lib/idlelib/autocomplete.py # Lib/idlelib/autocomplete_w.py # Lib/idlelib/idle_test/test_autocomplete.py

taleinat

I must say that I'm especially sad that this has not made it into IDLE. :(

terryjreedy

I did not proper cognize until today that mixing in the tab list had been undone. Moving on...

I want to handle mock-tk stuff separately. I believe that your patch would supersede those of issue 18504. I am reserving comments and change request for that or a new issue. I will make the new PR if you want. Nothing should be removed from this patch until obsoleted by merging the new PR.

As for dict string key completion: I am surprised (and not happy) that a seemingly simple and marginally useful addition should nearly double the code in autocomplete.py. So I will be looking to see if anything can be shrunk.

I redid all the experiments discussed in my bug list with string keys and a very short delay All the lists look correct. The main issue I encountered was navigating the list by typing partial entries. I discussed this as part of post on the issue about reconsidering inclusion of bytes keys.

terryjreedy

Changed title to reflect that string key completion works in editor (once code is run, as with imports and definitions) and upon request.

taleinat

As for dict string key completion: I am surprised (and not happy) that a seemingly simple and marginally useful addition should nearly double the code in autocomplete.py. So I will be looking to see if anything can be shrunk.

Indeed, I'm not convinced that including this feature is worth the extra code and effort. Removing the support for bytes would help somewhat, but the support for string keys would still be complex.

I'd feel fine if we decided not to implement this feature: If it means that the work in making this PR helped us make the decision, then it was worth it!

taleinat

Closing in favor of a newer version of this PR, GH-26039.

taleinat added 5 commits August 7, 2019 01:17

prefix preservation for str and bytes

96052fe

refactor back to original interface of fetch_completions()

e995f9a

refactor existing tests and improve mock_tk.Text._decode()

4a58931

add tests

deca90e

add a NEWS entry

2abfa3f

taleinat added type-feature A feature request or enhancement needs backport to 3.7 labels Aug 7, 2019

taleinat requested a review from terryjreedy August 7, 2019 20:33

taleinat self-assigned this Aug 7, 2019

the-knights-who-say-ni added the CLA signed label Aug 7, 2019

bedevere-bot added the awaiting core review label Aug 7, 2019

code cleanup and added comments

b5e7692

aeros reviewed Aug 8, 2019

View reviewed changes

aeros approved these changes Aug 8, 2019

View reviewed changes

aeros reviewed Aug 8, 2019

View reviewed changes

taleinat added 8 commits August 8, 2019 10:15

whitespace fix

4d54812

Escape key closes the completion window

0b41286

a bit of code cleanup

2929d52

many major improvements

0d0e6e0

implement try-open for dict keys, and fix handling of ']'

0087526

fix whitespace

8a4cb1a

improve NEWS entry wording and formatting

f5ed669

_is_completing_dict_key args: rename 'hp' and remove unused 'curline'

f2ef1fa

aeros reviewed Aug 9, 2019

View reviewed changes

be extra-safe and don't crash on import if sys.maxunicode < 0x10000

8459bba

taleinat removed the needs backport to 3.7 label Aug 14, 2019

decaz reviewed Aug 21, 2019

View reviewed changes

taleinat and others added 4 commits August 24, 2019 11:20

update comment

5f89338

Merge branch 'master' into idle-dict-key-completion

7a21705

fix completing after a bracket with no available dict key completions …

c9e0307

Also added tests for this case, as well as for typing an opening bracket in a string, which should not trigger dict key completion.

add a couple more test cases

d97213d

bedevere-bot added awaiting change review and removed awaiting changes labels Aug 31, 2019

taleinat added 2 commits November 14, 2019 13:05

Merge branch 'master' into idle-dict-key-completion …

0c1f907

# Conflicts: # Doc/whatsnew/3.8.rst

revert outdated additions to "What's New" docs

38c28f2

taleinat force-pushed the idle-dict-key-completion branch from 23ee864 to 38c28f2 Compare November 14, 2019 11:08

Merge branch 'master' into idle-dict-key-completion …

530e1f7

# Conflicts: # Doc/library/idle.rst # Lib/idlelib/autocomplete.py # Lib/idlelib/autocomplete_w.py # Lib/idlelib/idle_test/test_autocomplete.py

terryjreedy added the label Sep 20, 2020

terryjreedy changed the title ~~bpo-21261: IDLE shell auto-completion of dict keys~~ Sep 20, 2020

ambv removed the needs backport to 3.8 label May 4, 2021

taleinat mentioned this pull request May 11, 2021

gh-65460: IDLE: add completion of dict keys of type str #26039

Open

taleinat closed this May 11, 2021

rhettinger mentioned this pull request Apr 10, 2022

IDLE: autocomplete dictionary keys #65460

Open

Conversation

taleinat commented Aug 7, 2019 • edited by terryjreedy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aeros left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taleinat commented Aug 8, 2019

Uh oh!

taleinat commented Aug 8, 2019

Uh oh!

aeros commented Aug 8, 2019

Uh oh!

terryjreedy commented Aug 14, 2019

Uh oh!

taleinat commented Aug 14, 2019

Uh oh!

taleinat commented Aug 31, 2019

Uh oh!

taleinat commented Aug 31, 2019

Uh oh!

bedevere-bot commented Aug 31, 2019

Uh oh!

taleinat commented Nov 14, 2019

Uh oh!

taleinat commented Sep 19, 2020

Uh oh!

terryjreedy commented Sep 20, 2020

Uh oh!

terryjreedy commented Sep 20, 2020

Uh oh!

taleinat commented Sep 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taleinat commented May 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

taleinat commented Aug 7, 2019 •

edited by terryjreedy

Loading

aeros left a comment •

edited

Loading

taleinat commented Sep 20, 2020 •

edited

Loading