Skip to content

[terminfo] support the %'c' character constant - #1744

Merged
tstack merged 2 commits into
tstack:masterfrom
aspiers:terminfo-char-constant
Aug 24, 2026
Merged

[terminfo] support the %'c' character constant#1744
tstack merged 2 commits into
tstack:masterfrom
aspiers:terminfo-char-constant

Conversation

@aspiers

@aspiers aspiers commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #1743.

The terminfo parser added in 1e8221e handles %{nn} integer constants but has
no case for the %'c' character constant, so the operator falls through to
default: p++ and its bytes are copied into the output as literal text.

xterm-256color's setaf uses %'\010' and %'\020' to test the colour index
against 8 and 16, so the conditional never evaluates:

setaf(112)  ->  \E[\010'3112m      instead of  \E[38;5;112m
setaf(8)    ->  \E[\010'38m        instead of  \E[90m

0x08 and ' are not valid in a CSI parameter string, so terminals abort the
sequence and print the remainder as text. That produces the 3112m-style
garbage in the display and misaligns columns.

This adds case '\'', handling plain characters, octal escapes such as
%'\010', and the common named escapes.

Affected terminals

Any entry whose setaf/setab uses %'c' — xterm-256color, xterm-16color,
screen-256color, tmux-256color, alacritty, xterm-kitty, xterm-ghostty,
vte-256color. Not affected: xterm, xterm-color, screen, tmux, linux.

Verification

check before after
malformed CSI sequences per render 111 0
junk fragments visible on screen 19 0
standalone reproducer (8 setaf cases) all wrong all correct

Rendered under 13 different TERM values: entries using %'c' give 111 before
and 0 after; entries without it give 0 in both cases — exact correlation, no
exceptions.

make check gives 43 pass / 10 fail both with and without this patch. The 10
failures are pre-existing on master and unrelated (mostly SQL function tests).

The issue includes a self-contained reproducer that exercises the parser
directly, with no terminal involved.

Disclosure

The diagnosis, reproducer and patch were produced with AI assistance
(Claude Code) and reviewed before submission. The reproducer is self-contained
so the claims can be verified independently.

The terminfo parser handled %{nn} integer constants but had no case for
%'c', so the operator fell through to the default branch and its bytes
were copied into the output as literal text.

xterm-256color's setaf uses %'\010' and %'\020' to test the colour index
against 8 and 16, so the conditional never evaluated and tiparm_s()
returned a malformed CSI sequence containing a literal BS:

    setaf(112) -> \E[\010'3112m   instead of   \E[38;5;112m

0x08 and ' are not valid in a CSI parameter string, so terminals abort
the sequence and print the rest as text, which corrupts the display.

This affects every terminfo entry whose setaf/setab uses %'c', including
xterm-256color, xterm-16color, screen-256color, tmux-256color, alacritty,
xterm-kitty, xterm-ghostty and vte-256color.

Handle plain characters, octal escapes such as %'\010', and the common
named escapes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tstack
tstack requested a lite review from Copilot and removed request for Copilot August 23, 2026 20:07
tiparm_s consulted `exec` only when producing output, never when
mutating the stack, so `%p`, `%{n}`, `%'c'` and the binary operators
ran in both arms of a `%?...%t...%e...%;`.  Entries that place `%d`
after the `%;` then print whatever the untaken arm left behind:

    setaf=\E[%?%p1%{8}%<%t%p1%{30}%+%e%p1%'R'%+%;%dm

For setaf(0) the live arm pushes 30, the dead arm pushes 0 and 'R'
and adds them to 82, and `%d` pops 82.  Adding the `%'c'` push made
this reachable for the whole `*-16color` family: setaf(0..7) emitted
SGR 82-89, which terminals ignore, and setab(0..7) emitted SGR 92-99,
which are bright foreground colors, so a background request silently
recolored the foreground instead.  Gating every stack operation on
`exec` brings xterm-16color and xterm-256color to zero mismatches
against ncurses tparm.

The backslash handling in `%'c'` is also removed.  terminfo_load reads
compiled entries, and tic resolves escapes at compile time, so a source
`%'\010'` arrives as a raw 0x08 that the plain-character path already
handles.  The only byte the escape branch could actually encounter is
a literal backslash from a source `%'\\'`, and it mis-parsed that one,
consuming the closing quote as the escaped character and pushing 0x27
instead of 0x5C.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tstack

tstack commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Thanks for tracking this down — the diagnosis is right and the reproducer made it easy to verify. I've pushed a follow-up commit to your branch rather than round-tripping; details below so you can check my reasoning.

Why the extra commit

tiparm_s consults exec only when producing output, never when mutating the stack. %p, %{n} and the binary operators all run in both arms of a %?…%t…%e…%;. That was latent for %'c' because it used to push nothing — adding the push activated it.

The 16-color entries put %d after the %;, so they print whatever the untaken arm left on the stack:

setaf=\E[%?%p1%{8}%<%t%p1%{30}%+%e%p1%'R'%+%;%dm
setab=\E[%?%p1%{8}%<%t%p1%'('%+%e%p1%{92}%+%;%dm

For setaf(0): the live arm pushes 0+30 = 30, the dead arm now also pushes 0 and 'R' (82) and adds them to 82, leaving [30, 82], and %d pops 82. Measured against ncurses tparm for xterm-16color:

setaf 0–15 setab 0–15
your commit alone 8/16 wrong — setaf(0): \E[30m\E[82m 8/16 wrong
with the follow-up 0/16 0/16

setab(0..7) was the one that worried me most: it goes from a malformed CSI the terminal aborts to a well-formed but wrong SGR 92–99, which are bright foreground colors. A background-color request would silently recolor the foreground instead, with nothing on screen to suggest anything was broken. Gating every stack operation on exec fixes the family.

The backslash branch

I also dropped the escape handling. terminfo_load reads compiled entries, and tic resolves escapes at compile time, so a source %'\010' reaches the parser as a raw 0x08 — which the plain-character path already handled. I confirmed with a round-trip:

source:    setaf=\E[%?%p1%'\\'%<%t9%p1%d%e%p1%'\010'%+%d%;m
compiled:  ... %'\'...        (0x5C, raw backslash)
           ... %'<0x08>'...   (escape resolved by tic)

So the branch was reachable on exactly one input — a literal backslash from a source %'\\' — and mis-parsed it, eating the closing quote as the escaped character and pushing 0x27 instead of 0x5C. On that tic-compiled entry it took the wrong arm of the conditional for 53 of 256 inputs. Removing the branch takes it to 0.

One note on the PR description

On ncurses 6.x here, xterm-256color's setaf is \E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38;5;%p1%d%;m%{8}/%{16}, no %' at all — and measures 0 mismatches before your patch. Same for screen-256color, tmux-256color and vte-256color. The entries that actually use %'c' in setaf/setab are the -16color family. Your reproducer with raw %'\010' is 256/256 wrong before and 0/256 after, so the bug is real; the affected-terminals list just looks terminfo-DB-dependent. Out of curiosity, which ncurses version did you hit it on?

Verification

Differential harness against ncurses tparm over all 793 parameterized capabilities in /usr/share/terminfo and the Homebrew ncurses DB: 0 formats regressed relative to your commit, 6 improved; relative to pre-PR, 67 improved and 1 regressed — \Eg%p2%' '%+%c%p1%c (7 QNX entries), which depends on %c, unimplemented in this parser both before and after. Separately worth someone's time, but not this PR's problem.

@tstack

tstack commented Aug 23, 2026

Copy link
Copy Markdown
Owner

That comment above is from Claude...

Thanks for the PR. I wish terminfo wasn't an overcomplicated pile of crud, sigh. I'll get it merged in a little bit.

@aspiers

aspiers commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

The remaining CI failure looks pre-existing rather than related to this branch.

ci-build fails on one test, test_sql_views_vtab.sh, in both the build and coverage jobs. The same test was already failing on develop before this PR existed — run 30946960697 (2026-08-04, 8a8c776e8) fails on exactly test_cmds.sh, test_sessions.sh, test_sql_views_vtab.sh and test_text_file.sh, and the four ci-build runs on develop from 2026-08-03 to 2026-08-05 all failed.

Other observations:

  • The failure is identical before and after cf21cc9, so the follow-up commit did not change it.
  • test_view_colors.sh and test_tui.sh pass, and the colour codes in the captured test output are well-formed SGR sequences.
  • In the captured output for the two test_sql_views_vtab.sh cases that the log dumps, the actual output matches the expected files byte for byte, so the failing assertion is elsewhere in that script and the log does not include its diff.
  • I could not reproduce it locally: make check TESTS=test_sql_views_vtab.sh and a full make distcheck both pass that test here.

So I have not identified the actual cause — only that it does not appear to be introduced by this branch.

@tstack
tstack merged commit dce328b into tstack:master Aug 24, 2026
1 of 3 checks passed
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.

terminfo parser does not implement the %'c' character constant, corrupting setaf output

2 participants