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

Add Newline keybindings #854

Merged
merged 2 commits into from
Nov 20, 2024
Merged

Conversation

NotTheDr01ds
Copy link
Contributor

@NotTheDr01ds NotTheDr01ds commented Nov 19, 2024

Implements #792 and also should be the proper implementation of nushell/nushell#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
Copy link
Collaborator

fdncred commented Nov 19, 2024

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
Copy link
Contributor Author

NotTheDr01ds commented Nov 19, 2024

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
Copy link
Collaborator

fdncred commented Nov 19, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

NotTheDr01ds commented Nov 19, 2024

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.

  • Pro: Alt+Enter is bound-by-default on Windows Terminal and Shift+Enter doesn't get differentiated from a plain Enter on Nushell Linux, so WSL Nushell users will need another alternative (or unbind Alt+Enter as I have).
  • Con: Too much of a good thing.

@fdncred
Copy link
Collaborator

fdncred commented Nov 19, 2024

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 fdncred merged commit 7975013 into nushell:main Nov 20, 2024
6 checks passed
@fdncred
Copy link
Collaborator

fdncred commented Nov 20, 2024

Thanks

@sholderbach
Copy link
Member

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
Copy link
Collaborator

fdncred commented Nov 20, 2024

oops, should we revert?

@sholderbach
Copy link
Member

Nope small fix

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 this pull request may close these issues.

3 participants