Skip to content

Conversation

@Lillecarl
Copy link
Contributor

@Lillecarl Lillecarl commented May 7, 2025

Description

Fixes an issue where systemctl and other systemd commands completions are prefixed by ANSI color escape codes

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

Notes:

Since dependencies has changed an Nix is a bit finicky with cargoHashes and stuff I cloned 4.0.2 which I'm currently using and applied this patch which i then cherry picked to master, but considering the change is only in fish code it should be irrelevant.

It was a bit unclear from the documentation if set -lu would work without unexporting the variable for the running shell but tests on my machine keeps SYSTEMD_COLORS=true in my shell.

Last minute thing: Should i use set -lu or set -fu?

Fixes an issue where systemctl and other systemd commands completions are prefixed by ANSI color escape codes
@ridiculousfish ridiculousfish self-requested a review May 7, 2025 20:53
@ridiculousfish ridiculousfish merged commit 7a668fb into fish-shell:master May 8, 2025
7 of 8 checks passed
@ridiculousfish
Copy link
Member

Thank you!

@ridiculousfish ridiculousfish added this to the fish 4.1 milestone May 8, 2025
ridiculousfish added a commit that referenced this pull request May 8, 2025
@krobelus
Copy link
Contributor

krobelus commented May 8, 2025

nix should allow you to run cargo build, no?

It was a bit unclear from the documentation if set -lu would work without unexporting the variable

only the value in the inner scope matters:

$ set -gx SYSTEMD_COLORS true
$ set -lu SYSTEMD_COLORS
$ set -S SYSTEMD_COLORS
$SYSTEMD_COLORS: set in local scope, unexported, with 0 elements
$SYSTEMD_COLORS: set in global scope, exported, with 1 elements
$SYSTEMD_COLORS[1]: |true|
$ env | grep SYSTEMD
$

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants