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

fish confuses tty line driver; backspace over tab in cooked mode emits wrong number of ^Hs #2453

Closed
gnachman opened this issue Oct 2, 2015 · 11 comments
Assignees
Labels
bug Something that's not working as intended
Milestone

Comments

@gnachman
Copy link

gnachman commented Oct 2, 2015

To reproduce:

  1. Run cat
  2. Press tab
  3. Press backspace

Expected result:

The terminal emulator should receive 8 ^H control codes to move the cursor over the tab and back into the first column.

Actual result:

Two ^H codes are received. (note: this probably depends on your current directory; see below)

What's happening?

fish prints several escape sequences before running cat. Here's what I see:

^[[30m^[(B^[[m^[]0;cat  /Users/georgen^G^[[30m^[(B^[[m

This confuses the tty line driver into thinking the cursor is somewhere in the middle of the line, so it prints the wrong number of ^Hs when it tries to move the cursor back over the tab.

Solution

Print a carriage return after those escape sequences and just before running the command. Then the tty will know the cursor is in the first column and cooked mode will be happy again.

Here is a very loosely related bug which inspired this report: https://gitlab.com/gnachman/iterm2/issues/3866

@pickfire
Copy link
Contributor

pickfire commented Oct 3, 2015

It seems that it can't delete the tab.

@zanchey
Copy link
Member

zanchey commented Oct 16, 2015

Should be fixed with 13479fb - thanks!

@zanchey zanchey added this to the next-2.x milestone Oct 16, 2015
@ridiculousfish
Copy link
Member

Reverted as 036a29a pending a fix for #2499.

@faho faho reopened this Oct 21, 2015
@ridiculousfish
Copy link
Member

Probably this is a simple adjustment - basically we need to tell screen.cpp about our cursor-repositioning shenanigans.

@faho
Copy link
Member

faho commented Mar 7, 2016

I can't reproduce this anymore. @ridiculousfish: Did you simply forget to close?

@ridiculousfish
Copy link
Member

I can still reproduce in iTerm2, where this was reported against.

@krader1961
Copy link
Contributor

I can also still reproduce this. I fiddled around trying to fix this by applying @zanchey's change and adding a newline to the "abandon_line" logic in src/screen.cpp but couldn't get it to work. It looks like s_reset() is being passed screen_reset_abandon_line when that isn't appropriate (e.g., after a command like echo foo). Zsh seems to get this right so someone should look at how it does it.

P.S., The screen_reset_abandon_line_and_clear_to_end_of_screen symbol isn't used anywhere. That's suspicious.

@floam
Copy link
Member

floam commented Mar 12, 2016

@ridiculousfish ridiculousfish self-assigned this Mar 18, 2016
@krader1961 krader1961 added the bug Something that's not working as intended label Apr 2, 2016
@zanchey
Copy link
Member

zanchey commented Apr 28, 2016

@ridiculousfish, did you still want to see if we could sort this out? If so I think it's worth trying to get it into 2.3b2 as it's a little risky.

@ridiculousfish
Copy link
Member

Thanks for the ping, I'll take a look

ridiculousfish added a commit that referenced this issue Apr 29, 2016
fish_title currently outputs some escaped text, which can confuse
the line driver (#2453). Issue a carriage return so the line driver
knows we are at the beginning of the line, unless we are writing
the title as part of the prompt. In that case, we may have text from
the previous command still on the line and we don't want to move the
cursor.

Fixes #2453
@ridiculousfish
Copy link
Member

Fixed in master and integration branch. I used zanchey's approach, except suppress the \r when executing it for the prompt. This avoids #2499, because the prompt is the first thing we execute after running a command, but it fixes this bug because we're going to run fish_title again right before running the next command (and then we do issue the \r).

I tried hard to write a test for this, but like window resizing, it's very resistant to automated tests. :(

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

7 participants