Allow to customise shortcuts using a traitlet#13928
Conversation
e2f5d93 to
1d72045
Compare
| """.format( | ||
| "\n ".join([f"- `{k}`" for k in KEYBINDING_FILTERS]) | ||
| ), | ||
| ).tag(config=True) |
There was a problem hiding this comment.
Ok.
(i'm just going to write some Ok to know where I am in the review).
| final_module = parts[-1] | ||
| return f"{package}:{final_module}.{name}" | ||
| else: | ||
| return f"{package}:{name}" |
There was a problem hiding this comment.
That is the opposite convention of i believe pytest (and a few other things), where is <fully qualified path>:<object potentially with dot access to attribute>.
Any particular reasons to use this schema ?
There was a problem hiding this comment.
That's a good point. I have mixed feelings on the implementation I chose because as you say fully qualified path feels more standard. The reason why I did not go for it is that (a) some are really really long, see below for examples (b) it was initially written to use this in documentation and only later repurposed for customisation (this PR). Examples of worst offenders would be:
prompt_toolkit.key_binding.bindings.auto_suggest.load_auto_suggest_bindings.<locals>._accept
prompt_toolkit.shortcuts.prompt.PromptSession._create_prompt_bindings.<locals>._complete_like_readline
prompt_toolkit.key_binding.bindings.completion.display_completions_like_readline
In IPython it is not as bad but there is a number of long ones:
IPython.terminal.shortcuts.previous_history_or_previous_completion
IPython.terminal.shortcuts.auto_suggest.accept_and_move_cursor_left
IPython.terminal.shortcuts.auto_suggest.accept_in_vi_insert_mode
I am happy to go either way.
There was a problem hiding this comment.
Let's go with this for now, you already wrote it.
| key_bindings = create_ipython_shortcuts(self, skip=shortcuts_to_skip) | ||
| for binding in shortcuts_to_add: | ||
| add_binding(key_bindings, binding) | ||
| self.pt_app.key_bindings = key_bindings |
Carreau
left a comment
There was a problem hiding this comment.
That looks good to me, I'm going to delay the merging a bit as this is a big change I I want to make a small release before as there is a CVE that need fixing/release first.
So I'm going to do a 8.10, and this can go in 8.11.
| color_depth=self.color_depth, | ||
| tempfile_suffix=".py", | ||
| **self._extra_prompt_options() | ||
| **self._extra_prompt_options(), |
| final_module = parts[-1] | ||
| return f"{package}:{final_module}.{name}" | ||
| else: | ||
| return f"{package}:{name}" |
There was a problem hiding this comment.
Let's go with this for now, you already wrote it.
| "c-_": nc.undo, | ||
| } | ||
| }.items() | ||
| ] |
| """Skip over automatically added parenthesis/quote. | ||
|
|
||
| (rather than adding another parenthesis)""" | ||
| (rather than adding another parenthesis/quote)""" |
| assert up(event) is None | ||
| assert down(event) is None | ||
| assert swap_autosuggestion_up(event) is None | ||
| assert swap_autosuggestion_down(event) is None |
This is a refactor of keybindings code aiming to enable users to modify, disable, and add new shortcuts.
Closes #13878, relates to #13879.
Code changes
filters.pymodulecreate_ipython_shortcuts(shell)were moved to globals (which ensures nice identifier names and makes unit-testing easier)KEY_BINDINGSglobal variable; in future one could consider further splitting them up and moving bindings definition to respective modules (e.g.AUTO_MATCH_BINDINGStoauto_match.py).User-facing changes
c.TerminalInteractiveShell.shortcutsAfter a few iterations I arrived to a specification that separates the existing key/filter from the new key/filter and has a separate "create" flag used to indicate that a new shortcut should be created (rather than modifying an existing one):