Fish-shell: 2.4b1 prints escape characters before and after every process

[zanchey]

By private email, a tester reports:

Today I upgraded my fish shell to version 2.4b1.1 (using default apt-get upgrade). Apparently new version contains some unusual bug, every fish command is preceded by string [1 q and followed by [1 q, see attached screenshot.

Downgrading to 2.3.1 fixes my problem.
I encounter this problem regardless of my custom config used (I moved ~/.config/fish/config.fish to other directory, restarted shell, problem was still there).

[faho]

That's from __fish_cursor_xterm - echo -en "\e[$shape q" with $shape = 2 (block).

$TERM is set to xterm(-256color), but the terminal doesn't seem to support this. Which is it?

[ridiculousfish]

I can repro on CentOS5. Bisecting...

[faho]

@ridiculousfish: No need to bisect. It's gonna show the commit where I turned the vi-cursor on by default.

The question is how to detect this particular term and that it will bork this escape.

[ridiculousfish]

Bisects to cac3b239e3cfef658ae3d99a6c11602f76479ac8 as faho predicted

[ridiculousfish]

From my investigation, the cursor shape sequence is known as DECSCUSR and per Stéphane Chazelas "is supported since patch 282". We can detect this via $XTERM_VERSION.

$XTERM_VERSION has a value like XTerm(282) which is sort of annoying to check, but I think this is the right approach if we want this on by default. I verified in my repro case, the variable was XTerm(215).

[faho]

This should be fixed by the commit I just pushed. I verified it does not fail in konsole (since that does not have $XTERM_VERSION) or xterm 327. It does with "XTerm(280)" and invalid values for $XTERM_VERSION.

[ridiculousfish]

Unfortunately this isn't fixed on CentOS5 at least. The issue there is that $TERM is xterm, but $XTERM_VERSION is unset.

[floam]

Any chance they are betraying indication of support for this via terminfo somehow, in any of the extensions? infocmp -x?

[faho]

The issue there is that $TERM is xterm, but $XTERM_VERSION is unset.

$*§%$@! That means using $TERM to detect it is broken. We cannot do this unless we expect people on ancient OSs to manually disable it, which doesn't seem friendly.

[floam]

Possibly you could still detect it by trying the escape, and see if it moved the cursor... if it did erase that and never try it again.

[ridiculousfish]

@floam from brief testing it looks like infocmp -x has an entry Ss= if and only if DECSCUSR is set.

@faho what's the issue with only enabling it for $TERM=xterm and when $XTERM_VERSION is set (and sufficient version)?

[faho]

This really is an issue for _ancient_ xterm versions - from the changelog

Patch #202 - 2005/5/2 - XFree86 4.5.99.3
[...]
add environment variables $XTERM_SHELL and $XTERM_VERSION (request by Zdenek Sekera).

Yes. 2005. XFree86.

The issue there is that $TERM is xterm

@ridiculousfish: Is it actually just "xterm"?

If so, we might be able to get away with blacklisting $TERM = xterm, and reading $XTERM_VERSION for "xterm-256color".

Btw "DECSCUSR" was added in 252, in 2009, not 282 in 2012 (though it was extended then).

[faho]

what's the issue with only enabling it for $TERM=xterm and when $XTERM_VERSION is set (and sufficient version)?

The issue is that everyone and their mum claims to be "xterm" (or "xterm-256color"), including iTerm, but I don't believe they set $XTERM_VERSION.

Edit: Gnome-terminal at least can accept this sequence, sets $TERM to xterm-256color but does not set $XTERM_VERSION.

[floam]

I think iTerm should be setting xterm-256color, at least newer ones. It can also set other TERMs though. Ss seems ideal to use but of course plenty of terminfo.src's are out of date and won't have it. Detecting terminal capabilities reliably is a total shit show unfortunately.

[ridiculousfish]

@ridiculousfish: Is it actually just "xterm"?

Yes, exactly that.

I like the idea of basing the decision off of environment variables. It is less precise than reading from terminfo (i.e. infocmp) but much simpler.

[floam]

As long as nothing sets xterm-256color and is unable to handle this, it seems to make sense to leave xterm out, and rely on $XTERM_VERSION, else tput Ss ... for it there. (Or just leave it out of the party entirely.)

[faho]

As long as nothing sets xterm-256color and is unable to handle this, it seems to make sense to leave xterm out, and rely on tput Ss ... for it there.

First of all, we might use the tput thing to determine if the xterm terminfo is new enough, which would correspond to an installed xterm being new enough. But we should not use it for the sequence itself. Both konsole and iTerm default to xterm-256color but use a different sequence (and choke on the xterm one).

Secondly, here's what we could get with environment variables:

• For xterms from version 202 (from 2005) to 281, read $XTERM_VERSION and refuse to change cursor
• For xterms from 252/282 (from 2009/2012), read $XTERM_VERSION and change cursor
• For terms pretending to be xterm-256color, change cursor
• For terms pretending to be xterm, refuse
• For xterms < 202 that use xterm, refuse
• For xterms < 202 that use xterm-256color, try to change cursor and end up with spurious stuff on screen

The question here is if that last case exists. Is there anything but CentOS/RHEL that is still supported and uses xterm < 202? Do they set TERM = xterm-256color?

I can't come up with something that is supported as long, and I think if they are sooooo stale that they use 11+ year old xterm, they are probably also conservative enough to distrust color support (I recall 256 color support was a big release notes item for Fedora 19 or so).

[ridiculousfish]

Looks like xterm-256color is from patch #111 which predates the cursor shape stuff. So the bad case seems possible in principle, though in practice it's hard to say.

[faho]

So the bad case seems possible in principle, though in practice it's hard to say.

Okay, I think we need tput Ss in some form. We can't use the sequence, but we should be able to use it for detection. The question is how much we want to rely on it.

[faho]

So, here's what I came up with:

    # Check hard if we are in a supporting terminal.
    #
    # Issues here are term-in-a-terms (emacs ansi-term does not support this, tmux does),
    # that we can only figure out if we are in konsole/iterm/vte via exported variables,
    # and ancient xterm versions.
    #
    # tmux defaults to $TERM = screen, but can do this if it is in a supporting terminal.
    # Unfortunately, we can only detect this via the exported variables, so we miss some cases.
    #
    # We will also miss some cases of terminal-stacking,
    # e.g. tmux started in suckless' st (no support) started in konsole.
    # But since tmux in konsole seems rather common and that case so uncommon,
    # we will just fail there.
    #
    # We use the `tput` here just to see if terminfo thinks we can change the cursor.
    # We cannot use that sequence directly as it's not the correct one for konsole and iTerm.
    if not tput Ss > /dev/null
        # Whitelist tmux...
        and not begin
            set -q TMUX
            # ...in a supporting term...
            and begin set -q KONSOLE_PROFILE_NAME
                or set -q ITERM_PROFILE
                or test "$VTE_VERSION" -gt 1910
            end
            # .. unless an unsupporting terminal has been started in tmux inside a supporting one
            and begin string match -q "screen*" -- $TERM
                or string match -q "tmux*" -- $TERM
            end
        end
        and not string match -q "konsole*" -- $TERM
        # Blacklist
        or begin
            # vte-based terms set $TERM = xterm*, but only gained support relatively recently.
            set -q VTE_VERSION
            and test "$VTE_VERSION" -le 1910
        end
        or set -q INSIDE_EMACS
        return
    end

[ridiculousfish]

Cool. If you push to master or a branch, I can give it a try on CentOS5.

[floam]

We might want to whitelist Terminal.app.

I notice that there is no Ss with the Apple-provided terminfo but if I install a new ncurses via homebrew and do this:

env TERMINFO=/usr/local/Cellar/ncurses/6.0_2/share/terminfo/ fish

.. the tput Ss sequences do change the cursor. (Among other things like dim and italics. Wish they'd update their xterm-256color entries.)

[faho]

@floam: It's the same sequence as xterm and the way to detect it is $TERM_PROGRAM = "Apple_Terminal", right? Are there older supported versions of it that truly don't support it?

[floam]

Yes for the first question. I'm not sure about which OS release added the cursor support. It might be necessary to check TERM_PROGRAM_VERSION. Not quite sure how to figure that out without any old machines I have access to.

[floam]

I can only really attest that at least with 387 (latest), it works.

[faho]

@ridiculousfish: My "cursor" branch has it.

@floam: I'd like to not add that at this point if we don't have a definitive answer. Let's keep this change to a minimum for 2.4.0 at least.

[floam]

Sure, that'd make more sense for the next release.

[ridiculousfish]

@faho not seeing your cursor branch - perhaps you have not yet pushed it?

[floam]

https://github.com/fish-shell/fish-shell/compare/master...faho:cursor

[floam]

It's a pretty inconsequential point but probably one _could_ use the sequence from the tput Ss $i itself, by providing it a -T argument with the "real" terminal, or at least one with the escape one wants.

[zanchey]

@ridiculousfish, any luck? This is the only thing blocking 2.4.0.

[ridiculousfish]

Sorry for the delay. @faho 's cursor branch indeed fixes the problem on CentOS5.

[pawelmhm]

Hello there,

I'm the guy who reported this issue via email to @zanchey is it meant to be fixed in 2.4.0-2?

Because I updated to 2.4.0.2 today and I see the problem persists, see screenshot

[floam]

Hi, no, it wasn't, it was fixed after 2.4.0 was released. You'll need to build fish from git master.

edit: Er, it was partially fixed for 2.4.0. See https://github.com/fish-shell/fish-shell/issues/3535

[pawelmhm]

do you know when there will be next release? I prefer to install from PPA rather than build from source, this way I can keep my system sane. I see now old version is no longer in PPA so I can't even downgrade to old version.

[floam]

I'm not sure, but entering this will work around the issue for you:

function fish_vi_cursor; end
funcsave fish_vi_cursor

[pawelmhm]

indeed that helps, i added this to my config.fish. Thanks @floam

From above discussion I see that it happens only in some terminals. Should I upgrade my terminal too?

[floam]

It shouldn't be necessary to enter that in your config.fish file, and might slow things down - typing it out and entering it once will suffice because the funcsave command will create a fish_vi_cursor.fish file with a no-op function in ~/.config/fish/functions.

What terminal are you using and what is the output of echo $TERM and echo $VTE_VERSION?

[pawelmhm]

> echo $TERM
xterm
> echo $VTE_VERSION
3409

[faho]

echo $VTE_VERSION
3409

WAT? That version should support this sequence! Which terminal is it, are you using any multiplexers or other kinds of term-in-a-terms?

I'll try to find the documentation as to when this was introduced again.

[pawelmhm]

Which terminal is it, are you using any multiplexers or other kinds of term-in-a-terms?

No, this is just pure and simple xterm without anything fancy. Default terminal on ubuntu 14.04.

[faho]

just pure and simple xterm without anything fancy

I don't think so - if it were xterm, then $VTE_VERSION shouldn't be defined.

[faho]

Okay, this is a gtk3 gnome-terminal with vte 0.34.9. From https://github.com/GNOME/vte/blob/master/NEWS, version 0.19.1 has

• Configurable cursor shape (block, underline, I-beam).

I have absolutely no idea why this shouldn't be able to do the sequence.

I mean it's possible that that entry is about something different, but I cannot find anything else in that file.

[floam]

Configurable cursor shape (block, underline, I-beam).

Maybe it just means that the user can choose one of those cursors in the app configuration, but it doesn't allow switching on the fly through that sequence?

[faho]

Maybe it just means that the user can choose one of those cursors in the app configuration, but it doesn't allow switching on the fly through that sequence?

It's possible, but then I just can't find a mention of this sequence in that file at all. I searched for "shape", "DECSCUSR" and just "cursor" and I cannot find anything about it. I _think_ I've searched before for that version (though I believed wrongly that it was rather old).

Anyway, what's the oldest version that we know this works with? Because mine is "4601" (i.e. 0.46.1), and I doubt that's the first version to support it. If I don't get any more info, I can just use that.

[floam]

https://bugzilla.gnome.org/show_bug.cgi?id=720821 0.40 maybe.

[pawelmhm]

this is a gtk3 gnome-terminal with vte 0.34.9. From

how did you find this out? My help -> about says GNOME Terminal 3.6.2 does this match your findings?

edit: so I guess I'm using this: https://launchpad.net/ubuntu/+source/gnome-terminal/3.6.2-0ubuntu1

[faho]

@pawelmhm: I ran Ubuntu 14.04 in a vm and just checked apt show libvte<TAB>. The gnome-terminal version isn't as important, the vte one is.

[faho]

@pawelmhm: I believe this is now fixed by not sending the sequence for VTE-based terms < 0.40.0.

[pawelmhm]

thanks will test with fish source later in the day. If I'd like to benefit from this cool new feature I'm missing out with old VTE version what should I do? Upgrade libvte?

[floam]

You'll need a newer libvte at least, not sure if that means a newer gnome-terminal as well.

You'll also need to get rid of the fish_vi_cursor.fish in your .config/fish/functions, which is forcing it disabled for you now.

[faho]

@pawelmhm: The easiest solution is to get a different terminal. Xterm should work - as in real proper xterm. Like floam, I'm not sure how interdependent the pieces are for vte and gnome-terminal, so I can't recommend that route.

[floam]

If there is a newer version of ubuntu you can dist-upgrade to, I'll hazard a guess that the newer GNOME which comes along with a newer gnome-terminal which will probably have a newer VTE, could probably get you something new enough.

[faho]

Yeah, but of course that'll pull in a whole bunch of other stuff. Though I didn't think of that since I assumed it was the latest LTS. Since 16.04 is out, that'd also be a viable solution.

[faho]

@pawelmhm: Can you confirm that acc2353 does not try to set the cursor, i.e. does not leak this escape sequence? You should just be able to take fish_vi_cursor.fish from that commit and stick it into ~/.config/fish/functions/.

[aureooms]

Works in terminator!

[warptozero]

When I have fish_vi_key_bindings insert set on my server, git gives the following error message upon fetching or pushing:

fatal: protocol error: bad line length character: 
[1 ⏎

function fish_vi_cursor; end has no influence.

Could this be related or should I start a new issue?

[faho]

@warptozero:

function fish_vi_cursor; end has no influence.

It's not enough to run that interactively - the function defines event handlers that also exist without it. Try function fish_vi_cursor; end; funcsave fish_vi_cursor.

Could this be related or should I start a new issue?

Both. This is a different kind of incompatibility, and we'll need a bunch more info on it. Most of all your OS, your locale, your terminal and what you are using to connect to that server (ssh?). That's best done in a new issue.

[warptozero]

It's not enough to run that interactively

I'm using it in the config file though.

Thanks for the response, I'll start a new issue with the info you requested.