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

Add support for Alacritty preview in ANSI Format #3822

Closed
wants to merge 1 commit into from

Conversation

jeeftor
Copy link

@jeeftor jeeftor commented May 29, 2024

Change

Modification to fzf-preview.sh to support Alacritty which doesn't have sixel support

image
  • Alacritty black bg
  • Wezterm Blue bg

Alacritty doesn't support SIXELS but chafa can still work

Comment on lines +60 to +65
# Alacritty doesn't support sixels
if [[ $ALACRITTY_WINDOW_ID ]]; then
chafa --colors 256 -s "$dim" "$file"
else
chafa -f sixel -s "$dim" "$file"
fi
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The man page of chafa states that it auto-detects the right format.

OUTPUT ENCODING
       -f, --format format
           Set output format; one of [iterm, kitty, sixels, symbols]. The
           default is iterm, kitty or sixels if the connected terminal
           supports one of these, falling back to symbols ("ANSI art")
           otherwise.

Which means we can just drop -f sixel instead?

Suggested change
# Alacritty doesn't support sixels
if [[ $ALACRITTY_WINDOW_ID ]]; then
chafa --colors 256 -s "$dim" "$file"
else
chafa -f sixel -s "$dim" "$file"
fi
chafa -s "$dim" "$file"

Hmm, no, chafa fails to detect it's on iTerm2 and doesn't render image nicely on it. The issue is fixed on master, but no releases yet.

hpjansson/chafa#196


I don't use alacritty, but I believe it's capable of showing more than 256 colors. So why don't we just drop --colors 256?

If left unspecified, an optimal default will be chosen based on the current environment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give this a test ... on vacation for a week but have some time today to verify

Copy link
Author

@jeeftor jeeftor Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wez / Alacritty

image

I can confirm this ... on iterm2 it shows up as ansi/

Iterm2

image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have the new chafa version with the fix, I think we can just drop -f sixel as suggested above.

@junegunn junegunn closed this in d9c028c Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants