Skip to content

feat: support QuickPickItem.iconPath#4134

Merged
hacke2 merged 3 commits intomainfrom
feat/support-QuickPickItem.iconPath
Oct 30, 2024
Merged

feat: support QuickPickItem.iconPath#4134
hacke2 merged 3 commits intomainfrom
feat/support-QuickPickItem.iconPath

Conversation

@bk1012
Copy link
Copy Markdown
Member

@bk1012 bk1012 commented Oct 29, 2024

Types

  • 🎉 New Features

Background or solution

Support VSCode API: QuickPickItem.iconPath

image

Changelog

Summary by CodeRabbit

  • 新功能

    • 为快速选择项和快速打开项添加了可选的 iconPath 属性,允许指定图标路径。
    • 更新了快速选择和输入框方法,以支持图标的展示和输入验证的改进。
  • 改进

    • 增强了图标处理逻辑,支持 ThemeIcon 和其他图标类型的区分。
    • 提升了快速选择服务的灵活性,允许更广泛的配置选项。
    • 改进了 iconPath2URI 函数的类型安全性和逻辑处理。
  • 文档

    • 更新了相关接口的类型定义,以反映新的图标路径属性。

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 29, 2024

Walkthrough

该拉取请求对多个文件进行了修改,主要引入了 iconPath 属性到 QuickOpenItemOptionsQuickPickItem 接口中,以支持图标路径的指定。此外,QuickOpenItem 类添加了 getIconPath 方法以访问该属性。在 ExtHostQuickOpen 类的 showQuickPick 方法中,更新了 pickItems 数组以包含图标路径。其他文件也进行了相应的修改,以增强图标处理和快速选择功能的灵活性。

Changes

文件路径 修改摘要
packages/core-browser/src/quick-open/index.ts 添加 iconPath 属性到 QuickOpenItemOptionsQuickPickItem 接口;添加 getIconPath 方法到 QuickOpenItem 类。
packages/extension/src/hosted/api/vscode/ext.host.quickopen.ts 更新 showQuickPick 方法以包含 iconPath 属性;确保 showInputBox 方法中的 validateInputHandler 设置正确。
packages/quick-open/src/browser/quick-open-item.service.ts 修改 getButtons 方法以处理 ThemeIcon;添加 getIconClass 方法以检索图标类。
packages/quick-open/src/browser/quick-open.view.tsx 添加 iconPath 变量并更新渲染逻辑以使用 finalIconClass
packages/quick-open/src/browser/quick-pick.service.ts toItemOptions 方法中添加 iconPath 属性;更新 show 方法的签名以支持更多选项。
packages/quick-open/src/common/icon.ts 更新 iconPath2URI 方法的参数类型以增强类型安全性,添加对 codeUri 属性的条件检查。
packages/types/vscode/typings/vscode.quickpick.d.ts QuickPickItem 接口中添加 iconPath 属性。
packages/quick-open/src/browser/quick-title-bar.ts 修改 buttons 计算属性以处理 ThemeIcon,引入 getExternalIcon

Possibly related PRs

Suggested reviewers

  • Ricbet
  • erha19

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

🧹 Outside diff range and nitpick comments (4)
packages/quick-open/src/common/icon.ts (1)

Line range hint 3-12: 建议添加输入验证

函数缺少对输入参数的有效性验证。当 iconPathundefinednull 时可能会导致运行时错误。

