Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

wants to merge 1 commit into from
+1,160 −179

Conversation

taleinat
Copy link
Contributor

@taleinat taleinat commented May 11, 2021 •
edited by terryjreedy
Loading

This is an updated version of PR GH-15169, rebased onto the current main branch and with support for keys of type bytes removed.

Note that completion is intentionally limited to keys of type str 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.

@taleinat
IDLE: add completion of dict keys of type str
84fa6f1 
Signed-off-by: Tal Einat <532281+taleinat@users.noreply.github.com>
@taleinat taleinat added the type-feature A feature request or enhancement label May 11, 2021
@taleinat taleinat requested a review from terryjreedy as a code owner May 11, 2021 20:26
@taleinat taleinat changed the title IDLE: add completion of dict keys of type str bpo-21261: IDLE: add completion of dict keys of type str May 11, 2021
@taleinat taleinat mentioned this pull request May 11, 2021
Closed
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jun 11, 2021
@taleinat
Copy link
Contributor Author

taleinat commented Sep 10, 2021 •
edited
Loading

@terryjreedy, how do we move forward with this?

You previously mentioned the added complexity and amount of code as something you were weighing against the benefits of having this feature. To address that, this PR has less code than the previous one which also support keys of type bytes. Beyond that, I can commit to maintain this feature in the foreseeable future.

I will mention that using this locally has been extremely nice, and I think this would make a greater positive impact on users' experiences than you may think.

@terryjreedy
Copy link
Member

We need an update merge and resolution of test failures. Old test reports are gone. On my machine, test_idle passes on current main,. It fail thrice on this branch. Perhaps a result of changing mock_tk.Text.

FAIL: test_comment_parens (idlelib.idle_test.test_calltip.CalltipTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "f:\dev\3x\lib\idlelib\idle_test\test_calltip.py", line 359, in test_comment_parens
    self.open_close(comment)
  File "f:\dev\3x\lib\idlelib\idle_test\test_calltip.py", line 325, in open_close
    self.assertIsNone(self.ct.active_calltip, None)
AssertionError: <idlelib.idle_test.test_calltip.mock_TipWindow object at 0x000001E31B329EF0> is not None

Inserting ')' into mock Text on 324 should have closed it.

FAIL: test_repeated_force (idlelib.idle_test.test_calltip.CalltipTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "f:\dev\3x\lib\idlelib\idle_test\test_calltip.py", line 339, in test_repeated_force
    self.open_close(force)
  File "f:\dev\3x\lib\idlelib\idle_test\test_calltip.py", line 322, in open_close
    testfunc(self)  ###
  File "f:\dev\3x\lib\idlelib\idle_test\test_calltip.py", line 338, in force
    self.assertIs(self.ct.active_calltip, self.tip)
AssertionError: None is not <idlelib.idle_test.test_calltip.mock_TipWindow object at 0x000001E31B2EBE30>

self.ct.open_calltip(True) on line 337 should leave a calltip.

FAIL: test_repeated_parens (idlelib.idle_test.test_calltip.CalltipTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "f:\dev\3x\lib\idlelib\idle_test\test_calltip.py", line 350, in test_repeated_parens
    self.open_close(parens)
  File "f:\dev\3x\lib\idlelib\idle_test\test_calltip.py", line 325, in open_close
    self.assertIsNone(self.ct.active_calltip, None)
AssertionError: <idlelib.idle_test.test_calltip.mock_TipWindow object at 0x000001E31B5E6EE0> is not None

Repeat of first error.

@terryjreedy
Copy link
Member

terryjreedy commented Sep 13, 2021 •
edited
Loading

Manual testing: d[<wait> pops up string keys. Other keys, including bytes, are not listed.
d[<tab/force> and d[' (any quote) pops up string keys, except that tab with unique match fills it in, adds correct closing quote, and finishes with closing ]. Closing with return does same.
d[a<tab/force, where ais any non-quote char and entry is not partial bytesb'a, pops up module name list. d[b'<tab/force>` pops up list for string/bytes path completion.
Without checking old PR, I think these are correct.

Old bug: Esc does not remove and preliminary fill in. Opening does not fill in anything, so that click or down+up is needed to complete with first item. These needed to be fixed together.

Please post on the list as to how you found this useful.
ToDo: review rest of patch itself, starting with doc edit.

terryjreedy
terryjreedy requested changes Sep 13, 2021
View reviewed changes
Lib/idlelib/idle_test/test_text.py
@@ -22,24 +22,24 @@ def test_init(self):
def test_index_empty(self):
index = self.text.index

for dex in (-1.0, 0.3, '1.-1', '1.0', '1.0 lineend', '1.end', '1.33',
for dex in ('0.0', '0.3', '1.0', '1.0 lineend', '1.end', '1.33',
Copy link
Member

Choose a reason for hiding this comment

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

The test examples removed here and below are legal and intentional. Please reinstate or explain.

Misc/NEWS.d/next/IDLE/2019-08-07-23-27-54.bpo-21261.sqNQCD.rst
Comment on lines +1 to +2
IDLE's shell auto-completion works for dict keys of types :class:`str`
and :class:`bytes`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
IDLE's shell auto-completion works for dict keys of types :class:`str`
and :class:`bytes`.
IDLE's shell auto-completion works for dict keys of types :class:`str`.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@taleinat
Copy link
Contributor Author

taleinat commented Sep 14, 2021 •
edited
Loading

Updating that this is currently blocked by completions being broken on Linux (bpo-45193).

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 7, 2022
@rhettinger rhettinger mentioned this pull request Apr 10, 2022
Open
@terryjreedy terryjreedy changed the title bpo-21261: IDLE: add completion of dict keys of type str gh-65460: IDLE: add completion of dict keys of type str Oct 30, 2022
@python-cla-bot
Copy link

python-cla-bot bot commented Apr 18, 2025 •
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@terryjreedy terryjreedy terryjreedy requested changes

Assignees
No one assigned
Labels
awaiting changes type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@taleinat @terryjreedy @bedevere-bot @the-knights-who-say-ni @ezio-melotti