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

Input may end up on screen while running bind commands (e.g. when pasting) #7770

Closed
faho opened this issue Mar 3, 2021 · 5 comments
Closed
Milestone

Comments

@faho
Copy link
Member

faho commented Mar 3, 2021

As reported by @trixnz on gitter.

The fix for #2114 (5f16a29) made it so any fishscript run via bind commands runs with the terminal modes for external commands.

Those modes also include "echo", which means that, if input is received while a bind command is running, it may end up on screen.

This can cause the ending sequence for bracketed paste to appear. For some reason I've only been able to reproduce this on Ubuntu (20.04) on WSL2, which also happens to be quite sluggish, so it's possible there's a WSL interaction or timing exacerbating this. Neither Debian on WSL 1 nor Arch on bare metal exhibit the problem.

I'm not entirely sure what the proper fix here is. Maybe only donating the terminal for external commands, given that our builtins don't actually care about it?

@faho faho added the regression label Mar 3, 2021
@faho faho added this to the fish 3.2.1 milestone Mar 3, 2021
@vieira
Copy link

vieira commented Mar 4, 2021

Hello @faho,

Would this have anything to do with the following issue: from time to time when I paste something some escape sequences (not included in the copied text) appear in the middle of what I paste. If I wait a bit and paste again after a while it appears to go fine for a while and then starts happening again.

I attached a video that shows the issue as I am not sure I am explaining it properly.

Never happened to me before 3.2.0. I'm on macOS Catalina (10.15.7).

Screen.Recording.2021-03-04.at.22.24.09.mov

@faho faho changed the title Input may end up on screen while running bind commands Input may end up on screen while running bind commands (e.g. when pasting) Mar 5, 2021
@faho
Copy link
Member Author

faho commented Mar 5, 2021

Yes, that's the bracketed paste sequence.

@ridiculousfish Unless you have a better idea, I would revert 5f16a29 for 3.2.1. It seems the issue it fixes is much rarer than the one it introduces, and the proper fix appears to be a bit more complicated.

@faho faho closed this as completed in fd93559 Mar 10, 2021
@faho
Copy link
Member Author

faho commented Mar 10, 2021

Alright, I've reverted it.

@ridiculousfish
Copy link
Member

Sorry, I am behind on my notifications - I will see if I can fix it properly

@ridiculousfish
Copy link
Member

Really glad we caught this, thank you for reporting it. A simple way to reproduce it is bind q 'sleep 1', then when you type q and more text, the text will be wrongly echoed.

I think we can "thread this needle" by effectively calling term_steal but NOT term_donate. That is, if you run an ordinary command which does not modify tty modes (like /bin/echo) then we will retain shell modes. If you run a fancy command which does modify tty modes - like vim from #2114 - then it may modify the modes, and on return we will restore them. I will give it a try.

ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Mar 11, 2021
Restore terminal modes after running key bindings with external commands

This concerns the behavior when running an external command from a key
binding. The history is:

Prior to 5f16a29, fish would run these external commands in shell
modes. This meant that fish would pick up any tty changes from external
commands (see fish-shell#2114).

After 5f16a29, fish would save and restore its shell modes around
these external commands. This introduced a regression where anything the
user typed while a bound external command was executing would be echoed,
because external command mode has ECHO set in c_lflag. (This can be
reproed easily with `bind -q 'sleep 1'` and then pressing q and typing).
So 5f16a29 was reverted in fd93559.

This commit partially reverts fd93559. It has it both ways: external
commands are launched with shell modes, and also shell modes are restored
after the external command completes. This supports commands which muck
with the tty, but does not trigger ECHO mode.

Fixes fish-shell#7770. Fixes fish-shell#2114 (for the third time!)

This partially reverts commit fd93559.
ridiculousfish added a commit that referenced this issue Mar 11, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants