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

fix(compat): more accurate behavior of SGR 21 #309

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

Conversation

mkotowski
Copy link

What was changed

Switch const key for SGR 21 to a more widely supported variant.

Rationale

ECMA-48 and most common terminal emulators implement SGR 21 as double underline, which predates SGR 4 underline style extension. This behavior is much more common than "no bold" one.

Currently, x/ansi uses [Nn]oBoldAttr for SGR 21, but the consensus among terminal emulators (and ECMA-48) is to apply double underline or other visual indicator instead.

printf '\e[21mTest\e[m'

Double underline:

  • ECMA-48
  • WezTerm (flatpak 20240203-110809-5046fc22)
  • KDE konsole 23.08.5
  • Gnome Terminal Version 3.52.0 for GNOME 46 (Using VTE version 0.76.0 +BIDI +GNUTLS +ICU +SYSTEMD)

Single underline or other visual indicator:

  • xterm (372)
  • Linux console since Linux 4.17

Normal intensity / no bold:

Impact

  • This will break apps using the NoBold function, but x projects do not guarantee backwards compatibility.
  • After a cursory search, NoBold() does not seem to be used (at least according to GitHub's search) and impact of the change should be minimal.
  • This requires change in charmbracelet/sequin.

ECMA-48 and most common terminal emulators implement SGR 21 as
double underline, which predates SGR 4 underline style extension.
This behavior is much more common than "no bold" one.

Signed-off-by: Michał Kotowski <[email protected]>
@caarlos0
Copy link
Member

this LGTM

for reference, ghostty also shows double underline, rio does nothing, apple terminal does nothing as well

@aymanbagabas
Copy link
Member

Perhaps we should remove both NoBold and EcmaDoubleUnderline altogether. You can reset bold attribute using SGR 22 NormalIntensity. Double underline using SGR 21 is not widely implemented and usually, fwict, implemented using the underline extension i.e. SGR 4:n

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