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

OSC sequences are mis-counted in PrintableRuneWidth #64

Open
rwe opened this issue Oct 2, 2023 · 0 comments · May be fixed by #65
Open

OSC sequences are mis-counted in PrintableRuneWidth #64

rwe opened this issue Oct 2, 2023 · 0 comments · May be fixed by #65

Comments

@rwe
Copy link

rwe commented Oct 2, 2023

OSC sequences are sequences beginning with OSC (ESC ]) and ending in ST (ESC ], or BEL which is 0x07). Note that that's a right bracket, not a left one like CSI sequences.

Those sequences should be ignored, but currently they entirely misinterpreted.

Although PrintableRuneWidth ignores characters after ESC (0x1B), isTerminator does not correctly recognize either variant of ST: (ESC \\, or BEL).

In the former, it will mis-count the slash after the ESC; and the latter fails because BEL is not matched as a sequence terminator.

Specifically, this breaks text using OSC8 for describing URL links.
OSC8 is a mechanism for describing URI/URL links in terminals, described very digestibly here:
https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#the-escape-sequence

It's widely supported by many modern terminals (https://github.com/Alhadis/OSC8-Adoption/), and those that don't support it fall back to omitting the URL part. (Same text, just not clickable).

On a whim, I wanted to make the GitHub cli (https://github.com/cli/cli) support those links, but hit a stumbling block: its tableprinter measures column text width ultimately through muesli/reflow.

It could be fixed in their intermediate go-gh package, but it seems like this is the "right" place to fix it, or at least to have the issue logged.

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 a pull request may close this issue.

1 participant