-
Notifications
You must be signed in to change notification settings - Fork 398
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
feat: support property ThemeColor id #4298
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options. 变更概览变更说明本次拉取请求(PR)涉及三个文件中的类型和接口定义的修改,主要集中在颜色和主题相关的类型处理上。变更包括为 变更
可能相关的 PR
建议的审阅者
时序图由于变更主要涉及类型定义,不适合生成时序图。 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (2)
packages/extension/src/hosted/api/vscode/ext.host.statusbar.ts (2)
82-82
: 建议在注释中补充_backgroundColor
的使用范围。
为了帮助后续维护者快速了解该属性的用途,最好在声明处或其上方添加简要备注说明_backgroundColor
主要用于何种场景、可取值范围以及与_color
的差异。
155-155
: 在 setter 内进行字符串检查时存在潜在可维护性风险。
在 setter 中同时使用ALLOWED_BACKGROUND_CSS_VARS
与ALLOWED_BACKGROUND_COLORS
来限制可用色值。若后期需要扩展可用色值集,可能需要修改多个地方。可以考虑建立统一常量来源或集中配置机制,提高可维护性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/extension/src/common/vscode/ext-types.ts
(1 hunks)packages/extension/src/hosted/api/vscode/ext.host.statusbar.ts
(2 hunks)packages/types/vscode/typings/vscode.theme.d.ts
(1 hunks)
🔇 Additional comments (3)
packages/extension/src/hosted/api/vscode/ext.host.statusbar.ts (1)
151-151
: 严格仅返回ThemeColor | undefined
可能导致类型检查失败的风险。
目前对backgroundColor
的 getter 仅返回ThemeColor | undefined
,如有外部调用者期望使用 string 类型(比如自定义颜色值)可能会出现异常。需要确保在外部完全移除对 string 的依赖或添加相应兼容处理逻辑。packages/types/vscode/typings/vscode.theme.d.ts (1)
7-10
: 为ThemeColor
新增只读属性id
的变化可能破坏对某些外部插件的兼容性。
第三方若曾依赖可写的id
属性进行动态变更,更新后可能导致编译或运行时错误。可在文档中补充变更说明,引导使用者升级相应逻辑。如果需要,我可以帮忙写一段向后兼容的插桩代码,或者起草快速迁移指南。
packages/extension/src/common/vscode/ext-types.ts (1)
994-994
: 确保只读属性id
设置后不可修改。
当前只读属性设计可防止外部强行更新ThemeColor.id
,符合主题色标识一经创建就不可更改的业务逻辑。不过在某些动态主题切换场景,需要确保不会影响系统的颜色更新机制。
…into feature-themeColor
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
🧹 Nitpick comments (1)
packages/terminal-next/src/common/pty.ts (1)
542-542
: 类型定义保持一致性!
IShellLaunchConfig
接口中color
属性的类型定义更新与ITerminalProfile
保持一致,这是一个很好的改进:
- 确保了终端配置和启动配置之间的类型兼容性
- 统一了颜色属性的处理方式
- 提供了相同的主题支持能力
建议在项目中添加一个集中的类型定义文件(如
types/terminal.ts
),统一管理这些共享的类型定义,以避免将来可能出现的类型不一致问题。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/terminal-next/src/common/profile.ts
(1 hunks)packages/terminal-next/src/common/pty.ts
(1 hunks)packages/types/vscode/typings/vscode.window.d.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
🔇 Additional comments (1)
packages/terminal-next/src/common/profile.ts (1)
57-57
: 类型定义更新符合预期!扩展
color
属性的类型定义增加了主题色支持,使终端配置更加灵活。支持以下类型:
- 字符串类型的颜色值
- VS Code 主题色
- 可选(undefined)
这样的改动有助于:
- 更好地集成 VS Code 的主题系统
- 保持与终端配置的向后兼容性
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4298 +/- ##
==========================================
- Coverage 54.20% 54.20% -0.01%
==========================================
Files 1634 1634
Lines 99941 99941
Branches 21703 21703
==========================================
- Hits 54177 54170 -7
- Misses 38021 38027 +6
- Partials 7743 7744 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…elete type string
…elete type string
…elete type string
…elete type string
Types
Background or solution
Changelog
Summary by CodeRabbit
新功能
ThemeColor
类添加了id
属性,提供颜色的唯一标识符。改进
类型更新