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

Documentation suggests "keymap_mode" exists but it's not in the config #1625

Closed
cohml opened this issue Jan 25, 2024 · 5 comments
Closed

Documentation suggests "keymap_mode" exists but it's not in the config #1625

cohml opened this issue Jan 25, 2024 · 5 comments

Comments

@cohml
Copy link

cohml commented Jan 25, 2024

The documentation states

keymap_mode

Default: “emacs”

The initial keymap mode of the interactive Atuin search (e.g. started by the keybindings in the shells). There are four supported values: "emacs", "vim-normal", "vim-insert", and "``auto". The keymap mode "emacs"` is the most basic one. In the keymap mode `"vim-normal"`, you may use k and j to navigate the history list as in Vim, whilst pressing i changes the keymap mode to `"vim-insert"`. In the keymap mode `"vim-insert"`, you can search for a string as in the keymap mode `"emacs"`, while pressing Esc switches the keymap mode to `"vim-normal"`. When set to `"auto"`, the initial keymap mode is automatically determined based on the shell’s keymap that triggered the Atuin search. `"auto"` is not supported by NuShell at present, where it will always trigger the Atuin search with the keymap mode `"emacs"`.

However, the config actually appears to expose no such parameter:

❯ atuin default-config | grep -q keymap_mode && echo found || echo not found
not found

This seems like a giant oversight, so it makes me wonder whether I'm missing something, or else whether the missing parameter is a known thing. But searching the current issues for keymap_mode and returns nothing, hence this issue feels justified.

@akinomyoga
Copy link
Contributor

As far as I run the same command, it's in the config.

[murase@chatoyancy 0 atuin]$ atuin default-config | grep keymap_mode
# keymap_mode = "auto"

Which version of atuin are you using? The documentation reflects the configuration of the main branch. The config keymap_mode is a recently added configuration.

@ellie
Copy link
Member

ellie commented Jan 25, 2024

Yep, this is currently only on main and will be part of the next release

The documentation reflects the configuration of the main branch.

The documentation should probably be versioned, I'll add it to the todo

@cohml
Copy link
Author

cohml commented Jan 25, 2024

Then that would explain it, because I just installed atuin yesterday:

❯ atuin --version
atuin 17.2.1

I figured this was a known thing. However, the docs are also out of sync with the default value of enter_accept, yet the docs also explicitly acknowledge that. It was just weird why the enter_accept blurb had no such acknowledgement.

Anyway if this is already on the devs' radar, I will close the issue. Thanks for the lightening fast replies!

@cohml cohml closed this as completed Jan 25, 2024
@ellie
Copy link
Member

ellie commented Jan 25, 2024

Thanks!

The docs for enter_accept are accurate

This technically defaults to true for new users, but false for existing. We have set enter_accept = true in the default config file. This is likely to change to be the default for everyone in a later release.

We prefill enter_accept as true for new users. If it is absent, it defaults to false

This means we wouldn't change the behaviour for existing users, as it's quite a big change

@cohml
Copy link
Author

cohml commented Jan 25, 2024

Sorry, I guess I was unclear.

I just meant that as I was skimming the docs yesterday top to bottom, for enter_accept the first thing I saw was

Default: false

This initially confused me, because it was inconsistent with my local default. But then I read further and saw that the docs already acknowledge this discrepancy. (Maybe you could put an asterisk there or something? "Default: false*". NBD 😄 )

By contrast, the discrepancy between docs and reality for keymap_mode is not similarly acknowledged. The docs say that parameter is there, but for people downloading the latest release it just....isn't.

But anyway, I see that it's coming, not an oversight. So we're in the clear 👍

ellie pushed a commit to atuinsh/docs that referenced this issue Jan 29, 2024
Commit b8c6b9a corresponds to the widget name change introduced in
atuinsh/atuin#1631. Also, the current
documentation diverges from the current implementation. I updated the
widgets for the <kbd>up</kbd> key in commit 6e1f3df. Also, the
descriptions for the newly added widgets for `keymap_mode`
(atuinsh/atuin#1570) are added in commit
705214a.

Since the version dependence of documentation can confuse people as in
atuinsh/atuin#1625, I decided to add a note on
the version dependence and also a required version for each feature. I
personally think it is useful to maintain in the documentation the
minimal version requirement for each feature, but this might be the
perspective of a developer who needs to consider the compatibility of
products with arbitrary versions of other products. If you have a
different preference, please tell me that.

There is an extra commit 33a1d6d. Sorry, this is unrelated to the widget
name but a tiny clarification of a description I added in my previous PR
#10. If I should submit a change in a separate PR, please tell me that.
It's a small change, but I'll create a separate one if you'd prefer it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants