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

Space in prompt after ctrl-D while running command #8871

Closed
tomsmeding opened this issue Apr 13, 2022 · 7 comments
Closed

Space in prompt after ctrl-D while running command #8871

tomsmeding opened this issue Apr 13, 2022 · 7 comments
Labels
regression Something that used to work, but was broken, especially between releases
Milestone

Comments

@tomsmeding
Copy link
Contributor

I normally use ctrl-D to exit fish; this works only on an empty prompt, as expected. However, sometimes I am too early with hitting ctrl-D, and enter it already while a command is still running. (My intention is then to exit the shell directly after the command; incidentally, this is often git push.) In this situation, not only does the ctrl-D not result in closing the shell, it also puts a single space character in the prompt buffer so that I have to type another backspace and then another ctrl-D to actually exit the shell.

I'm not sure whether it is a good idea for the shell to exit if a ctrl-D is already in the buffer when a command exits; exiting the shell is a somewhat destructive action. However, I do think that the space character should not be there.

Bash does not have this behaviour; the prompt input is empty after a command exits.

Recording: https://asciinema.org/a/486924 (Easy reproduction: run sleep 2, press ctrl-D while the command runs, and observe a space character in the prompt input.)

System:

  • fish --version -> 3.3.1
  • uname -a = Linux arrow 5.16.16-arch1-1 #1 SMP PREEMPT Mon, 21 Mar 2022 22:59:40 +0000 x86_64 GNU/Linux
    • (Arch Linux)
  • $TERM = tmux-256color
  • sh -c 'env HOME=$(mktemp -d) fish' has the same behaviour, as shown in the asciinema recording.
@krobelus krobelus added bug Something that's not working as intended regression Something that used to work, but was broken, especially between releases and removed bug Something that's not working as intended labels Apr 13, 2022
@krobelus
Copy link
Member

bisects to 8ddd512 (Refine when we expand abbreviations, 2020-03-19), which added

# Ctrl-space inserts space without expanding abbrs
bind -k nul 'commandline -i " "'

so a workaround is bind -k nul self-insert

@mqudsi
Copy link
Contributor

mqudsi commented May 10, 2022

Where does the translation of ^D to ^@ take place, though?

I think we can fix this by using abbr -q first?

bind -k nul "abbr -q (commandline -t) && commandline -i ' '"

Actually, perhaps it's better ergonomics to always emit a space for ^@ unless the commandline is altogether empty?

@krobelus
Copy link
Member

Where does the translation of ^D to ^@ take place, though?

no idea :(

Actually, perhaps it's better ergonomics to always emit a space for ^@ unless the commandline is altogether empty?

good idea, let's do that for now (it's simpler hence more predictable)

@faho
Copy link
Member

faho commented May 11, 2022

Where does the translation of ^D to ^@ take place, though?

My guess: This isn't about reading ctrl-d, it's about reading EOF (do stty eof '^g' and it starts happening for ctrl-g instead). EOF is a read of 0 bytes. If that ends up in a c-string you have the terminating NUL.

So we get an empty read before we reset the terminal modes to our internal ones, and we're missing a check for the empty string somewhere. It's probably okay to ignore it - we'd otherwise have to translate it back from the EOF char, and that depends on the current exact terminal modes and that's racy.

@krobelus krobelus added this to the fish 3.5.0 milestone May 11, 2022
@tomsmeding
Copy link
Contributor Author

Thanks a lot! That workaround should indeed work this situation, I think. (Hard to test on an old fish that doesn't support $() yet.) Though I would also be curious whether @faho's predicted root cause is correct. :)

@krobelus
Copy link
Member

Hard to test on an old fish that doesn't support $() yet.

try

bind -k nul 'test -n (commandline | string collect) && commandline -i " "'

@tomsmeding
Copy link
Contributor Author

Ooh, a magic builtin! Binding works and fixes the issue. :)

Also I could have been less lazy and tested it on a command line without newlines...

faho pushed a commit to faho/fish-shell that referenced this issue May 19, 2022
Pressing Ctrl-D while a command is running results in a null key code in
our input queue. That key code is bound to insert a space (without expanding
abbreviations). Make it only insert a space if the commandline is non-empty,
to accommodate this use case.

This probably affects other keys as well.

Closes fish-shell#8871
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Something that used to work, but was broken, especially between releases
Projects
None yet
Development

No branches or pull requests

4 participants