建议添加参数验证:

 export function iconPath2URI(iconPath: URI | { light: URI; dark: URI }, themeType?: string): URI | undefined {
+  if (!iconPath) {
+    return undefined;
+  }
+
   if (URI.isUri(iconPath)) {
     // ... existing code ...
   }
packages/core-browser/src/quick-open/index.ts (1)

202-204: getIconPath 方法实现正确,建议添加方法注释!

方法实现符合类的 getter 模式,但建议添加 JSDoc 注释说明返回值类型和用途。

建议添加如下注释:

+  /**
+   * 获取图标路径
+   * @returns URI、带主题的图标路径对象或主题图标
+   */
  getIconPath(): URI | { light: URI; dark: URI } | ThemeIcon | undefined {
    return this.options.iconPath;
  }
packages/quick-open/src/browser/quick-open.view.tsx (1)

191-199: 图标类名计算逻辑完善!

使用 quickOpenItemService.getIconClass 方法统一处理 iconPathiconClass 的组合是个不错的设计。不过建议考虑添加错误处理。

建议添加错误处理逻辑:

  const finalIconClass = React.useMemo(
    () =>
-     quickOpenItemService.getIconClass({
-       iconPath,
-       iconClass,
-     }),
+     try {
+       return quickOpenItemService.getIconClass({
+         iconPath,
+         iconClass,
+       });
+     } catch (error) {
+       console.warn('获取图标类名失败:', error);
+       return iconClass;
+     },
    [iconClass, iconPath],
  );
packages/quick-open/src/browser/quick-open-item.service.ts (1)

Line range hint 39-59: 建议在getButtons方法中使用getIconClass以消除代码重复

getButtons方法中对于btn.iconPath的处理逻辑与新添加的getIconClass方法存在重复。为提升代码的可维护性和可读性,建议重构getButtons方法,直接调用getIconClass来获取iconClass,以避免重复代码。

建议修改如下:

return buttons.map((btn, i) => {
-  if (ThemeIcon.isThemeIcon(btn.iconPath)) {
-    return {
-      ...btn,
-      iconClass: getExternalIcon(btn.iconPath.id),
-      handle: i,
-    };
-  }
-
-  const iconUri = iconPath2URI(
-    btn.iconPath as URI | { light: URI; dark: URI },
-    this.themeService.getCurrentThemeSync().type,
-  );
-  const iconPath = iconUri && this.staticResourceService.resolveStaticResource(iconUri).toString();
-  const iconClass = iconPath && this.iconService.fromIcon('', iconPath, IconType.Background);
+  const iconClass = this.getIconClass({
+    iconPath: btn.iconPath,
+    iconClass: btn.iconClass,
+  });
   return {
     ...btn,
     iconClass,
     handle: i,
   };
});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 8a905f0 and 2b72ea5.

📒 Files selected for processing (7)
  • packages/core-browser/src/quick-open/index.ts (3 hunks)
  • packages/extension/src/hosted/api/vscode/ext.host.quickopen.ts (1 hunks)
  • packages/quick-open/src/browser/quick-open-item.service.ts (3 hunks)
  • packages/quick-open/src/browser/quick-open.view.tsx (2 hunks)
  • packages/quick-open/src/browser/quick-pick.service.ts (2 hunks)
  • packages/quick-open/src/common/icon.ts (1 hunks)
  • packages/types/vscode/typings/vscode.quickpick.d.ts (1 hunks)
🔇 Additional comments (12)
packages/quick-open/src/common/icon.ts (1)

3-3: 类型定义的改进值得肯定

any 类型改为明确的联合类型提高了类型安全性,这与 VSCode API 的类型定义保持一致。

packages/quick-open/src/browser/quick-pick.service.ts (2)

92-92: 代码实现正确且符合预期!

iconPath 的提取逻辑与现有的其他属性保持一致,正确处理了字符串和 QuickPickItem 类型的判断。


107-107: 验证图标路径的处理逻辑

iconPath 已正确添加到返回对象中。建议验证此更改与现有的图标处理逻辑(如 iconClass)的集成情况。

✅ Verification successful

Based on the search results, I can now generate the final response since I have enough information about how iconPath and iconClass are handled in the codebase. The code shows that:

  1. Both iconPath and iconClass can coexist in the data structure
  2. There's a dedicated getIconClass method in QuickOpenItemService that handles both properties
  3. The method has proper fallback logic: first checks iconClass, then processes iconPath if available
  4. The view component uses this service to resolve the final icon class

Here's the final response:

iconPathiconClass 的集成验证已完成

iconPath 的添加与现有的 iconClass 处理逻辑完全兼容:

  • QuickOpenItemService 中的 getIconClass 方法已经实现了对两种图标属性的优雅处理
  • 视图组件正确使用了这个服务来解析最终的图标样式
  • 现有代码确保了两种图标属性可以共存且有明确的优先级
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 iconPath 和 iconClass 的处理逻辑是否存在冲突

# 测试:搜索与图标相关的处理代码
rg -A 5 "iconPath|iconClass" packages/quick-open/src/browser/

Length of output: 10598

packages/core-browser/src/quick-open/index.ts (2)

128-128: 新增的 iconPath 属性实现完整且符合规范!

属性类型定义完全符合 VSCode API 规范,支持 URI、亮暗主题分离配置以及主题图标。


408-408: QuickPickItem 接口的 iconPath 属性定义完善!

属性定义与 QuickOpenItemOptions 保持一致,确保了接口的统一性。

packages/quick-open/src/browser/quick-open.view.tsx (2)

189-190: 图标路径获取实现正确!

使用 useMemo 缓存 iconPath 值可以避免不必要的重复计算,符合 React 性能优化最佳实践。


271-271: 图标渲染更新正确!

使用 finalIconClass 替换原有的 iconClass 符合新的图标处理逻辑,同时保持了与现有样式系统的兼容性。

packages/types/vscode/typings/vscode.quickpick.d.ts (2)

34-37: 代码变更正确实现了 QuickPickItem.iconPath 支持

新增的 iconPath 属性定义和文档说明完全符合 VSCode API 规范。属性被正确标记为可选,并且类型定义涵盖了所有支持的图标格式(Uri、light/dark Uri 对象和 ThemeIcon)。


37-37: 建议验证与 VSCode API 的兼容性

请确保该实现与 VSCode 的 QuickPickItem.iconPath 的行为保持一致,特别是在以下方面:

  1. 支持的图标格式(Uri、light/dark Uri 对象和 ThemeIcon)
  2. 图标的渲染行为
  3. 主题切换时的响应方式
✅ Verification successful

Let me gather more information about the VSCode API implementation to verify the compatibility.


基于搜索结果,我可以生成最终的验证结果:

iconPath 实现完全符合 VSCode API 规范

该实现与 VSCode API 完全一致:

  • 支持所有标准图标格式:Uri、light/dark Uri 对象和 ThemeIcon
  • 类型定义和文档注释与官方 API 保持一致
  • 在代码库中的实际使用(如 ext.host.quickopen.ts)也验证了实现的正确性
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 验证实现是否与 VSCode API 保持一致
# 搜索相关的测试用例和使用示例

# 查找测试文件
fd -e test.ts -e spec.ts | xargs rg -l "QuickPickItem.*iconPath"

# 查找使用示例
rg "QuickPickItem.*iconPath" -A 5

Length of output: 782


Script:

#!/bin/bash
# 搜索 VSCode 类型定义中的 QuickPickItem 接口定义
rg "interface QuickPickItem" -A 10 --type ts

# 搜索 ThemeIcon 的使用方式
rg "ThemeIcon" -A 3 --type ts

# 查找更多的实现示例
ast-grep --pattern 'interface QuickPickItem {
  $$$
  iconPath?: $_;
  $$$
}'

