python / cpython

The Python programming language
https://www.python.org/
Other
60.75k stars 29.32k forks source link

IDLE: Fix tab completion after settings and some keys #87820

Open terryjreedy opened 3 years ago

terryjreedy commented 3 years ago
BPO 43654
Nosy @gvanrossum, @rhettinger, @terryjreedy, @taleinat, @tetelevm
PRs
  • python/cpython#26403
  • python/cpython#26421
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/terryjreedy' closed_at = None created_at = labels = ['expert-IDLE', 'type-bug', '3.9', '3.10', '3.11'] title = 'IDLE: Fix tab completion after settings and some keys' updated_at = user = 'https://github.com/terryjreedy' ``` bugs.python.org fields: ```python activity = actor = 'taleinat' assignee = 'terry.reedy' closed = False closed_date = None closer = None components = ['IDLE'] creation = creator = 'terry.reedy' dependencies = [] files = [] hgrepos = [] issue_num = 43654 keywords = ['patch'] message_count = 13.0 messages = ['389673', '389791', '390731', '390732', '390755', '390824', '394542', '394545', '394631', '394883', '395063', '395201', '411043'] nosy_count = 5.0 nosy_names = ['gvanrossum', 'rhettinger', 'terry.reedy', 'taleinat', 'tetelevm'] pr_nums = ['26403', '26421'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue43654' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    terryjreedy commented 3 years ago

    (Original report by Mikhail on bpo-43647, running 3.9 on Linux; verified and extended by me running 3.10 on Windows.)

    Normally, "i\<tab>" brings up a completion window with 'id', 'if', 'import', etc. Opening a Settings windows with Options => Configure IDLE and closing with Apply and Cancel or OK (which also applies) disables tab completion. Other completions (forced with ^\<space> or auto with '.' or '/' and waits seem not affected. The only way to restore is to close and reopen each window.

    Tab completions are enabled in editor.py with these two lines. text.event_add('\<\<autocomplete>>', '\<Key-Tab>') text.bind("\<\<autocomplete>>", autocomplete.autocomplete_event) Attribute and path completions, not affected, are enabled with these. text.event_add('\<\<try-open-completions>>', '\<KeyRelease-period>', '\<KeyRelease-slash>', '\<KeyRelease-backslash> text.bind("\<\<try-open-completions>>", autocomplete.try_open_completions_event) Similarly for some other things.

    In configdialog, the relevant method is (179) def apply, who relevant calls are (219) deactivate_current_config and (230) activate_current_config. The former removes key bindings and the latter rebinds and makes other changes.

    What is different about Tab versus '.' is that is tab also used for indents and the indent space is reset by 'activate...'. I will later add some debug prints to console based on the clues above.

    terryjreedy commented 3 years ago

    Investigation of this issue is complicated by the fact that the editor test widgets are wrapped by multicall.MulticallCreator. It intercepts bind and event method calls (other than event_generate) for user pseudoevents. It keeps its own map of pseudoevent names to handler and key sequences instead of passing the information on to tk.

    The custom entry for "\<\<autocomplete>>" is the same, [\<bound method AutoComplete.autocompleteevent of \<idlelib.autocomplete.AutoComplete object at 0x00000209DA4FBD20>>, [(0, 0, 'Tab')]] both before deactivate... and after activate_... .

    I determined this by adding calls like the following print('ei5 ', instance.text.event_info('\<\<autocomplete>>')) and adding the following to the eventinfo(virtual) override. print('mei', virtual, self.\_eventinfo.get(virtual))

    I will next look at the custom binding data that should map keys to event handlers.

    d874385e-1431-4256-94f0-c8c66a2e2231 commented 3 years ago

    Hello again! I found another kind of bug, where the autosupplement window doesn't show up. In order for the bug to occur, you need to put a non-string value into locals(). Example:

    locals()['arg'] = 123  # or locals().setdefault('arg', 123)
    # ^ it's okay
    
    locals()[123] = 123  # or locals().setdefault(123, 123)
    # ^ completion window is not shown

    Of course, I know that "Whether or not updates to this dictionary will affect name lookups in the local scope and vice-versa not covered by any backwards compatibility guarantees" , but take a little bug :)

    Here is the traceback of the error

    Exception in Tkinter callback
    Traceback (most recent call last):
      File "/usr/lib/python3.9/tkinter/__init__.py", line 1885, in __call__
        return self.func(*args)
      File "/usr/lib/python3.9/idlelib/multicall.py", line 176, in handler
        r = l[i](event)
      File "/usr/lib/python3.9/idlelib/autocomplete.py", line 74, in autocomplete_event
        opened = self.open_completions(TAB)
      File "/usr/lib/python3.9/idlelib/autocomplete.py", line 146, in open_completions
        comp_lists = self.fetch_completions(comp_what, mode)
      File "/usr/lib/python3.9/idlelib/autocomplete.py", line 171, in fetch_completions
        return rpcclt.remotecall("exec", "get_the_completion_list",
      File "/usr/lib/python3.9/idlelib/rpc.py", line 219, in remotecall
        return self.asyncreturn(seq)
      File "/usr/lib/python3.9/idlelib/rpc.py", line 250, in asyncreturn
        return self.decoderesponse(response)
      File "/usr/lib/python3.9/idlelib/rpc.py", line 270, in decoderesponse
        raise what
    TypeError: '<' not supported between instances of 'int' and 'str'
    d874385e-1431-4256-94f0-c8c66a2e2231 commented 3 years ago

    I did a little experiment, and it turned out that the problem is not in the IDLE, but inside Python itself. With this case, the same behavior remains inside the terminal version of Python, and IPython also produces a similar error.

    terryjreedy commented 3 years ago

    I presume '^' means 'hit Tab'.

    The underlying Python problem is that namespaces are conceptually mappings of names (identifiers), a subset of strings, to objects. Python specifies that the global namespace is a dict, which I believe could mean a dict subclass restricting keys to strings or maybe even identifier strings. But CPython, at least, uses built-in dicts, allowing any hashable as a key. Ditto for attribute dicts and locals. At toplevel, locals() is globals().

    (There are a few semi-same uses to non-identifier string completions.
    >>> globals()["3.1459"] = None
    >>> 3.1459 # 3<tab>
    and
    >>> globals()['itertools.permutations'] = None
    >>> itertools.permutations # itert<tab>
    But not recommended or, I believe, guaranteed.)

    Completion lists are sorted. In original Python, sorting was universal. That was broken for a few types by 2.7. In 3.x, comparisons and hence sorting were restricted.

    Decoding the traceback. Completion requests are handled in the IDLE interface process. The remotecall in autocomplete.fetch_completions tries to execute fetch_completions in the user execution process. That fails trying to sort the list of globals. The exception is returned with the tag "CALLEXC". In rpc, decoderesponse filters out various failures and raises the returned exception.

    IDLE could filter out non-strings from global and attribute dicts or display a special message. Or do nothing on the basis that putting non-strings in namespaces and making them unsortable is either a user bug or a consenting adults, do at your own risk and deal with the consequences operation.

    Built-in dir() sorts keys and hence it and anything using it fails.

    >>> globals()[1]=None
    >>> dir()
    Traceback (most recent call last):
      File "<pyshell#1>", line 1, in <module>
        dir()
    TypeError: '<' not supported between instances of 'int' and 'str'

    Indeed, fetch_completions uses dir(), and this is where it fails.

    Vars() does not sort, allowing one to see the full contents.

    >>> vars()
    {'__name__': '__main__', '__doc__': None, '__package__': None, '__loader__': <class '_frozen_importlib.BuiltinImporter'>, '__spec__': None, '__annotations__': {}, '__builtins__': <module 'builtins' (built-in)>, 1: None}

    If one edits a file after putting a non-string in globals, the TypeError is not displayed. Instead, tab completion is silently disabled and spaces are inserted, just as after applying settings. This is not good and so a point in favor of doing something.

    d874385e-1431-4256-94f0-c8c66a2e2231 commented 3 years ago

    I also checked this behavior in Jython and IPython, and there errors are explicitly caused when putting/calling such values (I also checked on MicroPython online version, everything just hangs there). For IPython I created an issue on their Github, I'm thinking of doing the same for Jython. Since you're my guide to the world of bugs and issues for CPython, any advice on whether to create a separate issue here or leave it as is?

    P.S. I like the Python community, everyone here is kind and helpful :)

    taleinat commented 3 years ago

    The original issue appears to be caused by \<\<smart-indent>> also being bound to Tab, but \<\<autocomplete>> only being bound using eventadd() once in EditorWindow.\_init__(). Therefore, RemoveKeybindings() does remove the binding for tab due to \<\<smart-indent>>, but \<\<autocomplete>> is not bound again by ApplyKeybindings().

    That should mean that tab is still bound to \<\<autocomplete>> and \<\<smart-indent>>, as expected, but the order has changed: now \<\<smart-indent>> was the last event added. We need \<\<autocomplete>> to fire first.

    Specifically, commenting out the line for \<\<smart-indent>> from config.py causes this bug to no longer happen.

    taleinat commented 3 years ago

    Indeed, adding the events bound in EditorWindow.__init__ to RemoveKeybindings() and ApplyKeybindings(), so they are bound in the correct order, resolves this issue.

    See PR python/cpython#70591.

    terryjreedy commented 3 years ago

    Tal, I think you had the right solution the first time. To me, tab is (should be) a fixed key, and the event added just once, like the other fixed keys. I am quite surprised that it isn't. Please revise or make a new patch to remove it from both configkeys.def and config.py.

    Even if I agreed with unfixing other keys, only \<\<autocomplete>> would need rebinding, not an arbitrary subset of the fixed keys. We mostly did away with treating some features differently; what remains was mostly an expedient to get a too-big patch merged. I meant and still mean to finish the job.

    terryjreedy commented 3 years ago

    Currently, the keys that define certain pseudoevents and invoke the associated event handlers are fixed: tab, '.', and within strings, '/' and '\' for completions, '(' and ')' for calltip open and close, and ')', ']', and '}' for opener matching. Note that quote matching is indicated by coloring completed strings.

    26421 (which replaces #26403) proposes to augment this list with tab for smart indent, backspace for smart backspace, and newline for completing a line and maybe smart indenting. In other words, remove the following 3 lines

    '\<\<smart-backspace>>': ['\<Key-BackSpace>'], '\<\<newline-and-indent>>': ['\<Key-Return>', '\<Key-KP_Enter>'], '\<\<smart-indent>>': ['\<Key-Tab>'],

    from config-keys.def and the Settings Keys tab and add them as fixed binding to EditorWindow.__init__ just above the existing fixed pseudoevent -- keys bindings.

    Only fixing smart-indent and tab (or unfixing name-completion and tab) is needed to keep name completion working after re-doing setting. So why all three? 1. These three pairs go together; I don't see anything else that should be fixed. 2. By the standard used to fix some pairs already, these three should be also. I think it an accident of IDLE's history that they aren't*. 3. It might be that changing either of the other two binding could disable something, or might in the future. In other words, I consider having this bindings be mutable to be a bit buggy, with this issue being evidence.

    The unknown is whether anyone has changed these pseudoevent bindings and if so, how much do we care?

    Guido, do you have any comment on this proposal from a change policy perspective?

    rhettinger commented 3 years ago

    The unknown is whether anyone has changed these pseudoevent bindings and if so, how much do we care?

    I don't think we care. Getting tab completion sorted out is the priority.

    taleinat commented 3 years ago

    The unknown is whether anyone has changed these pseudoevent bindings and if so, how much do we care?

    Let's ask on idle-dev.

    Guido, do you have any comment on this proposal from a change policy perspective?

    I'm not an ex-BDFL, but I can't think of a reason this shouldn't be backported as per our usual policy regarding IDLE.

    taleinat commented 2 years ago

    Terry, for all intents and purposes you're the one in charge of IDLE now. I suggest deciding whether to change all three bindings as in the current PR or only the one for completions. Just le me know and if needed I'll make a new PR.