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

[Error] piping certain output into an empty Fish script results in various Fish errors #1292

Closed
yorickpeterse opened this issue Mar 18, 2024 · 3 comments
Labels
bug Something isn't working wait-on-user waiting for a reply

Comments

@yorickpeterse
Copy link

Describe the bug

After upgrading Distrobox, I ran into an issue where git log -1 produced errors such as the following:

test: Expected a combining operator like '-a' at index 3
     Rename <leader>ps to <leader>pweterse.com>c1799197d70^[[m^[[1;33m (^[[m^[[1;36mHEAD^[[m^[[1;33m -> ^[[m^[[1;32mmain^[[m^[[1;33m, ^[[m^[[1;31morigin/main^[[m^[[1;33m)^[[m
 ^
/etc/fish/conf.d/distrobox_config.fish (line 16):
    test -z $XAUTHLOCALHOSTNAME ; and set -e XAUTHLOCALHOSTNAME
    ^
from sourcing file /etc/fish/conf.d/distrobox_config.fish
        called on line 248 of file /usr/share/fish/config.fish
from sourcing file /usr/share/fish/config.fish
        called during startup

After some digging, I found this is due to how I have pager set up in Git:

[core]
    pager = git-diff-highlight | less -RS

After disabling that and doing some further testing, I can reproduce this by simply doing git log -1 | fish -c ''.

Based on this, it appears that Distrobox is passing STDIN input to some tests which can't handle the input.

To Reproduce

Make sure your host's shell is Fish such that the container also uses Fish. Next, create /tmp/output.txt with the following contents:

commit 1dbf78218e96aa10bfcfa68f6226bc1799197d70
Author: Yorick Peterse <git@yorickpeterse.com>
Date:   18 Mar 2024 03:15 +0100

    Rename <leader>ps to <leader>pw

commit 5ea8c2cbd6cd2f2fd8018845bbdf2a0865b33e70
Author: Yorick Peterse <git@yorickpeterse.com>
Date:   18 Mar 2024 03:14 +0100

    Add session management setup

commit 3d15f4ff3eb4d2475cc25f3d675d3feed7fac8cc
Author: Yorick Peterse <git@yorickpeterse.com>
Date:   18 Mar 2024 02:00 +0100

    Remove workspace logic
    
    Using sessions is a better approach.

commit a3f577b076db07d90d72d6e37b727f03cca54116
Author: Yorick Peterse <git@yorickpeterse.com>
Date:   15 Mar 2024 22:57 +0100

    Use histogram and linematch diffs
    
    Histogram diffs are supposed to be better for code compared to
    "patience", and the "linematch" option of NeoVim should help clear up
    certain diffs a bit.

commit 1414746fb34d7fcf24746a69a68a4a6ef3551314
Author: Yorick Peterse <git@yorickpeterse.com>
Date:   14 Mar 2024 22:07 +0100

    Revert "Remove gs/ge mappings"
    
    This reverts commit fa1f56f81bb82765601c75f77fee7f8ca368c9f4.
  1. mkdir /tmp/fedora
  2. distrobox create --image fedora:latest --name fedora --home /tmp/fedora --pull --no-entry
  3. distrobox enter fedora
  4. cat /tmp/output.txt | fish -c ''

Expected behavior

No errors, and Distrobox doesn't blindly pass STDIN around.

Logs

Doesn't seem relevant?

Desktop (please complete the following information):

  • Are you using podman, docker or lilipod?: podman
  • Which version or podman, docker or lilipod?: 4.9.3
  • Which version of distrobox?: 1.7.0.1
  • Which host distribution?: Fedora 39
  • How did you install distrobox?: Fedora repositories
@yorickpeterse yorickpeterse added the bug Something isn't working label Mar 18, 2024
@scottames
Copy link

scottames commented Mar 18, 2024

Can confirm, running into this as well. Though it's at all prompts based on the way fish handles sourcing config.

Same behavior, slightly different context. I'll try to find what's unique about my setup that causes this. The specific error I get:

❯ test: Expected a combining operator like '-a' at index 3
 pipestatusmode insert
 ^
/etc/fish/conf.d/distrobox_config.fish (line 16):
    test -z $XAUTHLOCALHOSTNAME ; and set -e XAUTHLOCALHOSTNAME
    ^
from sourcing file /etc/fish/conf.d/distrobox_config.fish
        called on line 248 of file /usr/share/fish/config.fish
from sourcing file /usr/share/fish/config.fish
        called during startup
test: Expected a combining operator like '-a' at index 3
 pipestatusmode insert
 ^
/etc/fish/conf.d/distrobox_config.fish (line 16):
    test -z $XAUTHLOCALHOSTNAME ; and set -e XAUTHLOCALHOSTNAME
    ^
from sourcing file /etc/fish/conf.d/distrobox_config.fish
        called on line 248 of file /usr/share/fish/config.fish
from sourcing file /usr/share/fish/config.fish
        called during startup

My git config has pager set to: pager = delta

  • Removing this entirely does not change the behavior however. I get the same errors on each prompt.
  • Setting XAUTHLOCALHOSTNAME prior to entering the distrobox resolves the issue for me. Not sure why line 14 handles the empty variable fine but line 16 does not.

yorickpeterse added a commit to yorickpeterse/dotfiles that referenced this issue Mar 18, 2024
In light of 89luca89/distrobox#1292, and some
other grievances I have with Distrobox (basically it doing way more than
necessary, and all using piles of shell code), this commit refactors my
dev setup to use Toolbox instead.

This commit also switches it back to Fedora, after Arch decided to kick
out llvm15 from the repositories, requiring you to build it using the
AUR. While I'm planning to update Inko to a newer version of LLVM, I
prefer to have a more stable development environment, which Fedora is
ideal for.
@89luca89
Copy link
Owner

Thanks @yorickpeterse for the report!

I'm not familiar with fish, the file /etc/fish/conf.d/distrobox_config.fish is only there to be sourced by login shell
Its strange that is sourced by each invocation of the shell itself (especially if non interactive)

If there is a way to detect login shell (or not) we can add it to the config file in order to skip for non-login shells

@89luca89 89luca89 added the wait-on-user waiting for a reply label Mar 25, 2024
@yorickpeterse
Copy link
Author

@89luca89 Fish has a status function that can be used for checking if a session is interactive, whether it's a login shell, etc. For example, status is-interactive returns 0 for an interactive session, 1 otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wait-on-user waiting for a reply
Projects
None yet
Development

No branches or pull requests

3 participants