feat(search): introduce keymap-dependent vim-mode

[akinomyoga]

This is a possible refinement to the vim mode as suggested in #1553 (comment).

In this PR, we introduce three keymap modes, emacs, vim-insert, and vim-normal (which was extended from the current VimMode). When the config vim is enabled, the keys are processed with special rules for the current keymap mode. This is actually the same as the current main. The real behavioral change is related to the following config keymap_mode:

The keymap mode with which the Atuin search is started is controlled by the config keymap_mode. When keymap_mode is not specified, the keymap mode on the startup is determined based on the shell's keymap that started the Atuin search. When the config keymap_mode is specified, the keymap mode on the startup is forced to be the specified one.

@YummyOreo @ellie What do you think of this interface? If you have another idea, I can adjust the PR.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
atuin-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 16, 2024 1:06pm

[akinomyoga]

@Nemo157 I added commit 075622d, where widgets with specific keymap modes are prepared. You can e.g. bind widgets as follows:

bindkey -a / _atuin_search_viins_widget
bindkey -a k _atuin_up_search_vicmd_widget

[Nemo157]

Nice, that works about how I'd expect. I wonder if the generated key-bindings should be changed based on vim-mode too. Currently the ^r binding overrides the "redo" binding from zsh's normal-mode, and k// seem much more familiar bindings to use. I also assume these widgets are internal API so I shouldn't really be binding to them directly.

[akinomyoga]

Currently the ^r binding overrides the "redo" binding from zsh's normal-mode, and k// seem much more familiar bindings to use.

Hmm, I agree. In fact, in the Bash integration, we avoid to overwrite C-r in Bash's vi-command keymap:

atuin/atuin/src/shell/atuin.bash

Lines 226 to 232 in c2439d1

	if [[ $__atuin_bind_ctrl_r == true ]]; then
	# Note: We do not overwrite [C-r] in the vi-command keymap for Bash because
	# we do not want to overwrite "redo", which is already bound to [C-r] in
	# the vi_nmap keymap in ble.sh.
	bind -m emacs -x '"\C-r": __atuin_history'
	bind -m vi-insert -x '"\C-r": __atuin_history'
	fi

I think we can add the keybinding to / in a default set of the keybindings. On the other hand, I'm not sure if we can remove the keybinding to C-r from vicmd. I feel we can remove it, but some users might already rely on it in vicmd.

I also assume these widgets are internal API so I shouldn't really be binding to them directly.

I assume them to be a part of the public interface because they seem to appear in a sample configuration:

• zsh - Key Binding | Atuin Docs

(Edit) If they are really a part of the public interface, maybe should we rename them so that they do not start with _? The identifiers starting with _ appear to be internal API.

[akinomyoga]

I thought about it again, and I think another option for the configuration interface would be to provide a single configuration keymap_mode = { "emacs" (default) | "vim-insert" | "vim-normal" | "auto" } instead of vim (i.e. remove vim). By making emacs default, it doesn't change the behavior for the existing users. When the user wants the automatic determination of the startup keymap mode, the user can specify keymap_mode = "auto". When the user wants to always start the Atuin search with the vim-normal mode, the user can specify keymap_mode = "vim-normal".

[akinomyoga]

I thought about it again, and I think another option for the configuration interface would be

I implemented this in the latest commit 96eeca0. I feel this is cleaner, though we have discussed the global config vim plus another config in #1553. The default value is emacs to keep the behavior for the existing users unchanged. The users who would like the Atuin search to always start with the vim-normal mode can specify keymap_mode = "vim-normal" instead of the current way vim = true. The users who would like the automatic determination of the startup keymap of the Atuin search can specify keymap_mode = "auto".

[ellie]

Let's avoid extending this match in any future PRs, it's huge and getting difficult to understand. It would make sense to abstract this out, but I'll make an issue for this. Happy with this for now.

[akinomyoga]

I also thought about it, but I kept it since you seemed to have an idea in #1553 (review).

[ellie]

Looks good to me, thank you!

Would you also be able to update the docs please?

[ellie]

ah and the shellcheck

[akinomyoga]

Thank you! I've fixed SC2048, rebased on top of the latest main, and squashed trivial commits.

[akinomyoga]

I opened atuinsh/docs#8 in atuin/docs.