Length of output: 52691

packages/extension/src/hosted/api/vscode/ext.host.quickopen.ts (1)

98-98: 支持 QuickPickItem.iconPath 功能已正确实现

iconPath 属性的添加符合 VSCode API 的规范,实现方式简洁且正确。类型转换确保了类型安全性。

让我们验证一下 iconPath 的类型定义:

✅ Verification successful

Let's gather more information about the iconPath property definition.


QuickPickItem.iconPath 的类型转换实现正确且安全

从类型定义中可以看到,iconPath 属性在 VSCode 的 QuickPickItem 接口中被正确定义为图标路径或主题图标。当前实现中的类型转换完全符合接口定义,确保了类型安全性。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 QuickPickItem 接口中 iconPath 的类型定义
# 预期:找到 QuickPickItem 接口的定义,确认 iconPath 的类型

rg -A 5 "interface QuickPickItem"

Length of output: 1378


Script:

#!/bin/bash
# Search for the complete QuickPickItem interface definition in VSCode typings
rg -A 15 "interface QuickPickItem {" packages/types/vscode/typings/vscode.quickpick.d.ts

# Search for iconPath usage in the codebase
ast-grep --pattern 'iconPath: $_'

Length of output: 240


Script:

#!/bin/bash
# Search for QuickPickItem interface definition with fixed regex
rg -A 15 "interface QuickPickItem" packages/types/vscode/typings/vscode.quickpick.d.ts

