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

XTSMGRAPHICS and XTWINOPS support for usage with sixels #3695

Closed
ferdinandyb opened this issue Sep 14, 2023 · 9 comments
Closed

XTSMGRAPHICS and XTWINOPS support for usage with sixels #3695

ferdinandyb opened this issue Sep 14, 2023 · 9 comments

Comments

@ferdinandyb
Copy link

Issue description

I'm out of my depth here, so I may not be phrasing things correctly here. This issue came up when looking at matplotlib backends supporting sixels in tmux. It was noted there that tmux doesn't seem to support XTSMGRAPHICS and XTWINOPS anntzer/mplterm#3 (comment) (and also that it would be nice to be able to query INPUT_BUF_LIMIT to be automatically size sixels to fit) of which at least XTSMGRAPHICS seems to be expected if sixel support is present, while I guess XTWINOPS is just very useful? See about XTSMGRAPHICS and sixels here as well: wez/wezterm#609.

Required information

Please provide the following information:

  • tmux version: latest master
  • Platform: Linux x86_64
  • $TERM inside and outside of tmux: contour/tmux-256color
@nicm
Copy link
Member

nicm commented Sep 14, 2023

Correct, tmux does not support these.

@nicm nicm closed this as completed Sep 14, 2023
@nicm nicm reopened this Sep 14, 2023
@ferdinandyb
Copy link
Author

Oh yeah, I should've drank my coffee and write an actual question/issue: could support for these be added?

@nicm
Copy link
Member

nicm commented Sep 14, 2023

Yes, why not, although note these are xterm extensions (hence the XT) and are not a requirement of SIXEL.

Please try this: x.diff.txt

@ferdinandyb
Copy link
Author

That was fast thanks! Unfortunately currently I'm sitting in front of WSL so I can only test tonight ...

@ferdinandyb
Copy link
Author

This changes the error in the matplotlib backend library, so I'm going to take this back to there for a round too.

Just for verification, I did this with the provided patch (I'm posting because of the interaction asking about the -R assumption):

❯ git rev-parse HEAD
c57a09269b2a40bc78cb03febb24acfdaddd7331
❯ patch -p1 < x.diff.txt
patching file image-sixel.c
patching file input.c
patching file screen-write.c
Reversed (or previously applied) patch detected!  Assume -R? [n] y
patching file tmux.h

@nicm
Copy link
Member

nicm commented Sep 14, 2023

The patch has some other bits in it which I already applied, you can revert them (y to -R) or not, it won't make any difference.

@ferdinandyb
Copy link
Author

Thanks @nicm again for the very speedy update, it now works perfectly :) This is going to make working with data on remote machines so much easier.

image

@nicm
Copy link
Member

nicm commented Sep 15, 2023

I have applied this now, thanks!

@nicm nicm closed this as completed Sep 15, 2023
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants