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

--no-strip-ansi prints ANSI codes and garbles output #785

Closed
srithon opened this issue Dec 18, 2024 · 10 comments · Fixed by #789 · May be fixed by #799
Closed

--no-strip-ansi prints ANSI codes and garbles output #785

srithon opened this issue Dec 18, 2024 · 10 comments · Fixed by #789 · May be fixed by #799
Labels
bug Something isn't working cmd/filter
Milestone

Comments

@srithon
Copy link
Contributor

srithon commented Dec 18, 2024

This flag was introduced in PR #784, after a commit in PR #739 unconditionally stripped ANSI codes from stdin. PR #739 was made in response to issue #689; in order to prevent the display from being garbled by incorrect ANSI code handling, we opted to simply strip ANSI codes from the input entirely. The implementation in PR #784 (and my own draft implementation in PR #782) simply undo that stripping behavior without addressing the underlying issues from #689, resulting in strange outputs in certain cases. Below is a minimal example which results in garbled output on my machine in Kitty, Alacritty and st.

#!/bin/sh

green=$(tput setaf 2)
reset=$(tput sgr0)

{
  echo "Option 1"
  echo "Option 2"
  echo "${green}Option 3${reset}"
} | gum filter --no-strip-ansi

Here are recordings for each tested terminal. Note that Kitty was a lot worse than the others; interestingly, when I tried using asciinema to record Kitty's output, the recording showed the same 2m result as the other terminals; to show what it actually looked like on-screen I had to use a screen recording.

  1. Kitty
  2. Alacritty
  3. st
@caarlos0
Copy link
Member

caarlos0 commented Jan 6, 2025

oh, I see, lemme fix that!

@caarlos0 caarlos0 added bug Something isn't working cmd/filter labels Jan 6, 2025
@caarlos0
Copy link
Member

caarlos0 commented Jan 6, 2025

to resolve this properly I think we need 2 things:

  1. matching et al against the ansi-stripped string (just implemented this in a local branch)
  2. when printing the matched options, we'll need to deal with ansi sequences when highlighting as well - gonna try to implement this into x/ansi

@caarlos0
Copy link
Member

caarlos0 commented Jan 6, 2025

is this what you expect would happen?

CleanShot 2025-01-06 at 16 17 57@2x

@srithon
Copy link
Contributor Author

srithon commented Jan 7, 2025

is this what you expect would happen?

CleanShot 2025-01-06 at 16 17 57@2x

This looks perfect, thank you! I'll test it out and let you know if I find anything else.

@bashbunni bashbunni added this to the v0.15.0 milestone Jan 12, 2025
@joshmedeski
Copy link

Hey, when using my sesh, my session manager I've added the --no-strip-ansi flag and it's allowing my icons' colors to render properly, but when I type a filter it's skipping the first two letters of the matching result ("downl" only highlights "wnl").

SCR-20250113-oltg

I'm guessing it's related to this feature, any ideas what's causing it? When the icons are remove from the script the matching result works as expected.

@meowgorithm
Copy link
Member

@joshmedeski Hey! And chance you can provide a one-liner we can use to reproduce the bug you're seeing?

@joshmedeski
Copy link

joshmedeski commented Jan 13, 2025

Once you install sesh you can run this command:

sesh connect "$(sesh list -i | gum filter --limit 1 --fuzzy --no-sort --placeholder 'Pick a sesh' --prompt='⚡' --no-strip-ansi)"

The -i flag for the sesh list -i command is the most important part to recreate the issue.

And this is how I render icons.

@srithon
Copy link
Contributor Author

srithon commented Jan 13, 2025

Here's a reproducible example of the problem which doesn't require installing sesh.

  • 90 is the color code for gray
  • the second printf argument is the settings icon
printf '\033[%dm%s\033[39m %s' 90  Downloads | gum filter --no-strip-ansi

@caarlos0
Copy link
Member

oh, yeah, emoji grapheme, will fix! thanks for the reproducible btw!

@caarlos0
Copy link
Member

this should do the trick: #799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cmd/filter
Projects
None yet
5 participants