Conversation
A colors.toml is a file people write by hand, and they comment it:
accent = "#4ECDC4" # secondary-400 -- the paired accent
The quotes were only stripped when the value both opened and closed with
one, so a trailing comment left them in place, QColor rejected the whole
string, and the theme silently fell back to the built-in yellow. The
accent omacut followed was the one colour the theme had ruled out.
Take a quoted value up to its closing quote, and cut an unquoted one at
the first '#' that isn't the colour's own leading hash -- so bare hex and
named colours survive a note after them too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016rcJ8LUydunVeEgaT9hHDr
Chessing234
left a comment
There was a problem hiding this comment.
fixes silent fallback to yellow when themes annotate accents. small and correct.
|
Reviewed, and the fix holds. Nothing to change here; the two things worth naming are both about malformed input rather than about themes. Built and ran on a disposable VM rather than on the review machine, so none of this branch's code executed where credentials live. The before/after was measured rather than taken on trust. The same probe was compiled twice on that VM, once against Cases the added tests do not cover were run too, and all of them answer correctly: a comment with no space before it ( Two low findings, neither of which affects a well-formed theme:
Both are strictly more lenient than One thing the maintainer should see, since it is not visible from this repository alone. This is the same defect in three apps that each carry a copy of the parser, and all three now have a fix open, written by three different people to three different designs: omacom/omacalc#14 for omacalc#12 adds a line-level Reviewed by Claude Opus 5 in Claude Code and independently by Codex at xhigh reasoning. Codex reached the same conclusions on the traces and the empty-value bounds that had already been worked out here, so that is agreement rather than confirmation, and its independence is not currently guaranteed. The trailing-junk case above is its own: it was not part of the first pass, and it was verified on the VM before being written down. Waiting on the maintainer for the merge and for the cross-repository shape decision. Nothing is needed from the author. |
A
colors.tomlis written by hand, and people annotate it:accentFromColorsFileonly stripped the quotes when the value both opened and closed with one. A trailing comment means it doesn't, so the quotes stayed on,QColor::fromStringrejected the whole string, and the theme silently fell back to the built-in#FFD60A. The accent omacut followed was the one colour the theme had explicitly ruled out.Nothing reports this — the fallback is a valid colour, so the app just looks like it isn't following the theme.
The fix
A quoted value is taken up to its closing quote. An unquoted one is cut at the first
#that isn't the colour's own leading hash, so bare hex (accent = #4ECDC4 # note) and named colours (accent = teal # note) also survive a comment after them.Verified against a real theme
Built both ways and read the same file — a theme whose accent line carries a comment:
master#FFD60A(the fallback)#4ECDC4(the theme's accent)./bin/test— 27 passing.themeAccentReadsOmarchyColorsgains four cases: a commented double-quoted value, a commented single-quoted one, a commented bare hex, and a commented named colour. The existing cases are untouched and still pass, so uncommented themes read exactly as before.Unrelated to #5 and based on
master, so the two can go in either order.