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

tmux: enable sixel support #26057

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

laggardkernel
Copy link
Contributor

@laggardkernel laggardkernel commented Oct 3, 2024

Description

tmux added SIXEL support since 3.4, enable this feature during build.

https://github.com/tmux/tmux/blob/2df15ad08c9e4cf286cdb38cb645fc9066c3098d/CHANGES#L102

test method, build and open shell in tmux, install ImageMagick

convert /path/to/pic sixel:-

You should get a preview of the picture within tmux, if your terminal also support SIXEL images. (terminals including iTerm2, wezterm, etc)

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 12.7 21G816 arm64
Command Line Tools 14.2.0.0.1.1668646533

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install? (without -t trace mode, which is broken)
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@macportsbot
Copy link

Notifying maintainers:
@tessus for port tmux.

@macportsbot macportsbot added type: enhancement maintainer: open Affects an openmaintainer port labels Oct 3, 2024
@casr
Copy link
Contributor

casr commented Oct 3, 2024

What would be the result with terminals that do not support Sixel such as Terminal.app or Alacritty?

Should it be applied as a variant instead?

edit: And if the first point has no issues, should this be enabled as a default variant?

@laggardkernel
Copy link
Contributor Author

@casr SIXEL is a protocol defines how to convert image file into escape sequences. ImageMagick is a tool who supports this process.

For terminals who understand this protocol, it will render the generated escape sequence as picture and display it in the terminal. For those who don't, in my test, they just ignore it and do nothing.

Should it be applied as a variant instead?

I don't think so. It works only when user try to preview an image in the terminal explicitly, using some tools like ImageMagick.

Tmux doesn't render the image directly, it works as a middleman in the preview process. The sixel support of tmux just make sure the escape sequence is passed out to terminals, untouched, but not intercept the sequence and handle it by tmux itself.

@tessus
Copy link
Contributor

tessus commented Oct 3, 2024

I agree, this is the same as other escape sequences being sent by tmux natively. If the terminal does not understand them, it depends on the terminal to handle this situation.

TL;DR
Terminals are notorious for ignoring standards and/or extending the functionality without proper specs. In a perfect world, apps could query the terminal whether it supports certain features and in many cases this even works. However, it is not something an app can rely on completely. Thus there are many workarounds in a lot of apps to support multiple terminals seemlessly.

@casr
Copy link
Contributor

casr commented Oct 3, 2024

I'll argue for a variant because:

  1. tmux treats this as a feature that must be enabled
  2. it's suffered resistance in the past with some projects adopting it due to things like performance concerns or 'why this standard over another?'

For those reasons, I think having the ability to toggle this as a variant would be welcomed by some.

I do not see a problem making this a default variant though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: enhancement
Development

Successfully merging this pull request may close these issues.

4 participants