[#132] Wire xshd colorizer through TG Scheme code-token VisualRoles#134
Conversation
Plan for routing the Editor's xshd colorizer through Terminal.Gui's Scheme / VisualRole system. Three-phase work across two repos: - Phase 0 (TG): config benchmark baseline — gui-cs/Terminal.Gui#5310 - Phase 1 (TG): code-token VisualRoles + Code view + markdown unification — gui-cs/Terminal.Gui#5311 - Phase 2 (Editor, this branch): wire xshd colorizer through TG Scheme, remove UseThemeBackground, add Theme switcher to ted Implementation commits will follow on this branch once TG #5311 ships. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 of specs/syntax-theme/spec.md. Routes the xshd highlighter through Terminal.Gui's Scheme/VisualRole machinery so themes drive syntax colors and the editor background follows the active scheme (fixes #128; restores #99's theme story via ThemeManager). - XshdRoleMap: xshd <Color name> -> VisualRole.Code* bridge table (spec 6.2) - HighlightingColor.Role + xshd category= override (XshdColor/V2Loader/ XmlHighlightingDefinition/SaveXshdVisitor); logged in UPSTREAM.md - HighlightingColorizer.ToAttribute resolves: explicit scheme role -> xshd foreground over scheme bg -> default; ctor takes role-lookup funcs - Removed Editor.UseThemeBackground end-to-end (theme decides bg now) - Editor repaints on ThemeManager.ThemeChanged - ted: status-bar Theme shortcut cycles ThemeManager.Theme - Tests: scheme-role / xshd-fallback / XshdRoleMap coverage / category override (unit) + scheme-swap render smoke (integration). 439 + 174 green. Blocked on gui-cs/Terminal.Gui#5313 (code-token VisualRoles) reaching a published develop pre-release. <TerminalGuiVersion> must be bumped to that build before CI can go green; verified locally against a #5313 pack via -p:TerminalGuiVersion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reconciles develop's ted settings-UI port (#123) + indent fixes (#136) with Phase 2's removal of Editor.UseThemeBackground. UseThemeBackground is fully retired from the ted demo (the Editor knob is gone; theme decides bg): - TedApp.cs: dropped the Editor init flag, the "Use Theme Background" checkbox + its View-menu item, and the SaveViewSettings write-back; re-applied the Phase 2 Theme status-bar shortcut on top of develop's restructured ctor. - EditorSettings.cs: removed the UseThemeBackground persisted setting (property + ReadBool load + JSON serialize). - TedApp.MarkdownPreview.cs: Markdown preview no longer seeds UseThemeBackground from the removed Editor property (TG default). - Dropped the now-invalid Constructor_Defaults_UseThemeBackground_To_True test and the EditorSettings.UseThemeBackground reset. Verified against a local TG #5313 pack: 0 warnings/0 errors, Tests 439/439, IntegrationTests 190/190, dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Verified Editor #134 green against the merged TG develop (209c0a34c: #5313 + #5312): build 0/0, Tests 439/439, IntegrationTests 190/190. Remaining: bump <TerminalGuiVersion> to the first published 2.1.1-develop.<n> post-dating the #5313 merge, then un-draft #134. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gui-cs/Terminal.Gui#5313 (code-token VisualRoles) is published on the develop NuGet feed as 2.1.1-develop.95 (later .125-.163 are bad/unlisted). Bumped <TerminalGuiVersion> off the placeholder and removed the TODO. Verified against the real published package: build 0 warnings/0 errors, Tests 439/439, IntegrationTests 190/190, dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28797e77bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses PR #134 review (discussion r3254066078): Role was a plain auto-property, bypassing the freeze guard every other mutable property (Foreground/Background/Name/...) enforces. Loaded definitions are frozen, so this let callers mutate token-role mapping post-freeze. Now uses a _role backing field + the same IsFrozen throw; MergeWith sets _role directly like the other field merges. Load path is unaffected (the xshd populate visitor runs before Freeze()). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_role Follow-up to PR #134 review. CLAUDE.md Properties section now requires an explicit backing field to sit immediately above the property it backs (never a top-of-type block), with a fork-policy carve-out for existing upstream field blocks in lifted files. Moved the new _role field next to HighlightingColor.Role accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b50acc8df3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Implements the syntax-highlighting theme bridge by routing xshd colors through Terminal.Gui VisualRole/Scheme roles and removing the old editor-level theme-background toggle.
Changes:
- Adds xshd color-name/category mapping to
VisualRole.Code*roles and rewrites colorizer resolution. - Removes
UseThemeBackgroundfrom editor and ted settings/UI paths. - Adds theme-switch/status-bar support and test/spec coverage for scheme-driven syntax colors.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Directory.Build.props |
Bumps Terminal.Gui dependency. |
CLAUDE.md |
Documents field/property placement convention. |
specs/syntax-theme/spec.md |
Adds syntax-theme design/specification. |
third_party/AvaloniaEdit/UPSTREAM.md |
Documents fork additions for role/category plumbing. |
src/Terminal.Gui.Editor/Highlighting/XshdRoleMap.cs |
Adds xshd color-name/category to VisualRole mapping. |
src/Terminal.Gui.Editor/Highlighting/HighlightingColor.cs |
Adds runtime role metadata to highlighting colors. |
src/Terminal.Gui.Editor/Highlighting/Xshd/XshdColor.cs |
Adds xshd category property. |
src/Terminal.Gui.Editor/Highlighting/Xshd/V2Loader.cs |
Parses xshd category. |
src/Terminal.Gui.Editor/Highlighting/Xshd/XmlHighlightingDefinition.cs |
Resolves roles while materializing colors. |
src/Terminal.Gui.Editor/Highlighting/Xshd/SaveXshdVisitor.cs |
Round-trips xshd category. |
src/Terminal.Gui.Editor/Rendering/HighlightingColorizer.cs |
Resolves highlighting attributes via explicit scheme roles or xshd fallback. |
src/Terminal.Gui.Editor/Rendering/VisualLineBuildContext.cs |
Removes theme-background flag. |
src/Terminal.Gui.Editor/Rendering/VisualLineBuilder.cs |
Stops overriding styled-segment backgrounds via removed flag. |
src/Terminal.Gui.Editor/Editor.cs |
Hooks theme changes, removes UseThemeBackground, wires role-aware colorizer. |
src/Terminal.Gui.Editor/Editor.Drawing.cs |
Removes highlighter-background viewport fill and updates colorizer refresh. |
examples/ted/TedApp.cs |
Removes background setting UI and adds theme shortcut. |
examples/ted/TedApp.MarkdownPreview.cs |
Removes markdown preview background toggle forwarding. |
examples/ted/EditorSettings.cs |
Removes persisted UseThemeBackground setting. |
tests/Terminal.Gui.Editor.Tests/HighlightingTests.cs |
Updates/adds colorizer and role-map unit tests. |
tests/Terminal.Gui.Editor.IntegrationTests/EditorRenderingTests.cs |
Updates rendering integration tests for scheme-driven highlighting. |
tests/Terminal.Gui.Editor.IntegrationTests/TedAppTests.cs |
Removes old background default assertion. |
tests/Terminal.Gui.Editor.IntegrationTests/TedSettingsPersistenceTests.cs |
Removes reset of retired setting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A (r3254078860): VisitColor no longer drops an unnamed <Color> that carries only category= — early-return now also checks color.Category, so a role-only inline color materializes and reaches the colorizer. D (3254079982): XshdRoleMap.ResolveRole rejects category values that parse numerically but are not defined roles (Enum.IsDefined guard); falls through to the name table. E (3254079991): XshdRoleMap.Defaults is now private + FrozenDictionary — not mutable, not reachable via InternalsVisibleTo. G (3254080013): HighlightingColorizer takes ONE Func<VisualRole,Attribute?> (themed attr iff the scheme sets the role explicitly, else null) instead of a resolver + separate explicit-check predicate that could be half-wired. Editor + WithDefaultAttribute + tests updated. B (3254079970): spec Scenario 3 reworded to the implemented resolution order (xshd fallback, not derive-through Code/Editable/Normal). C (3254079978): spec typo unitl -> until. F (3254080003): HighlightingColor.Role XML doc no longer claims the fallback is "used unchanged". Plus: CLAUDE.md codifies "no static members on View-derived types" (the architectural rule behind E / the parallel-test guidance). Tests-first for the behavior bugs: Category_Numeric_String_Is_Not_Treated_As_Role and Anonymous_Category_Only_Color_Materializes_With_Role were added red, then made green. Build 0/0, Tests 458/458, IntegrationTests 190/190, dotnet format clean against the pinned 2.1.1-develop.95. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99318c194b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…306) P2 (3254095052): HighlightingEngine.PushColor coalesces adjacent sections when their colors compare Equal. With role-only / category-derived colors, two contiguous tokens differing solely by Role would merge and the second would inherit the first token's Role -> wrong theming. Equals and GetHashCode now include _role. UPSTREAM.md fork log updated. Test-first: HighlightingColor_Equality_Considers_Role added red (Role ignored -> colors equal), green after the fix. Build 0/0, Tests 459/459, IntegrationTests 190/190, dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `Code cleanup` commit 7302e00 left `[new()` where house style wants `new ()` (space before parens). ubuntu-latest `dotnet format --verify-no-changes` failed on it (build + all tests were green on every OS). Auto-formatted; no other files affected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
.98 is the current latest published develop pre-release and post-dates the #5313 merge. Verified against the real package: build 0/0, Tests 459/459, IntegrationTests 190/190, dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Brings in develop: code-themes #134/#132 (xshd colorizer wired through TG Scheme code-token VisualRoles), TG pin .95→.98, Editor.MultiCaret.cs whitespace fix, HighlightingColor Role equality. Conflict: tests/.../EditorRenderingTests.cs — resolved by keeping our renamed MultiCaret_Renders_Reverse_Blink_Attribute_On_Text (the reverse-video caret change develop never had) and DROPPING the orphaned UseThemeBackground_Defaults_To_True test: develop removed the UseThemeBackground property in 04315ea ([#132] colorizer rework) with no replacement boolean, so the test referenced dead surface (it only survived on this branch via the earlier intermediate develop merge). Build clean; Tests 460/460; IntegrationTests 205/205; dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adopt Editor PR gui-cs/Editor#134 (syntax-theme phase 2): EditorClet: - Add Theme shortcut to status bar (cycles ThemeManager.Theme) - Markdown preview auto-selects TextMate theme based on terminal background instead of hard-coding DarkPlus ConfigClet: - Add Theme shortcut to status bar - Add ViewportSettings (scrollbars) - Set BorderStyle = LineStyle.None for consistency with EditorClet The Editor class now auto-subscribes to ThemeManager.ThemeChanged internally, so theme cycling triggers proper repaint with VisualRole- driven syntax colors automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adopt Editor PR gui-cs/Editor#134 (syntax-theme phase 2): EditorClet: - Add Theme DropDownList to status bar (selects ThemeManager.Theme) - Markdown preview auto-selects TextMate theme based on terminal background instead of hard-coding DarkPlus ConfigClet: - Add Theme DropDownList to status bar - Add ViewportSettings (scrollbars) - Set BorderStyle = LineStyle.None for consistency with EditorClet The Editor class now auto-subscribes to ThemeManager.ThemeChanged internally, so changing the theme triggers proper repaint with VisualRole-driven syntax colors automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rvey All four beta features (find-and-replace #104, clipboard #107, word-wrap #106, multi-caret #105) plus vertical-multi-caret #133 and syntax-theme Phase 2 #134 have merged since the 2026-05-13 plan snapshot. Bring the spec set back in line with develop: - Per-feature status lines -> Done with PR/issue refs (clipboard, multi-caret, find-and-replace, word-wrap, vertical-multi-caret, syntax-theme). - plan.md rescoped to the beta endgame: new "Done (beta)" table, a "Remaining for beta" that reflects no feature work is left (only external/verification/cut items), evidence-checked DoD boxes, refreshed dependency graph and header. - decisions.md: OPEN-005 annotated as settled by syntax-theme; add OPEN-006 (single-line/input-mode decision). - codex-autonomous-sprint.md: flag stale work pool. - New specs/textview-parity-gap/spec.md: survey of TextView capabilities Editor lacks (autocomplete, overwrite mode, single-line mode, kill-ring, context menu) with dispositions; wired into plan.md follow-ups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements Phase 2 of
specs/syntax-theme/spec.md— tracked by #132, closes #99 and #128 on merge.Routes the xshd syntax highlighter through Terminal.Gui's
Scheme/VisualRolemachinery so themes drive syntax colors and the editor background follows the active scheme.What's here
specs/syntax-theme/spec.md(the 3-phase plan).XshdRoleMap— built-in xshd<Color name>→VisualRole.Code*bridge (spec §6.2). Unmapped names fall back to the xshd-declared color.HighlightingColor.Role+ xshdcategory=— per-<Color>override that wins over the name table (XshdColor/V2Loader/XmlHighlightingDefinition/SaveXshdVisitor). Logged inthird_party/AvaloniaEdit/UPSTREAM.md.HighlightingColorizer.ToAttributerewrite — resolution order: (1) explicit scheme role → (2) xshd foreground over scheme background → (3) default. Ctor takesgetRoleAttribute/isRoleExplicitlySetfuncs supplied byEditor.Editor.UseThemeBackgroundremoved end-to-end — theme decides backgrounds now (the default json colors are unreadable in ted #128 fix).VisualLineBuildContext/VisualLineBuilder/Editor.Drawingcleaned up; the ted demo's "Use Theme Background" UX and theEditorSettings.UseThemeBackgroundsetting retired with it.Editorrepaints onThemeManager.ThemeChanged; ted gains a status-bar Theme shortcut that cyclesThemeManager.Theme.XshdRoleMapcoverage /category=override; integration: scheme-swap render smoke + background-follows-scheme.Status — unblocked & verified ✅
Terminal.Gui #5313 (code-token VisualRoles, TG issue #5311) is published on the develop NuGet feed as
2.1.1-develop.95(the later.125–.163builds are bad/unlisted).Directory.Build.propsis pinned to it. Verified against the real published package:dotnet build— 0 warnings / 0 errorsTerminal.Gui.Editor.Tests— 439 / 439Terminal.Gui.Editor.IntegrationTests— 190 / 190dotnet format --verify-no-changes— cleanBranch is also merged up to current
develop(reconciled with the #123 ted settings-UI port and #136 indent fixes).Merge checklist
<TerminalGuiVersion>bumped to the develop pre-release containing #5313 (2.1.1-develop.95);TODOremoved.🤖 Generated with Claude Code