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

Problems with st-sixel #32

Open
ghost opened this issue Apr 24, 2020 · 11 comments
Open

Problems with st-sixel #32

ghost opened this issue Apr 24, 2020 · 11 comments

Comments

@ghost
Copy link

ghost commented Apr 24, 2020

Hi

I'm using st-sixel (https://github.com/galatolofederico/st-sixel) which lsix doesn't realise has sixel capabilities. I got lsix working on it by brute force (setting hassixel=yup) but I don't the clever (correct) way to do this.

Hope you're able to help

Ta

@smhmd
Copy link

smhmd commented Sep 9, 2020

Seconded.

Reproduce:

git clone https://github.com/charlesdaniels/st.git
cd st 
# change font in config.h if you want a working font
make
./st

This will launch new st. Run the command convert foo.jpg -geometry 800x480 sixel:-.
image

@hackerb9
Copy link
Owner

hackerb9 commented Sep 10, 2020

Thank you for reminding me of this issue.

The problem is that st is not responding correctly to the Send Device Attributes query. If it supports sixels, it should include the number "4" (delimited by semicolons) in its response. You can see that it responded with "6c" which means it only supports VT102 escape sequences, not sixel. This is an easy problem to fix — the response string can be changed to "63;4" and it'd work — but it should be done by the developers of st (@charlesdaniels and @galatolofederico). I'll file a bug report to let them know.

@smhmd
Copy link

smhmd commented Sep 10, 2020

Because sixel support is added through a patch, I'm not sure if filing a bug to upstream would result in a fix unless what you are talking about is of concern to vanilla st. Maybe this should be taken up with @saitoha.

@hackerb9
Copy link
Owner

In the discussion of the patch, @charlesdaniels says,

“I have not implemented the escape sequences which would allow sixel support to be correctly detected by programs running within the terminal, so many programs must be ‘forced’ into their relevant sixel modes.”

Maybe Charles does indeed wish to implement a fix and just needs a nudge?

@charlesdaniels
Copy link

charlesdaniels commented Sep 10, 2020

I don't use st anymore, and instead use uxterm in VT430 mode (hint: add UXTerm*decTerminalID: vt340 to ~/.Xresources). I don't have any plans to resume working on my st fork at this time. I think st is a worthy project, but UXterm is ubiquitous, well supported, and well tested -- xterm is the standard against which other terminal emulators are judged. Not looking to start a flame war though -- if st solves the problems you want solved, that's great and you should use it.

When I was using it actively, I simply patched my copy of lsix to disable the check for sixel support.

Perhaps lsix should check for an environment variable like LSIX_FORCE_SIXEL_SUPPORT or something and disable the relevant check if so.

@data-man
Copy link

Yet another active fork: LukeSmithxyz/st
But:

2496: /* TODO: render sixel /;
2505: /
TODO: implement sixel mode */

@charlesdaniels
Copy link

I would like to note that my fork of st is probably not a good starting point for adding sixel support. It more or less worked, but was flaky and had some odd behaviors. Additionally, the diff I started with is unclear on licensing, so the actual license of my fork is questionable (as noted in the README) too. Finally, st has continued to be updated, and I have not kept up with it. I think I tried bringing it forward to a newer base version of st a while ago but there were merge conflicts that were too much for me to resolve at the time.

If someone really wants to do this right, it would be best to start with a fresh copy of st and contribute your patches upstream (the st homepage tracks community patches). I never cleaned mine up properly to contribute it there, but that would be the ideal.

Sixel is not a terribly complicated format. I think that a C programmer with a good knowledge of xlib could do this right in a weekend or two. My xlib skills are not so good though, hence why I didn't do that myself.

Good luck!

@hackerb9
Copy link
Owner

hackerb9 commented Sep 10, 2020

Perhaps lsix should check for an environment variable like LSIX_FORCE_SIXEL_SUPPORT or something and disable the relevant check if so.

Done.

I understand you're not working with st any more, but do you think you could add just the small fix I suggested? Basically, instead of returning 6, return 12;4 to Send Device Attributes. I see that your repository is the upstream for five other repositories that use your sixel implementation. It'd be easiest to get this fixed in your repo than in each of those.

Thanks.

@charlesdaniels
Copy link

I'll see what I can do. It's been a long time since I looked the handling for escape codes, but this shouldn't be too hard.

I should probably also update the README to indicate it's not active anymore while I'm at it.

@smhmd
Copy link

smhmd commented Sep 10, 2020

@bakkeby, could you look into adding this into st-flexipatch (and perhaps a community patches)?

@charlesdaniels
Copy link

I think I have fixed it, hope this resolves your troubles!

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

No branches or pull requests

4 participants