fix(cli): honor custom theme border.default when terminal reports OSC 11 background#27887
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where custom theme border colors were being ignored or overwritten by terminal background color detection. By introducing explicit tracking for user-defined border colors and updating the theme creation logic, the system now correctly honors custom Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
📊 PR Size: size/M
|
There was a problem hiding this comment.
Code Review
This pull request introduces changes to preserve explicit custom border colors when a terminal background is set, refactoring custom theme creation and caching to track whether a theme has an explicit border color. The reviewer identified an inconsistency in the precedence of explicitBorderDefault and customTheme.DarkGray when resolving colors.DarkGray versus semanticColors.border.default, and suggested a fix to ensure consistent behavior.
| }, | ||
| border: { | ||
| default: colors.DarkGray, | ||
| default: customTheme.DarkGray ?? explicitBorderDefault ?? colors.DarkGray, |
There was a problem hiding this comment.
There is an inconsistency in how border.default is resolved.
In colors.DarkGray (lines 424-431), explicitBorderDefault takes precedence over customTheme.DarkGray:
DarkGray:
explicitBorderDefault ??
customTheme.DarkGray ??
...However, in semanticColors.border.default (line 611), customTheme.DarkGray takes precedence over explicitBorderDefault:
border: {
default: customTheme.DarkGray ?? explicitBorderDefault ?? colors.DarkGray,
},If a user configures both border.default and DarkGray in their custom theme, this inconsistency causes the border color to change depending on whether the terminal background is detected/set (since ThemeManager.getSemanticColors() overwrites border.default with colors.DarkGray when a terminal background is active).
To ensure consistent behavior, explicitBorderDefault should take precedence in both places.
| default: customTheme.DarkGray ?? explicitBorderDefault ?? colors.DarkGray, | |
| default: explicitBorderDefault ?? customTheme.DarkGray ?? colors.DarkGray, |
There was a problem hiding this comment.
Fixed in 3627886 — semanticColors.border.default now uses explicitBorderDefault ?? customTheme.DarkGray ?? colors.DarkGray, matching the colors.DarkGray precedence.
7cde408 to
3627886
Compare
… 11 background fix(cli): honor custom theme border.default under OSC 11;fix(cli): align border.default precedence in semantic colors
3627886 to
a7377c3
Compare
Summary
Makes documented custom theme border colors actually apply, including on terminals that report background color via OSC 11.
Fixes #27786
Problem
docs/cli/themes.mddocumentsborder.default(andborder.focused), but two code paths prevented custom border colors from sticking:createCustomTheme()ignoredborder.default—semanticColors.border.defaultalways came from computedDarkGray.ThemeManager.getColors()always recomputedDarkGraywhen OSC 11 background was detected, even if the user explicitly configured border colors. That override flowed intogetSemanticColors()viaborder.default.Additionally, when loading settings themes,
DEFAULT_THEME.colorsis merged in first. That injects a defaultDarkGraywhich previously took precedence over a user-providedborder.default.Root cause
Fix
createCustomTheme()border.defaultand use it before fallback interpolation.border.defaultover merged-in defaultDarkGray.ThemeManagerborder.defaultorDarkGrayin the raw settings entry).hasExplicitBorderColorintocreateCustomTheme().DarkGraywhen the user explicitly set border colors.Tests
border.defaultmaps tocolors.DarkGrayandsemanticColors.border.default.border.default: '#282c34'keeps that color aftersetTerminalBackground('#282c34').Testing