-
Notifications
You must be signed in to change notification settings - Fork 65
TODO: feat: Add Winter Theme with snowflake-decorated logo #373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces winter theme support throughout the codebase. Adds a new Changes
Sequence DiagramssequenceDiagram
participant User
participant UI as MiscView
participant Settings as Settings Manager
participant Patches as Patch Engine
participant Theme as Winter Theme Module
User->>UI: Toggle "Enable Winter theme"
UI->>Settings: updateSettings({misc.enableWinterTheme: true})
Settings->>Patches: Apply patches with config
Patches->>Theme: Check enableWinterTheme
alt enableWinterTheme enabled
Patches->>Theme: writeWinterTheme(content)
Theme->>Theme: Apply multiple patch steps
Note over Theme: Force winter hook<br/>Add switch case<br/>Replace logos<br/>Update palettes
Theme-->>Patches: Modified content
Patches->>Patches: Write patched content
end
Patches-->>Settings: Complete
Settings-->>UI: Settings updated
UI-->>User: Theme applied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @package.json:
- Line 69: The package "@anthropic-ai/claude-code" is listed as a runtime
dependency but is only used for path detection/string construction; remove it
from "dependencies" in package.json and either delete the entry entirely or move
it under "devDependencies" instead; update any build or packaging scripts that
assume it exists at runtime (search for the literal "@anthropic-ai/claude-code"
usage) and run npm/yarn install to verify no runtime imports break.
In @patch-native-216.js:
- Around line 13-16: The code uses execSync to run Unix-only utilities via the
jsCode assignment (execSync(`strings "${BINARY_PATH}" | sed -n '/^#!/,/^$/p'`)),
which will fail on Windows; update the code to check the platform
(process.platform) before invoking execSync and either (a) throw or log a clear
platform-not-supported error when process.platform === 'win32' or (b) implement
or call a cross-platform alternative to extract the script header from
BINARY_PATH, and ensure the branch is covered so jsCode is only produced when
the platform supports those utilities; also document this behavior in the
surrounding comment or error message so callers know the platform requirement.
- Line 10: The constant BINARY_PATH is hardcoded to a user-specific location
which will break for other environments; change BINARY_PATH to be configurable
by first checking an environment variable (e.g. process.env.CLAUDE_BINARY_PATH)
and then a command-line argument fallback (e.g. process.argv[...] or a parsed
flag), with a sensible default or error if neither is provided; update any
usages of BINARY_PATH in this module so they read the new configurable value
(preserve the symbol name BINARY_PATH so callers remain unchanged).
In @src/patches/winterTheme.ts:
- Around line 571-577: Several RGB string values in the winter theme are
inconsistent because they include spaces after commas (userMessageBackground,
bashMessageBackgroundColor, memoryBackgroundColor); update those string literals
to the same compact 'rgb(r,g,b)' format used elsewhere (e.g., clawd_body,
rate_limit_fill) so parsing is consistent — find and edit the keys
userMessageBackground, bashMessageBackgroundColor, and memoryBackgroundColor in
the winterTheme palette to remove the spaces after commas.
In @WINTER_THEME.md:
- Around line 65-67: Update the prerequisite in WINTER_THEME.md by replacing the
string "Claude Code 2.1.5 NPM installation" with "Claude Code 2.1.6 NPM
installation" so the doc matches the patch script; verify consistency with
patch-native-216.js where the version comment, BINARY_PATH, and the installation
command reference 2.1.6 and ensure the README text exactly matches that version.
In @winter-patch.js:
- Around line 198-201: Replace the incorrect middle segment in row 2 that
currently uses the pattern "▛███▜" with five full blocks "█████" so the line
`${createFnVar}.createElement(${textVar},{color:"ice_blue",backgroundColor:"clawd_background"},"▛███▜"),`
becomes the variant that renders "█████"; update the string passed to
createElement (identify via createFnVar and textVar) to match the expected
"█████" pattern used in WINTER_THEME.md and winterTheme.ts.
- Around line 50-56: The current regex hardcodes the function name and variable
names (function aB, A, Q) making it brittle; update the pattern passed to
applyRegexOnce to capture the function name and the two return variable names
dynamically (e.g., capture groups for the function identifier and the two vars)
and also include a lenient fallback form, then update the replacement callback
used by applyRegexOnce to reuse the captured function name and variable names
(use the captured name and second var for the replacement return value
["winter", <secondVar>]) while keeping the same patch id 'force-winter-hook'.
🧹 Nitpick comments (5)
patch-native-216.js (1)
1-59: Consider whether this development script belongs in the repository.This script provides manual patching instructions and cannot actually complete the patching workflow. If it's intended as developer documentation or a debugging aid, consider:
- Moving it to a
scripts/ortools/directory- Adding a clear disclaimer in the filename or header (e.g.,
patch-native-216-debug.js)- Documenting its purpose more clearly
src/patches/index.ts (1)
710-720: Clarify the relationship between theme addition and writeWinterTheme.The winter theme is added to
themesToApply(lines 515-531) ANDwriteWinterThemeis called here. This two-step approach works because:
writeThemesadds the theme colors to the switch statementwriteWinterThemepatches the logo, forces the theme hook, adjusts layout, etc.Consider adding a brief comment to clarify this isn't duplication:
📝 Suggested clarification
+ // Apply winter theme patch (logo snowflakes, force hook, layout adjustments) + // Note: Winter theme colors were already added via themesToApply above if (config.settings.misc?.enableWinterTheme) { if ((result = writeWinterTheme(content))) {WINTER_THEME.md (2)
7-11: Add language specifier to code block for better rendering.Per markdownlint MD040, fenced code blocks should specify a language. Since this is ASCII art, use
textor leave empty with proper justification.📝 Suggested fix
-``` +```text * ▐▛███▜▌ * * ▝▜█████▛▘ * * ▘▘ ▝▝ *</details> --- `78-82`: **Add language specifier to path example code block.** <details> <summary>📝 Suggested fix</summary> ```diff - ``` + ```text ~/.local/share/fnm/node-versions/v24.1.0/installation/lib/node_modules/@anthropic-ai/claude-code/cli.js ```src/patches/winterTheme.ts (1)
136-140: Low-risk ReDoS concern with dynamic regex.Static analysis flagged this regex as potentially vulnerable since
reactVaris derived from input. However, the risk is low because:
reactVaris extracted from minified JavaScript identifiers (alphanumeric +$)- The extraction pattern on line 119 already constrains it to
[$\w]+For defense-in-depth, consider escaping the variable:
🛡️ Optional hardening
+ // reactVar is already constrained to [$\w]+ but escape for safety const textMatch = logoArea.match( - new RegExp(`${reactVar}\\.createElement\\(([$\\w]+),\\s*null`) + new RegExp(`${reactVar.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\.createElement\\(([$\\w]+),\\s*null`) );Alternatively, create a helper similar to
escapeIdentfromindex.ts.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.cursorrulesWINTER_THEME.mdpackage.jsonpatch-native-216.jssrc/defaultSettings.tssrc/installationDetection.tssrc/patches/index.tssrc/patches/winterTheme.tssrc/types.tssrc/ui/components/MiscView.tsxwinter-patch.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-31T20:58:54.456Z
Learnt from: signadou
Repo: Piebald-AI/tweakcc PR: 322
File: package.json:84-86
Timestamp: 2025-12-31T20:58:54.456Z
Learning: In package.json, ensure engines.node reflects the runtime requirement (the minimum Node.js version needed to run the built artifacts), not the build-time tool. For this project, set "engines": {"node": ">=20.0.0"} if runtime requires Node 20.x, even if the build tool (e.g., tsdown) needs a newer version.
Applied to files:
package.json
🧬 Code graph analysis (2)
src/patches/winterTheme.ts (2)
src/patches/index.ts (1)
showDiff(91-129)src/types.ts (1)
Theme(1-67)
src/patches/index.ts (2)
src/patches/winterTheme.ts (2)
WINTER_THEME_COLORS(516-578)writeWinterTheme(419-513)src/patches/themes.ts (1)
writeThemes(53-128)
🪛 ast-grep (0.40.5)
src/patches/winterTheme.ts
[warning] 137-137: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${reactVar}\\.createElement\\(([$\\w]+),\\s*null)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 markdownlint-cli2 (0.18.1)
WINTER_THEME.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
17-17: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (10)
src/installationDetection.ts (1)
164-183: LGTM! Robust version extraction with dual pattern support.The implementation correctly aggregates version occurrences from both minified and comment formats, selecting the most frequent. This improves detection reliability for various Claude Code installation formats.
src/types.ts (1)
118-118: LGTM!The
enableWinterThemeboolean field is appropriately placed in theMiscConfiginterface, consistent with other misc settings.src/defaultSettings.ts (1)
758-758: LGTM!The
enableWinterThemedefault is set tofalse, making this seasonal feature opt-in. This is the appropriate default behavior.src/ui/components/MiscView.tsx (2)
36-36: LGTM!The
defaultMiscfallback object correctly includesenableWinterTheme: false, matchingDEFAULT_SETTINGS.
199-212: LGTM! Clear UI toggle for the winter theme feature.The new misc item follows the established pattern of other toggle items. The description appropriately notes that changes must be applied to take effect, which is good UX.
package.json (1)
49-49: React 19 is fully supported by Ink 6.1.0. The v6 release specifically added React 19 support (peer dependency: React >=18.0.0), so the@types/react^19.0.0 andreact^19.1.1 bump is compatible and requires no changes.src/patches/index.ts (1)
515-531: LGTM! Winter theme integration with existing theme system.The logic correctly:
- Preserves existing themes from config
- Checks for duplicate winter theme before adding
- Creates a new array to avoid mutating the config
winter-patch.js (1)
256-261: Limited React alias coverage.Only
I5andE9variants are handled. The TypeScript version (winterTheme.ts) uses dynamic detection to find React variables. If future versions use different aliases, this script will silently fail to patch logos.src/patches/winterTheme.ts (2)
418-513: Well-structured patch orchestration with good observability.The
writeWinterThemefunction:
- Tracks applied and failed patches for debugging
- Returns
nullwhen no changes needed (idempotent)- Logs detailed status information
- Applies patches in correct order (logos before palette to avoid position shifts)
515-578: LGTM! Complete winter theme color palette.The
WINTER_THEME_COLORSexport provides a full color palette matching theTheme['colors']interface with appropriate ice-blue accents for the winter aesthetic.
| clawd_body: 'rgb(173,216,230)', // ice blue | ||
| clawd_background: 'rgb(0,0,0)', | ||
| userMessageBackground: 'rgb(55, 55, 55)', | ||
| bashMessageBackgroundColor: 'rgb(65, 60, 65)', | ||
| memoryBackgroundColor: 'rgb(55, 65, 70)', | ||
| rate_limit_fill: 'rgb(173,216,230)', // ice blue | ||
| rate_limit_empty: 'rgb(80,83,112)', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent RGB format may cause parsing issues.
These values have spaces after commas while all other RGB values in the palette don't:
userMessageBackground: 'rgb(55, 55, 55)', // has spaces
bashMessageBackgroundColor: 'rgb(65, 60, 65)', // has spaces
memoryBackgroundColor: 'rgb(55, 65, 70)', // has spacesIf the consuming code uses strict parsing, this inconsistency could cause issues.
🔧 Suggested fix for consistency
- userMessageBackground: 'rgb(55, 55, 55)',
- bashMessageBackgroundColor: 'rgb(65, 60, 65)',
- memoryBackgroundColor: 'rgb(55, 65, 70)',
+ userMessageBackground: 'rgb(55,55,55)',
+ bashMessageBackgroundColor: 'rgb(65,60,65)',
+ memoryBackgroundColor: 'rgb(55,65,70)',🤖 Prompt for AI Agents
In @src/patches/winterTheme.ts around lines 571 - 577, Several RGB string values
in the winter theme are inconsistent because they include spaces after commas
(userMessageBackground, bashMessageBackgroundColor, memoryBackgroundColor);
update those string literals to the same compact 'rgb(r,g,b)' format used
elsewhere (e.g., clawd_body, rate_limit_fill) so parsing is consistent — find
and edit the keys userMessageBackground, bashMessageBackgroundColor, and
memoryBackgroundColor in the winterTheme palette to remove the spaces after
commas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @winter-patch.js:
- Around line 346-351: The loop in winter-patch.js hardcodes minified React
variable names ['I5','E9'] which will break on other builds; replace that static
list by reusing the dynamic discovery logic used in winterTheme.ts (the same
pattern used by replaceLogoWithSnowflakes) to detect the correct create-element
variable(s) in src before calling replaceCreateElementReturnBlock, and if no
candidates are discovered emit a clear warning or add a documented fallback
path; update comments to note these values are build/version-specific and ensure
you reference replaceCreateElementReturnBlock and replaceLogoWithSnowflakes when
adding the discovery and fallback logic.
🧹 Nitpick comments (4)
src/patches/winterTheme.ts (1)
136-140: Static analysis flagged potential ReDoS, but risk is low here.The
reactVaris captured from a prior regex that only matches[$\w]+characters, so it cannot contain malicious patterns. The constructed regex is also simple without nested quantifiers.Consider adding a brief comment documenting this safety assumption for future maintainers:
+ // reactVar is safe - captured from [$\w]+ pattern, cannot contain regex metacharacters const textMatch = logoArea.match( new RegExp(`${reactVar}\\.createElement\\(([$\\w]+),\\s*null`) );winter-patch.js (2)
1-17: Hardcoded version limits script reusability.
TARGET_VERSION = '2.1.5'means this script will need manual updates for each new Claude Code version. Consider:
- Accepting version as a CLI argument with a default
- Making the version check optional via a
--forceflag- Documenting this limitation prominently
-const TARGET_VERSION = '2.1.5'; +const TARGET_VERSION = process.env.WINTER_PATCH_VERSION || '2.1.5';
70-167: Significant code duplication withwinterTheme.ts.Functions
patchForceWinterInThemeHook,patchThemeSwitchToAcceptWinter,patchThemePickerOptions,patchThemeLabelMap, andpatchAddIceBlueToPaletteslargely duplicate logic fromsrc/patches/winterTheme.ts.If both files need to be maintained, consider:
- Extracting shared logic to a common module
- Generating this script from the TypeScript source
- Clearly documenting that changes must be synced between files
Otherwise, if this is a one-off helper script for a specific version, consider adding a deprecation notice or moving it to a
scripts/ortools/directory.src/patches/index.ts (1)
714-724: Potential redundancy in winter theme switch case handling, though safe due to idempotency.The winter theme is added to
themesToApply(lines 519-535), which causeswriteThemesto rebuild the entire switch statement includingcase"winter"with its colors. Later,writeWinterThemecallspatchAddWinterCaseToSwitchwhich attempts to add the same case again.However,
patchAddWinterCaseToSwitchincludes an idempotency check (line 370) that detects an existingcase"winter"and returnschanged: false, preventing duplication. The code is safe but adds unnecessary complexity with dual paths handling the same concern.Consider whether
patchAddWinterCaseToSwitchis still needed when winter theme is already handled throughwriteThemes, or document why both paths are maintained.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.cursorrulespackage.jsonsrc/defaultSettings.tssrc/installationDetection.tssrc/patches/index.tssrc/patches/winterTheme.tssrc/types.tssrc/ui/components/MiscView.tsxwinter-patch.js
🚧 Files skipped from review as they are similar to previous changes (5)
- src/types.ts
- package.json
- src/ui/components/MiscView.tsx
- src/installationDetection.ts
- src/defaultSettings.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/patches/index.ts (2)
src/patches/winterTheme.ts (2)
WINTER_THEME_COLORS(519-581)writeWinterTheme(422-516)src/patches/themes.ts (1)
writeThemes(53-128)
🪛 ast-grep (0.40.5)
src/patches/winterTheme.ts
[warning] 137-137: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${reactVar}\\.createElement\\(([$\\w]+),\\s*null)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (14)
src/patches/winterTheme.ts (8)
1-17: LGTM: Clear module documentation.The header comment provides a helpful overview of all patches applied by this module.
23-55: LGTM: Robust parenthesis balancing with string awareness.The function correctly handles escape sequences and all three string literal types, which is important for parsing minified JavaScript.
58-89: LGTM: Palette patching with idempotency.The negative lookahead patterns ensure ice_blue is not added multiple times.
269-287: LGTM: Safe bounded loop for logo replacement.The
snowflakeCount < 5limit prevents runaway processing while allowing for multiple logo instances.
296-363: LGTM: Robust pattern matching with bounded fallback.The lenient fallback pattern correctly uses
[^}]{0,500}to prevent catastrophic backtracking while still matching variations in minified output.
365-419: LGTM: Multiple fallback insertion points for robustness.Good defensive approach with cascading fallbacks (dark-daltonized → monochrome → default) and bounded regex patterns.
421-516: LGTM: Well-orchestrated patch sequence with clear diagnostics.The function provides good visibility into which patches were applied vs. skipped, helpful for debugging.
518-581: LGTM: Complete winter theme color palette.The palette covers all required color keys with consistent RGB formatting and helpful inline comments indicating the ice-blue themed colors.
src/patches/index.ts (3)
34-37: LGTM: Helpful guidance for patch authors.This note about using indexOf/includes before applying regex is valuable for preventing catastrophic backtracking.
70-70: LGTM: Import for winter theme module.
519-540: LGTM: Clean immutable theme list construction.The approach correctly:
- Avoids mutating
config.settings.themes- Checks for existing winter theme to prevent duplicates
- Uses the computed
themesToApplyfor downstream processingwinter-patch.js (3)
23-68: LGTM: Simple utility functions.The
applyRegexOncepattern with labeled results is clean for tracking which patches were applied.
197-290: LGTM: Balanced parenthesis parsing for logo block replacement.The logic correctly handles string literals and escape sequences when finding block boundaries.
358-365: Minor: Redundant file read for backup.Line 360 reads the file again, but the original content is already in
srcfrom line 322. However,srcmay have been modified by patches at this point, so reading the file again is actually correct for getting the unpatched content.Wait - on re-examination, the backup should contain the original unpatched content, but by line 360,
srchas been modified by all the patch steps. So reading from the file is correct only if we haven't written yet - which we haven't (write happens at line 363). But line 360 readstargetwhich hasn't been written yet, so it still contains the original.This is correct but could be clearer by storing the original content at the start.
Adds a seasonal winter theme to Claude Code with: - Ice-blue color palette based on dark theme - Snowflake-decorated Clawd ASCII logo (* decorations) - Force winter theme via patched theme hook - Proper logo container height for 3-row snowflakes - ice_blue color added to RGB and ANSI palettes New settings in misc section: - enableWinterTheme: toggle winter theme on/off (default: false) Also includes helper scripts for native binary patching.
Adds a seasonal winter theme to Claude Code with:
New settings in misc section:
Also includes helper scripts for native binary patching.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.