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 "alacritty" to the list of TERM values of terminals supporting dynamic titles #7073

Merged
merged 1 commit into from Jun 2, 2020

Conversation

rye
Copy link
Contributor

@rye rye commented Jun 1, 2020

Description

This change fixes support for "dynamic titles" for users of the Alacritty terminal. Users with the terminfo for alacritty installed in their terminfo database would have their TERM values set to alacritty by default (as opposed to xterm-256color), which would break fish-shell's understanding.

Prior to this change, the does_term_support_setting_title function would return false because the term value alacritty matches the pattern tty (causing this line to return false). Other values of TERM not including the substrings tty or /vc/ and not matching any of the prior patterns would instead be interpreted as having support for dynamic titles, but in this case because the string alacritty contains "tty", it would get interpreted as not having support for dynamic titles.

I'd be happy to make any additional changes requested!

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

This change is necessary to fix dynamic titles for the Alacritty
terminal.  We do this by simply adding the (wchar_t *) literal
L"alacritty" to the end of the title_terms array.  This variable is
ultimately used in the subsequent function
does_term_support_setting_title (dtsst) for the purposes of whitelisting
certain terminals.

If an Alacritty user does not have the terminfo for alacritty present in
their terminfo database, Alacritty sets the TERM variable to
"xterm-256color", but if the terminfo for Alacritty is present, TERM is
instead set to "alacritty".

Prior to this change, none of the "fallback patterns" in the dtsst
function (which is used to ultimately decide whether or not a given
value of the TERM environment variable is supported) would apply to a
value of "alacritty".  Ordinarily, the dtsst function would return true
if nothing matches, but one of the final checks involves testing the
result of ttyname_r to see if it contains the substring "tty", which
causes dtsst to return false.  In the case where TERM="alacritty", this
is erroneous, because Alacritty does, indeed, support changing its title
and will also silently ignore attempts to change the title if that
behavior has been disabled by the user [1].

The changed file, src/env_dispatch.cpp, was reformatted by clang-format
in accordance with the documented procedures for contributors.

Signed-off-by: Kristofer Rye <kristofer.rye@gmail.com>

[1]: https://github.com/alacritty/alacritty/blob/1dacc99183373bffa3ba287aa3241f3b1da67016/alacritty_terminal/src/term/mod.rs#L896-L900
@rye
Copy link
Contributor Author

rye commented Jun 1, 2020

I'm not sure if this PR warrants an addition to CHANGELOG.rst, but I'll be happy to make a note if it does.

@ridiculousfish ridiculousfish merged commit 146ec61 into fish-shell:master Jun 2, 2020
@ridiculousfish
Copy link
Member

Thank you!

@ridiculousfish ridiculousfish added this to the fish 3.2.0 milestone Jun 2, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants