Fish `nix` completion function reliably erases/writes over the shell prompt. #513

New issue

Closed

opened 2024-09-10 19:52:39 +00:00 by eddyb · 3 comments

eddyb commented

2024-09-10 19:52:39 +00:00

Describe the bug

The new progress bar (which outputs terminal escape sequences on stderr) can impact shell completion, causing e.g. fish prompts to be erased and/or written over.

I've only tested fish (the main shell I use), but I suspect other shells might also affected.

Steps To Reproduce

Run fish
Type nix (with a space at the end) and press TAB
Watch the prompt get erased

Another way to see this issue is running these commands:

$ echo -n A; complete -C 'nix ' >/dev/null; echo B
B
$ echo -n A; NIX_GET_COMPLETIONS=1 nix '' >/dev/null; echo B
B
$ echo -n A; NIX_GET_COMPLETIONS=1 nix '' >/dev/null 2>/dev/null; echo B
AB

Expected behavior

When performing completions, the progress bar shouldn't be shown on stderr, as its \r\e[K line erase sequences end up applying to the prompt (or to the A in the commands above).

Either nix should behave differently when NIX_GET_COMPLETIONS is set (i.e. turning off the progress bar), ~~or the completion script wrapping the command should redirect stderr (which is what a lot of completion scripts do when wrapping commands they don't control, but that's probably suboptimal here)~~.

(EDIT: see comment below, looks like NIX_GET_COMPLETIONS disabling the progress bar already was the case, and was accidentally bypassed by a string->enum refactor)

`nix --version` output

nix (Lix, like Nix) 2.91.0

## Describe the bug The new progress bar (which outputs terminal escape sequences on stderr) can impact shell completion, causing e.g. `fish` prompts to be erased and/or written over. I've only tested `fish` (the main shell I use), but I suspect other shells might also affected. ## Steps To Reproduce 1. Run `fish` 2. Type `nix ` (with a space at the end) and press TAB 3. Watch the prompt get erased Another way to see this issue is running these commands: ```console $ echo -n A; complete -C 'nix ' >/dev/null; echo B B $ echo -n A; NIX_GET_COMPLETIONS=1 nix '' >/dev/null; echo B B $ echo -n A; NIX_GET_COMPLETIONS=1 nix '' >/dev/null 2>/dev/null; echo B AB ``` ## Expected behavior When performing completions, the progress bar shouldn't be shown on stderr, as its `\r\e[K` line erase sequences end up applying to the prompt (or to the `A` in the commands above). Either `nix` should behave differently when `NIX_GET_COMPLETIONS` is set (i.e. turning off the progress bar), ~~or the completion script wrapping the command should redirect stderr (which is what a lot of completion scripts do when wrapping commands they don't control, but that's probably suboptimal here)~~. (**EDIT**: see comment below, looks like `NIX_GET_COMPLETIONS` disabling the progress bar already was the case, and was accidentally bypassed by a string->enum refactor) ## `nix --version` output ``` nix (Lix, like Nix) 2.91.0 ```

👍 2

eddyb added the

bug

label 2024-09-10 19:52:39 +00:00

eddyb commented

2024-09-10 20:08:58 +00:00

Author

I've come up with this hacky workaround for now:

cat /run/current-system/sw/share/fish/vendor_completions.d/nix.fish \
  | sed -E 's|(\(_nix_complete)\)|\1 2>/dev/null)|' > ~/.config/fish/completions/nix.fish

However, @solson brought to my attention that this was more accidental than I thought:

the NIX_GET_COMPLETIONS env var does get checked in parseLogFormat:
https://git.lix.systems/lix-project/lix/src/tag/2.91.0/src/libmain/loggers.cc#L10-L11
but none of the calls to setLogFormat (other than --log-format, which runs too late) pass a string,
so parseLogFormat (which respects the env var) never gets called normally
(and even with e.g. --log-format=raw, the damage has already been done by that point)
looking at blame on the main call reveals that it recently changed:
```
-    setLogFormat("bar");
+    setLogFormat(LogFormat::bar);
```
(which accidentally bypassed the NIX_GET_COMPLETIONS env var check)

cc @alois31 @qyriad

I've come up with this hacky workaround for now: ```sh cat /run/current-system/sw/share/fish/vendor_completions.d/nix.fish \ | sed -E 's|($_nix_complete)$|\1 2>/dev/null)|' > ~/.config/fish/completions/nix.fish ``` --- _However_, @solson brought to my attention that this was more accidental than I thought: 1. the `NIX_GET_COMPLETIONS` env var does get checked in `parseLogFormat`: https://git.lix.systems/lix-project/lix/src/tag/2.91.0/src/libmain/loggers.cc#L10-L11 2. but none of [the calls to `setLogFormat`](https://git.lix.systems/lix-project/lix/search?q=setLogFormat&fuzzy=true) (other than `--log-format`, which runs too late) pass a string, so `parseLogFormat` (which respects the env var) never gets called normally (and even with e.g. `--log-format=raw`, the damage has already been done by that point) 3. looking at [blame on the `main` call](https://git.lix.systems/lix-project/lix/blame/commit/cc183fdbc14ce105a5661d646983f791978b9d5c/src/nix/main.cc#L366) reveals that [it recently changed](https://git.lix.systems/lix-project/lix/commit/0dd1d8ca1cdccfc620644a7f690ed35bcd2d1e74#diff-c36e49830b794cf381d9306d9be119cd4adc19a0): ```diff - setLogFormat("bar"); + setLogFormat(LogFormat::bar); ``` (which accidentally bypassed the `NIX_GET_COMPLETIONS` env var check) cc @alois31 @qyriad

solson commented

2024-09-10 20:17:14 +00:00

Member

I also tried using asciinema rec -c "NIX_GET_COMPLETIONS=1 nix '' >/dev/null" to see what Lix is actually printing:

[0.020591, "o", "\r\u001b[K"]
[0.021163, "o", "\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K"]

(This is asciinema's internal format with I guess timestamps, some tag, and then the captured terminal output.)

So it's apparently printing \r\e[K quite a number of times when the progress bar is enabled, which might be considered a separate issue even if the progress bar gets re-disabled for NIX_GET_COMPLETIONS. (I'm not sure.)

I also tried using `asciinema rec -c "NIX_GET_COMPLETIONS=1 nix '' >/dev/null"` to see what Lix is actually printing: ```json [0.020591, "o", "\r\u001b[K"] [0.021163, "o", "\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K\r\u001b[K"] ``` (This is asciinema's internal format with I guess timestamps, some tag, and then the captured terminal output.) So it's apparently printing `\r\e[K` quite a number of times when the progress bar is enabled, which might be considered a separate issue even if the progress bar gets re-disabled for `NIX_GET_COMPLETIONS`. (I'm not sure.)

lix-bot commented

2024-09-11 17:03:43 +00:00

Member

This issue was mentioned on Gerrit on the following CLs:

commit message in cl/1911 ("fish-completion: leave the shell prompt intact")

This issue was mentioned on Gerrit on the following CLs: * commit message in [cl/1911](https://gerrit.lix.systems/c/lix/+/1911) ("fish-completion: leave the shell prompt intact")

❤️ 2