-
Notifications
You must be signed in to change notification settings - Fork 328
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
Sixel support for preview panel #840
Conversation
…ing run inside it
Whoa, this is advancing really fast! Glad to see that! Anxious to have sixels working here ç.ç Please keep up the good work, won't be unnoticed o/ |
Showing the previews works for me with
And my cleaner script is
Excited for this patch! |
chafa -f sixel ftw, the universal solution. ANSI for non sixel support and chafa -f sixel for foot users xD |
Hey @horriblename, is it possible to make the converted image cache permanent and read it from there or this is 100% dependent on the preview script? I have a very (very, very..) slow computer and it takes some (noticeable) time to convert the image to sixels. I bet i am not the only one xD Thanks again for your work mate |
I noticed that loading from cache takes too long as well, I will work on that later. Clearing the image (efficiently) takes priority now, if I can't do that there's no point going on lol. Bringing up command prompts (via |
Konsole behaves wierdly with sixels, writing text over images just writes on top of the image, whereas wlterm and foot overwrites the cells |
I've settled on this:
This doesn't work on Konsole because their implementation is bugged and the only way to delete the image there is to redraw the entire screen |
" Clearing the image (efficiently) takes priority now, if I can't do that there's no point going on lol." |
detecting sixels in preview script now fills corresponding area with braille blank `\u2800` and saves sixel to reg.sixels
printReg doesn't process sixels directly anymore
I have a bunch of free time lately so no worries haha That said, it's working mostly flawlessly now, still a few bugs here and there. If anyone wants to test it keep these in mind:
#!/bin/sh
case "$(printf "%s\n" "$1" | awk '{print tolower($0)}')" in
*.bmp|*.jpg|*.jpeg|*.png|*.xpm|*.webp|*.gif)
chafa "$1" -f sixel -s "$(($2-2))x$(($3-2))" | sed 's/#/\n#/g'
;;
*) cat "$1";;
esac
exit 0
That's all, and leave a comment if you encounter a bug Bugs
|
Hey @horriblename , sorry if this is a dumb question but why don't just move the preview script to something integrated with LF itself and just use a config file? Wouldn't this make it more stable and have less bugs? Preview scripts at this point are just more of the same: text for texts, image for images, videos and pdfs. I know this isn't ranger but scope.sh is a very convenient way to make chages to preview. Why the preview script like like lfimg or sptv are really necessary at this point? One thing at a time, i know, but what is the limitation? |
Not really a question that I can answer for others, but I'm guessing that the preview script makes it more customizable and flexible. I use
I'm not sure what bugs you're talking about, but my experience with lf was fairly smooth, I used the lfimg scripts for a long time and I didn't even know how it works behind the scene, up till weeks ago when I switched to wayland and ueberzug didn't work. If you're talking about the bugs from this PR, (don't to be frank, if this PR is accepted, it would simplify my preview script a LOT, but that's still not enough reason to eliminate scripts, as they work just fine, plus other people might still find the extra bit of flexibility useful. tl;dr preview scripts are more flexible and they work just fine, so there's no reason to eliminate them |
To reiterate, you treat a sixel sequence as you would any other form of text, give it to lf and it'll handle the rest. No external cleaner scripts, no exiting with signal 1, nothing. It's a normal text and nothing more. |
… between different images
Things still missing:
If anyone wants to help with any of these, please feel free. @gokcehan Hope you can review this, thanks! |
I'm looking to switch from ranger to lf due to the portable binaries, so it is great seeing here the sixel support being worked on. One thing I noticed with sixel in ranger is that resizing the images make a huge difference in performance, and the UI is much more responsive. BTW, my terminal emulator is foot. |
If you want to limit the image size you can do it in the preview script: w=$2
# set w to 40 if greater than 40
["$2" -gt 40 ] && w=40
h=$3
["$3" -gt 30 ] && h=30
# if you're using chafa:
chafa "$1" -f sixel -s "${w}x${h}" | sed 's/#/\n#/g' I'm not very good at bash so there might be a better way |
@tinywrkb there are also some optimization options in chafa that might help you:
|
By size I meant size as in memory, the resolution would stay the same, and I don't think this is going to help much tbh, just something you can consider |
@horriblename Thanks for working on this and sorry for not getting back to you earlier. As you might be aware, I have mostly been ignoring anything but the simplest patches in lf as I have burned out and also I was busy irl. But I felt like dropping a comment for this patch as it seems to be getting popular. Personally I don't think it is a good idea to directly handle terminal with non-standard escape codes in general as I feel like such things belong to the terminal handling library to be portable (e.g. tcell). We have had issues with such implementations in the past so I don't want to take responsibility for it. I should say that I think it is perfectly feasible to maintain this patch in your own fork as changes in this fork are rare these days. I'm aware there are many people interested in sixels and I'm sure they would be happy to see this implementation. In the past, I suggested some other contributors to create a wiki page for actively maintained forks with patches not included in this fork. Instead, we could also add a forks section in the readme for better visibility. |
@horriblename How do I build your fork? I am interested in testing it out, but not sure how the build instructions differ from the main project. |
@Guiltybyte I just updated the instructions on my repo
|
@gokcehan yeah I understand that it's really hacky, thanks for taking a look though. I'll maintain a fork moving forward
Alright, I'll submit a separate pr when I get around to doing that :) I'm closing this PR, any further developments will be done on the fork |
see issue
This PR aims to add support for sixel graphics
It does this by printing directly to stdout, without going through tcell
- [ ] check for sixel support- [ ] manually reset DECSDM(not important)ESC[?80l
on app initui
ui.sixels
SetContent
cells in the image to braille space\u2800
Problems
bufio.Scanner: token too long
sed 's/#/\n#/g'
).Commands (:
etc.) lags a lot and cursor shows at the wrong location