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

nushell: atuin should not always trigger on ⬆️ #1025

Closed
freijon opened this issue Jun 2, 2023 · 12 comments · Fixed by #1770
Closed

nushell: atuin should not always trigger on ⬆️ #1025

freijon opened this issue Jun 2, 2023 · 12 comments · Fixed by #1770

Comments

@freijon
Copy link

freijon commented Jun 2, 2023

With the nushell integration the "Arrow up" key is used to browse the history with atuin.
However, I don't always want that. For example when I'm in "picker mode" of nushell.

Example:
Type "math" and press . A menu with available math commands is shown that (normally) can be navigated with ⬆/⬇. However with the atuin integration, the ⬆ key closes the pick menu and shows the history with atuin.

The history should only show up when in normal prompt mode. Not when a menu is shown.

@alextremblay
Copy link

hmmm... i think that might be a nushell bug. I think nushell should be overriding global keybinds like "up arrow" with local keybinds but isn't doing that.

Have you opened an issue in the nushell repo? if so, can you link to it here?

@arcuru
Copy link
Contributor

arcuru commented Oct 25, 2023

This issue leads to a pretty terrible experience, should we disable the up arrow by default for Nushell until this can be fixed?

@ellie
Copy link
Member

ellie commented Oct 25, 2023

This issue leads to a pretty terrible experience, should we disable the up arrow by default for Nushell until this can be fixed?

I think that would probably be best, though hopefully their keybindings become more flexible soon 🙏 Are you happy to make the PR?

arcuru added a commit to arcuru/atuin that referenced this issue Oct 26, 2023
The up-arrow keybinding triggers anytime the key is pressed, including in menus or multiline commands. Disabling it by default for new users. See atuinsh#1025
azzamsa added a commit to azzamsa/dotfiles that referenced this issue Jan 9, 2024
Welcome to the club!

However, after several hours of setup. Nushell is not ready for daily usage.

- [nushell: atuin should not always trigger on ⬆️ · Issue #1025 · atuinsh/atuin](atuinsh/atuin#1025)
- [`column_not_found` in Nushell · sxyazi/yazi · Discussion #501](sxyazi/yazi#501)
- [Generated .zoxide.nu has incorrect syntax · Issue #661 · ajeetdsouza/zoxide](ajeetdsouza/zoxide#661)
- [Nushell support · Issue #463 · Schniz/fnm](Schniz/fnm#463)
- Syntax highlighting still requires manual setup.
  - https://github.com/nushell/tree-sitter-nu/blob/main/installation/neovim.md#manual-installation
- nufmt is still very much in beta.
azzamsa added a commit to azzamsa/dotfiles that referenced this issue Jan 18, 2024
Welcome to the club!

However, after several hours of setup. Nushell is not ready for daily usage.

- [nushell: atuin should not always trigger on ⬆️ · Issue #1025 · atuinsh/atuin](atuinsh/atuin#1025)
- [`column_not_found` in Nushell · sxyazi/yazi · Discussion #501](sxyazi/yazi#501)
- [Generated .zoxide.nu has incorrect syntax · Issue #661 · ajeetdsouza/zoxide](ajeetdsouza/zoxide#661)
- [Nushell support · Issue #463 · Schniz/fnm](Schniz/fnm#463)
- Syntax highlighting still requires manual setup.
  - https://github.com/nushell/tree-sitter-nu/blob/main/installation/neovim.md#manual-installation
- nufmt is still very much in beta.
@from-nibly
Copy link

FYI the default keybinding in nu is this

    {
        name: move_up
        modifier: none
        keycode: up
        mode: [emacs, vi_normal, vi_insert]
        event: {
            until: [
                {send: historyhintcomplete }
                {send: menuup}
                {send: up}
            ]
        }
    }

I think the "until" is how you leverage "doing the right thing at the right time". I don't know any more details than that.

@remmycat
Copy link
Contributor

remmycat commented Feb 25, 2024

I'm not sure where you copied this from, but the default config seems to be this and hasn't changed in a while, except for formatting:

        {
            name: move_up
            modifier: none
            keycode: up
            mode: [emacs, vi_normal, vi_insert]
            event: {
                until: [
                    {send: menuup}
                    {send: up}
                ]
            }
        }

You can find the until reference here: https://www.nushell.sh/book/line_editor.html#until-type

Based on this, I modified the commented out part in the init script

$env.config = (
    $env.config | upsert keybindings (
        $env.config.keybindings
        | append {
            name: atuin
            modifier: none
            keycode: up
            mode: [emacs, vi_normal, vi_insert]
            event: {
                until: [
                    {send: menuup}
                    {send: executehostcommand cmd: (_atuin_search_cmd '--shell-up-key-binding') }
                ]
            }
        }
    )
)

This seems to work! I can navigate through menus and when not in one I'm opening atuin.
Maybe I'm missing something but I think this is the fix.

Basically it first tries to do the menuup action, if that fails (because we're not in a menu), it tries atuin.

I was thinking to add a {send: up} fallback at the end when atuin fails, but the docs make it sound like currently only the menu actions can fail in a way that makes the next "until" item fire.

And yeah, when I modify the command to produce a positive exit code, the current behaviour seems to be that it just silently stops or crashes in some way, and nu gives me a new line. Instead of going to the next action in the array (what I thought might happen).

@from-nibly
Copy link

from-nibly commented Feb 25, 2024 via email

@remmycat
Copy link
Contributor

Awesome - I went ahead and created a PR before I forget - let me know if the fix works for you!

@remmycat
Copy link
Contributor

Oh, and just for the next person being confused, the original issue description is probably meant to say Type "math" and press tab. since the default tab-autocompletion opens as a menu with options.

At least that is how I could reproduce the behaviour in menus.

@from-nibly
Copy link

from-nibly commented Feb 26, 2024 via email

ellie pushed a commit to atuinsh/docs that referenced this issue Feb 28, 2024
This updates the docs in case atuinsh/atuin#1770
lands, which fixes the referenced issue
atuinsh/atuin#1025

Since this hasn't happened yet I created this as a draft - feel free to
un-draft it!
@msminhas93
Copy link

msminhas93 commented Mar 16, 2024

Even though this PR is merged the init file has commented code for the binding with a link to this PR. Is that expected?

# The up arrow keybinding has surprising behavior in Nu, and is disabled by default.
# See https://github.com/atuinsh/atuin/issues/1025 for details

@remmycat
Copy link
Contributor

@msminhas93 The text should indeed not be generated anymore, no. Make sure that you have version 18.1.0 installed (or a newer commit).

Since the fix is part of what the init script outputs, you will not get the new version of the init file unless you manually recreate it after updating to the latest version. Note that this will overwrite manual changes you might have made to the init file.

As per the script in the Readme, assuming you haven't changed the paths:

atuin init nu | save -f ~/.local/share/atuin/init.nu

@msminhas93
Copy link

Thanks that was 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

Successfully merging a pull request may close this issue.

7 participants