feat:Bind Ctrl-v to visually editing the selected command, exactly as per fc

[wolf]

My old workflow was:

• select the command I want
• hit tab, so now it is on the command line, atuin is gone, nothing has been executed
• I use Vim mode on the command line, so Escape puts me in normal mode
• vv launches my favorite editor
• edit, write, and quit which makes the updated command execute

My new workflow follows in the footsteps of fc:

• select the command I want
• hit Ctrl-v, launches my favorite editor on the text of that command
• edit, write, and quit which makes the updated command execute

I tried to make this as much like fc as I could because I feel like atuin hides and replaces fc. Emacs-mode on the command line requires two control keys to jump into the editor. Vim-mode needs you to get into normal mode, that is NOT inserting, then hit v. Everything else in atuin is a single key, and when stepping through the history list, you're already not in insert mode. Like fc, I look for editors in this order:

• the value of $FCEDIT (I know this feels weird, but you are "fixing a command")
• the value of $EDITOR
• the value of $VISUAL
• or else vim

I am very open to discussion. I want to know if this is a desirable feature, the right implementation, in the right place, with the right names. The functionality is more important to me than exactly how I implemented it. And atuin staying true to itself is even more important than that. I posted in the CLI channel on the Discord server describing this.

Checks

• I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
• I have checked that there are no existing pull requests for the same thing

[jeremycline]

Not a maintainer here, just procrastinating on other work I should be doing, so take this suggestion with a grain of salt.

You might consider a different fallback here. Some minimal Fedora derivatives come without EDITOR set and with vim-minimal installed which doesn't provide /usr/bin/vim, just /usr/bin/vi. Debian looks like it relies on the alternatives system so calling editor will get you the default.

You could probe to see if there's a /usr/bin/editor and then fall back to vi, or you could turn it into an error instructing the user to set the correct environment variables. That'd look something like this (which I've not tested at all):

        .or_else(|_| std::env::var("VISUAL"))
        .ok_or_else(|| eyre::eyre!("To edit commands, set the EDITOR environment variable to your preferred editor"))

[wolf]

Using vi instead of vim sounds right. In systems with vim, vi is usually just another name for vim. A thing I was thinking, related to this, is that perhaps config.toml could optionally specify an editor, and that would be something to check for first. Reporting an error seems like user-friendly behavior. Is that the general way atuin handles situations like this? With respect to the exact editors used, I feel like I would want to probe for vi and issue the error only after that. That way I maintain the same sequence used by fc (modulo any change in config.toml). But maybe my desire to emulate fc is not appropriate.

When you say /usr/bin/editor, you're just using editor as a stand-in, right? Just double-checking. You're asking that I make sure no matter what they have specified, even in one of those variables, that their choice actually exists?

Finally, it occurs to me that this is a feature you might want to turn on/off in config.toml. It seems unlikely, at least to me, but it's a thought. I feel like "off" means "just don't touch Ctrl-v". Hmm. And also I should look to see any place else that mentions keys and consequences, e.g., help text, documentation, etc. A PR that adds a key should update such things as well.

It wasn't immediately obvious to me how to write a test (or a couple of tests) in the test section of interactive.rs since running a visual editor requires interactive input. This is on my mind.

[wolf]

Oh I see, editor is not just a place-holder. On some OSs, that's an actual thing. Actually probing (as you suggest) for the existence of /usr/bin/editor sounds like good behavior. I wonder if we also need to check the actual OS, so that we know editor is appropriate. My initial thought is no.

[wolf]

I think I know how to do this as your are suggesting. Making sure specific editors exist feels like the code will be bulky compared to what is currently in this PR. Looking specifically for /usr/bin/editor; and maybe adding a key to config.toml. But I also think these are good ideas, regardless of whether you are a maintainer or not.

I will explore implementing this, and update this PR for further review.

[jeremycline]

Indeed, I wouldn't enumerate nvim, emacs, etc. As long as the probe for /usr/bin/editor happens after checking the EDITOR environment variable I don't think you need to worry about checking the OS, either, although that might be necessary if some distribution ships something else as /usr/bin/editor and also don't set an EDITOR environment variable.

Personally, I feel like we can get away without a config.toml setting for the editor since the EDITOR environment variable is so widely used and I have a hard time imagining someone who wants to use nvim for git commit messages, but emacs for editing commands in Atuin.

For whatever it's worth, I lean slightly towards presenting the user with an error over defaulting to vi if nothing is set. I expect nearly every Atuin user knows how to use vi, but for any that don't it would be a shocking experience to accidentally end up there. I don't have a great sense of whether or not that matches the general Atuin style though.

There's a table of shortcuts, but I'm not sure where the source for that document is.

Finally, for testing maybe you could set EDITOR to a shell script that writes a string to the given tempfile so it's not interactive? I've not thought hard about that, though.

[wolf]

Oh! A script instead of an actual editor is a great idea! I don't know why I didn't think of that. That's exactly how I test all my dotfile shell functions.