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

sixel: Implement DEC-accurate calculation of cursor position for jumping #149

Merged
merged 1 commit into from
Jan 11, 2025

Conversation

hzeller
Copy link
Owner

@hzeller hzeller commented Jan 10, 2025

Right now only known to be implemented by development version of Windows Terminal.
Until we can detect that this is the terminal we're running, requires to set environment variable TIMG_SIXEL_FULL_CELL_JUMP=1

Issues: #145

@hzeller
Copy link
Owner Author

hzeller commented Jan 10, 2025

@j4james this implements the calculation suggested in #145 (comment)
Can you patch this in your local copy and and test if it works for you (possibly also @gestate-8 ) ?

Right now, you need to set both environment variables TIMG_SIXEL_FULL_CELL_JUMP=1 and TIMG_SIXEL_NEWLINE_WORKAROUND=1 as we can't autodetect yet hat we're under the windows terminal

@j4james
Copy link
Contributor

j4james commented Jan 11, 2025

@hzeller It works for me. Although I think it would have been nicer if you just extended the TIMG_SIXEL_NEWLINE_WORKAROUND variable so it's range of options instead of a bool (i.e. the DEC compatible option would now be 2). That way users only have one variable they need to set, and if you want to support more terminals in the future, you can easily add another option to the list. Not a big deal though.

@gestate-8
Copy link

Works very well with envs.
The images are arranged slightly overlapping each other without envs.

2025-01-11

@hzeller
Copy link
Owner Author

hzeller commented Jan 11, 2025

Thanks everyone for playing with the option, sounds like the way to go then.

...Although I think it would have been nicer if you just extended the TIMG_SIXEL_NEWLINE_WORKAROUND variable

Since the values are somewhat independent behaviours it would need to be a bitset, so it could have values

  • 1 = TIMG_SIXEL_NEWLINE_WORKAROUND
  • 2 = TIMG_SIXEL_FULL_CELL_JUMP
  • 3 = TIMG_SIXEL_NEWLINE_WORKAROUND | TIMG_SIXEL_FULL_CELL_JUMP

Const: it might be harder to understand what exactly is modified.
Pros: it would be easier to just set a single environment variable and try out all possible values much more easily. And users probably don't care about the exact details as long as they can stop incrementing numbers once it works :)

Let me think about it, but now I also lean towards a single environment variable.

@hzeller hzeller force-pushed the feature-20250110-full-cell-jump branch 3 times, most recently from ca1459d to 027e15e Compare January 11, 2025 19:40
Right now only known to be implemented by development version of
Windows Terminal.

`TIMG_SIXEL_NEWLINE_WORKAROUND` now is a bitmap that allows for the
various newline behaviors to be set

  * 1 = regular newline/carriage return choice
  * 2 = full cell jump (this new feature)
  * 3 = the OR'ed value out of 1 and 2

Until we can detect that this is the terminal we're running, requires
to set environment variable; for windows terminal, that is
`TIMG_SIXEL_NEWLINE_WORKAROUND=3`.

Issues: #145
@hzeller hzeller force-pushed the feature-20250110-full-cell-jump branch from 027e15e to 513f4c1 Compare January 11, 2025 19:41
@hzeller
Copy link
Owner Author

hzeller commented Jan 11, 2025

Ok, made it now the single environment variable TIMG_SIXEL_NEWLINE_WORKAROUND again, which can have values 0..3. For WindowsTerminal, the value should be set to 3.

@hzeller hzeller merged commit 75027dd into main Jan 11, 2025
7 checks passed
@hzeller hzeller deleted the feature-20250110-full-cell-jump branch January 11, 2025 21:22
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.

3 participants