Skip to content

fix: unify colour resolution behind a terminal profile - #84

Open
aymanbagabas wants to merge 1 commit into
microsoft:mainfrom
aymanbagabas:feat/terminal-config
Open

fix: unify colour resolution behind a terminal profile#84
aymanbagabas wants to merge 1 commit into
microsoft:mainfrom
aymanbagabas:feat/terminal-config

Conversation

@aymanbagabas

@aymanbagabas aymanbagabas commented Aug 4, 2026

Copy link
Copy Markdown
Member

First of a three PR stack: #84#85#86.

A screenshot and a color assertion disagreed about what color a cell was, because each resolved palette indices through its own hardcoded table. render/svg.rs had a private Theme where red was #e88388; assert/color.rs had an ANSI16 table where red was #800000. Nothing kept them in sync and nothing noticed they had drifted.

The bug

Same program, same red text, on main:

$ shell-use submit 'printf "\033[31mERROR\033[0m: red text\n"'
$ shell-use screenshot --out shot.svg     # paints ERROR #e88388

$ shell-use expect text ERROR --fg '#800000'   # a color not in the image
$ echo $?
0                                              # ...passes

$ shell-use expect text ERROR --fg '#e88388'   # the color you can see
$ echo $?
1                                              # ...fails

Asserting the color the image actually paints failed, and asserting a color that appears nowhere passed. Either the picture was lying or the assertion was, and a test suite could not tell you which.

before — image paints #e88388 after — image paints #800000
before after

Both tables are deleted. One Colors type resolves every index once, so the renderer and the assertion cannot disagree by construction. --fg '#800000' now passes and matches the picture.

Profiles

The palette is no longer a private constant, so a session can be given one. --config picks the file and --profile the entry; discovery cascades ./shell-use.toml then ~/.shell-use/shell-use.toml.

[profiles.solarized]
scrollback = 10000

[profiles.solarized.colors]
background = "#002b36"
foreground = "#839496"
cursor     = "#d33682"
red        = "#dc322f"
green      = "#859900"
blue       = "#268bd2"
$ shell-use run --config shell-use.toml --profile solarized -- bash

solarized

A profile is exactly 19 values: the 16 ANSI colors plus foreground, background, and cursor. It is read only for the life of the session, so a program cannot rewrite the palette a test was pinned against. Anything outside 0-15 comes from a static xterm table, and scrollback defaults to 10000.

Notes for review

  • --fg <index> compares the cell's palette index, so it was never affected by this bug and still is not. Only #rrggbb resolves through the palette.
  • Snapshots record the slot ("fg": 1), not RGB, so existing baselines survive a palette change. There are tests pinning that.
  • Defaults are the VGA palette, foreground #c0c0c0 on background #000000, which is what the assertion side already used, so no assertion changes meaning.

A screenshot and a color assertion disagreed about what a cell was
painted. `render/svg.rs` carried a private sixteen-color table and
`assert/color.rs` carried a different one, so `expect --fg "#800000"`
passed on a cell the screenshot drew `#e88388`. Both tables are deleted
here and both callers resolve through one profile, which is what makes
them agree by construction rather than by coincidence.

The palette had to become configurable to fix it anyway: the two tables
could only be collapsed by choosing which one was right, and that choice
belongs to the user rather than to whichever module was read first. The
shipped default is the VGA/xterm palette that `TERM=xterm-256color`
already promises, which is what the assertion side used.

A profile is read from `shell-use.toml` and sets scrollback and colors.
Only the sixteen ANSI slots and the three defaults are configurable;
indices 16-255 are the xterm color cube and gray ramp, which are fixed by
the spec, so a config that could move them would let two sessions
disagree about what `--fg 196` means.

Profiles are named, and `--profile` selects one. The file is looked up
nearest first, project before user, so a repository can pin the terminal
its tests expect. Resolution happens in the CLI rather than the daemon:
the daemon is long-lived and shared, so it has no single working
directory to resolve a project-local config against, and a resolved
profile travels on `Request::Open` the same way timeouts already do.

Absent settings take the default, and the field is `#[serde(default)]`,
so a client that predates this keeps the behavior it had. Scrollback
moves from a hardcoded 5,000 to a configurable 10,000, matching
alacritty's own default.

Two things are deliberately errors rather than silent fallbacks: an
unknown profile name, which reports the ones that exist, and a config
file that does not parse, which would otherwise run the session with
settings nobody asked for. A *missing* file stays fine, since running
without one is normal.

Screenshots will look different: the default background is now black
rather than the previous dark blue-gray, and the palette is saturated
rather than muted. Both are recoverable in a profile.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
@aymanbagabas

Copy link
Copy Markdown
Member Author

@cpendery could you take a look? This is the base of a three PR stack:

Read them in that order. GitHub could not request you as a reviewer directly (I do not have push access here), hence the mention.

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.

1 participant