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

Fix autosuggestions in multi-line mode, vi command mode delay #13991

Merged
merged 3 commits into from Mar 30, 2023

Conversation

krassowski
Copy link
Member

Fixes #13970. Relates to #13443.

Documents #12603 with a new emacs_like_insert_mode filter alias.

@krassowski krassowski added bug autosuggestions Related to fish-like autosuggestion feature (as opposed to the tab-completions) labels Mar 26, 2023
@Carreau
Copy link
Member

Carreau commented Mar 27, 2023

+1,

I pushed a commit that use module level __getattr__ to emit a deprecation warning for accept_in_vi_insert_mode, added a test that it does, and remove the last few usage of it.

I'll let you review the last commit and merge if you are ok with it.

@@ -186,12 +186,12 @@ def create_identifier(handler: Callable):
# 2) prompt-toolkit checks if we are at the end of text, not end of line
# hence it does not work in multi-line mode of navigable provider
Binding(
auto_suggest.accept_in_vi_insert_mode,
auto_suggest.accept_or_jump_to_end,
Copy link
Member Author

Choose a reason for hiding this comment

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

If a user configured shortcuts, their custom configuration will no longer work after this change. This is a twofold issue:

  • if they overridden (modified/removed) this shortcut by matching by the old command name (accept_in_vi_insert_mode), the override will no longer work as it will not find a match (accept_in_vi_insert_mode is no longer here)
  • if they added a new shortcut using the old command name, it will not work either as accept_in_vi_insert_mode command is not exposed anymore (currently we only allow commands which are in existing bindings; we could have a separate register of additional allowed commands which would alleviate this problem)

This is relatively minor breakage given that shortcuts were just introduced, and the name of this command was in a way misleading (it does not check for vi insert mode), so I don't have a strong opinion here.

Copy link
Member

Choose a reason for hiding this comment

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

Good point, though if we don't emit a deprecation warning people will use the old name for ever.

Let's try to break, and if we have complaints I can do a patch release.

@Carreau Carreau merged commit 4e7b940 into ipython:main Mar 30, 2023
22 checks passed
@Carreau Carreau added this to the 8.12 milestone Mar 30, 2023
Carreau added a commit that referenced this pull request Mar 30, 2023
This is a pre-requisite of #13992 but the shortcut is disabled by
default by `never` filter. The idea here is that this could be merged
as-is (ideally after rebasing on top of #13991) to allow user testing.
Once this is in, users can emulate part of the behaviour proposed in
#13992 with the following snippet:

```python
custom_shortcuts = [
    {
        "command": "IPython:auto_suggest.resume_hinting",
        "new_keys": ["right"],
        "new_filter": "default_buffer_focused & ((vi_insert_mode & ebivim) | emacs_insert_mode)"
    }
]
%config TerminalInteractiveShell.shortcuts = custom_shortcuts
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosuggestions Related to fish-like autosuggestion feature (as opposed to the tab-completions) bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto suggestions cannot be completed when in the middle of multi-line input
2 participants