Skip to content

Add support for SGR 2 faint and SGR 9 strikethrough - #1380

Open
antonme wants to merge 2 commits into
mobile-shell:masterfrom
antonme:upstream-pr-sgr-faint-strikethrough
Open

Add support for SGR 2 faint and SGR 9 strikethrough#1380
antonme wants to merge 2 commits into
mobile-shell:masterfrom
antonme:upstream-pr-sgr-faint-strikethrough

Conversation

@antonme

@antonme antonme commented Mar 28, 2026

Copy link
Copy Markdown

This adds support for the ECMA-48 SGR attributes for faint (2) and
strikethrough (9), along with their reset sequences.

In particular:

  • 2 enables faint / decreased intensity
  • 9 enables strikethrough / crossed-out
  • 22 clears both bold and faint
  • 29 clears strikethrough

The implementation is intentionally small and only touches:

  • src/terminal/terminalframebuffer.h
  • src/terminal/terminalframebuffer.cc
  • src/tests/emulation-attributes.test

It wires the already-defined faint attribute into the SGR parser/emitter and
adds strikethrough to the rendition enum so both attributes are parsed and
re-emitted correctly.

Test coverage extends the existing emulation-attributes regression test with
cases for:

  • 2
  • 9
  • 1;22
  • 2;22
  • 9;29

This is a current rebase of the older #1059 change onto current master.

@ak2k

ak2k commented Aug 8, 2026

Copy link
Copy Markdown

We hit #1276 in practice (a TUI whose dim-styled and strikethrough text degrades through mosh) and found this PR already solving it, so we put it through independent verification before deploying it to our own machines. Sharing the evidence in case it helps review:

  • Exhaustive behavioral check: swept all 256 attribute start-states × SGR parameters 0–300 (77,056 cases) comparing Renditions::sgr() output of this PR's implementation against a reference implementation written independently from ECMA-48 — byte-identical, including the SGR 22 clears-bold-and-faint pairing and the 9/29 set/reset.
  • Mixed-version safety: ran patched-server↔stock-client and stock-server↔patched-client sessions. Attributes degrade to today's behavior; no desync, no crash. (Expected, since the state diff travels as host bytes re-parsed by each end's emulator and there's no protobuf change — but confirmed empirically.)
  • Test additions: confirmed emulation-attributes-truecolor fails on unpatched sources and passes with the PR — the new cases genuinely exercise the change. No other make check outcome moves; zero new warnings at --enable-compile-warnings=maximum; clang-tidy adds no findings on the changed lines.

One suggestion: with strikethrough added, attribute_type now has exactly 8 entries — the full width of the attributes : 8 bit-field. A future 9th attribute would be stored and read back as 0 with no diagnostic even at -Wall -Wextra -pedantic (verified). A one-liner would make that a compile error instead:

static_assert( SIZE <= 8, "attribute_type outgrew the 8-bit attributes bit-field" );

We've been running a 1.4.0 backport of this PR in production (Linux servers + macOS clients) since early August with no issues. Happy to share the test harness if useful.

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.

2 participants