nushell / reedline

A feature-rich line editor - powering Nushell
https://docs.rs/reedline/
MIT License
549 stars 155 forks source link

Add Newline keybindings #854

Closed NotTheDr01ds closed 6 days ago

NotTheDr01ds commented 1 week ago

Implements #792 and also should be the proper implementation of https://github.com/nushell/nushell/pull/13606


Adds Alt+Enter and Shift+Enter as keybindings to insert a newline in Emacs and Vi-insert modes.

In the Nushell PR, @sholderbach said:

so it might be that for some Alt-Enter won't work while for others Shift-Enter or Ctrl-Enter is unreliable.

I figure there's no reason we can't just bind both so that multiple platforms and terminals can handle this gracefully. I know Alt+Enter is commonly captured by Windows Terminal, and Shift+Enter doesn't seem to be recognized on Linux (also via input listen).

If the Terminal captures one of the keybindings, then hopefully the other is available. And if not, the user can always add their own in their Nushell config.

fdncred commented 1 week ago

Like you indicated, Alt-Enter definitely doesn't work for Windows because that's a system wide hook to go full screen in whatever app you're using, namely Windows Terminal. On Windows I use Shift-Enter. On MacOS, some people wanted Alt-I, not sure if that's a vim thing or what.

I'm not opposed to multiple keybindings but if we know Alt-Enter never works on Windows, we might want to only apply it in non-Windows environments. Maybe?

NotTheDr01ds commented 1 week ago

Alt+Enter isn't a system wide binding - I use it in Nushell in Windows Terminal personally. It's bound by default in Windows Terminal, but it's also easy to unbind it.

Other terminals in Windows probably have different default keybindings.

fdncred commented 1 week ago

Ya, I think you're right about the system level aspect but Alt-Enter does the same thing in WT, cmd.exe, alacritty.exe, wezterm.exe, yori.exe, tabby.exe. There may be some terminal app on Windows that doesn't use it, but I haven't found it yet.

NotTheDr01ds commented 1 week ago

There may be some terminal app on Windows that doesn't use it, but I haven't found it yet.

Interesting - I guess us old-time Fish shell users have just gotten used to unbinding it, since Fish only provides Alt+Enter.

I'm not opposed to multiple keybindings but if we know Alt-Enter never works on Windows, we might want to only apply it in non-Windows environments. Maybe?

The question is - Why disallow it? Binding it in Reedline/Nushell is never going to cause any problems1. That's why I think we should just go ahead and provide multiple options.

1 AFAIK, keybindings are always handled in the following order (hand-waving over a few, like hardware):

  1. If the OS or window-manager has the keybinding, it will act on it and (typically) not pass it on further. If it is not handled, then it gets passed to the ...
  2. Terminal, which if it has the keybinding, will act on it and (typically) not pass ito n further. If it is not handled, then it gets passed to the ...
  3. Application/shell/line-editor

So, since Reedline will only get the Alt+Enter event if it is unbound at the higher-levels, it shouldn't matter if we just leave the keybinding in the code for all platforms, right?

NotTheDr01ds commented 1 week ago

Do we want a third-fallback like Ctrl+J (Emacs)? Someone mentioned using that, and again, there doesn't seem to be any harm in binding it. Users who want to bind any of these to another function are free to do so in Nushell.

fdncred commented 1 week ago

I'm good with double binding it. It's not worth all this conversation. We can always change it if it gets in the way.

fdncred commented 6 days ago

Thanks

sholderbach commented 6 days ago

Ctrl-J should probably be Enter as folks have been complainig that their tools that send \n (that is Ctrl-J in ansi) don't trigger a run as the other shells do that also bind it. (The Enter key itself gets represented by \r/Ctrl-M)

See #832 (wasn't merged by me because of the hardcoding but should be fine as a soft binding like you did)

fdncred commented 6 days ago

oops, should we revert?

sholderbach commented 6 days ago

Nope small fix