Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add "alacritty" to the list of TERM values of terminals supporting dynamic titles#7073

Merged
ridiculousfish merged 1 commit into
fish-shell:masterfish-shell/fish-shell:masterfrom
rye:alacritty-dynamic-titlerye/fish-shell:alacritty-dynamic-titleCopy head branch name to clipboard
Jun 2, 2020
Merged

Add "alacritty" to the list of TERM values of terminals supporting dynamic titles#7073
ridiculousfish merged 1 commit into
fish-shell:masterfish-shell/fish-shell:masterfrom
rye:alacritty-dynamic-titlerye/fish-shell:alacritty-dynamic-titleCopy head branch name to clipboard

Conversation

@rye

@rye rye commented Jun 1, 2020

Copy link
Copy Markdown
Contributor

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

rye commented Jun 1, 2020

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.