# Search for iconPath usage with ripgrep instead
rg "iconPath.*QuickPickItem" -A 2

Length of output: 1289

packages/quick-open/src/browser/quick-open-item.service.ts (2)

2-2: 导入语句更新

引入了getExternalIconThemeIcon,为支持iconPath功能提供了必要的依赖,符合新增特性需求。

Also applies to: 5-5


61-81: getIconClass方法实现合理

新增的getIconClass方法逻辑完善,考虑了iconClass已存在的情况,以及iconPathThemeIconURI两种类型的处理方式,增强了代码的灵活性和可扩展性。

Comment thread packages/quick-open/src/common/icon.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

🧹 Outside diff range and nitpick comments (1)
packages/quick-open/src/common/icon.ts (1)

3-3: 建议完善返回值类型定义

函数签名的参数类型改进很好,但建议进一步明确返回值类型的语义。

建议修改为:

- export function iconPath2URI(iconPath: URI | { light: URI; dark: URI }, themeType?: string): URI | undefined {
+ export function iconPath2URI(iconPath: URI | { light: URI; dark: URI }, themeType?: string): URI | undefined /* 返回undefined表示转换失败 */ {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 2b72ea5 and d65a834.

📒 Files selected for processing (1)
  • packages/quick-open/src/common/icon.ts (1 hunks)

Comment thread packages/quick-open/src/common/icon.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d65a834 and b078a37.

📒 Files selected for processing (1)
  • packages/quick-open/src/browser/quick-title-bar.ts (2 hunks)
🔇 Additional comments (1)
packages/quick-open/src/browser/quick-title-bar.ts (1)

4-4: 导入语句更改看起来没有问题!

新增的导入语句与功能需求相符,位置放置正确。

Also applies to: 7-7

Comment thread packages/quick-open/src/browser/quick-title-bar.ts
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 29.62963% with 19 lines in your changes missing coverage. Please review.

Project coverage is 54.25%. Comparing base (d5d4dae) to head (b078a37).
Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
.../quick-open/src/browser/quick-open-item.service.ts 23.52% 8 Missing and 5 partials ⚠️
packages/quick-open/src/browser/quick-title-bar.ts 40.00% 2 Missing and 1 partial ⚠️
packages/core-browser/src/quick-open/index.ts 0.00% 2 Missing ⚠️
packages/quick-open/src/common/icon.ts 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4134      +/-   ##
==========================================
- Coverage   54.26%   54.25%   -0.01%     
==========================================
  Files        1598     1598              
  Lines       97564    97598      +34     
  Branches    19946    19964      +18     
==========================================
+ Hits        52941    52954      +13     
- Misses      37074    37079       +5     
- Partials     7549     7565      +16     
Flag Coverage Δ
jsdom 49.81% <29.62%> (-0.01%) ⬇️
node 15.60% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hacke2
Copy link
Copy Markdown
Member

hacke2 commented Oct 30, 2024

图片是不是稍微有点大?调小一点?

Copy link
Copy Markdown
Member

@hacke2 hacke2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hacke2 hacke2 merged commit 17181cb into main Oct 30, 2024
@hacke2 hacke2 deleted the feat/support-QuickPickItem.iconPath branch October 30, 2024 08:33
@coderabbitai coderabbitai Bot mentioned this pull request Oct 31, 2024
1 task
@coderabbitai coderabbitai Bot mentioned this pull request Nov 12, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎨 feature feature required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants