Skip to content
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

fix: when using language pack extensions, extensions that use i10n do not load correctly #4181

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xkaede
Copy link
Contributor

@xkaede xkaede commented Nov 24, 2024

Types

  • 🐛 Bug Fixes

Background or solution

1、使用插件:vscode-language-pack-zh-hans-1.96.0、json-language-features-1.88.1
2、切换语言使用zh-cn。
3、打开settings里的json相关配置描述,显示的是英文的。
3、编写一个有语法错误的json。显示的语言是英文的。

Changelog

Summary by CodeRabbit

  • 新特性

    • 更新了多个方法以支持异步操作,确保更好的性能和响应性。
    • 在配置贡献中集成了本地化处理,以改善用户体验。
  • 修复

    • 修改了语言包获取方法的返回类型,确保一致性和可靠性。
  • 文档

    • 更新了方法签名以反映异步处理的变化。

@opensumi opensumi bot added 🎨 feature feature required 🐞 bug Something isn't working labels Nov 24, 2024
Copy link
Contributor

coderabbitai bot commented Nov 24, 2024

Walkthrough

此拉取请求包含多个文件的更改,主要涉及将多个方法的返回类型更改为返回 Promise,以支持异步操作。具体而言,getLanguagePack 方法在多个类中进行了更新,以确保其返回类型一致。其他方法如 $fetchBuiltInBundleUricontribute 也进行了相应的异步处理修改。此外,downloadExtension 函数的签名也进行了更新,以增加一个新的参数 version,简化了逻辑。

Changes

文件路径 更改摘要
packages/extension/mocks/extension.service.client.ts 更新 getLanguagePack 方法的返回类型为 Promise<void>
packages/extension/src/browser/vscode/api/main.thread.localization.ts $fetchBuiltInBundleUri 方法修改为异步,使用 await 调用 getLanguagePack
packages/extension/src/browser/vscode/contributes/configuration.ts contribute 方法修改为异步,并添加 localizationsContributionPoint 依赖。
packages/extension/src/browser/vscode/contributes/localization.ts 添加私有成员 _whenContributed 和 getter 方法 whenContributed,并修改 contribute 方法。
packages/extension/src/common/extension.ts 更新 IExtensionNodeClientService 接口中的 getLanguagePack 方法返回类型为 Promise
packages/extension/src/node/extension.service.client.ts 更新 getLanguagePack 方法的返回类型为 Promise,确保一致性。
packages/startup/entry/web-lite/lite-module/extension/index.ts 更新 getLanguagePack 方法的返回类型为 Promise,以支持异步操作。
scripts/download.js 更新 downloadExtension 函数的签名,添加 version 参数,并简化逻辑。

Possibly related PRs

Suggested reviewers

  • Ricbet

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 (1.23.1)

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options.
warning eslint > @humanwhocodes/[email protected]: Use @eslint/config-array instead
warning eslint > @humanwhocodes/config-array > @humanwhocodes/[email protected]: Use @eslint/object-schema instead
warning eslint > file-entry-cache > flat-cache > [email protected]: Rimraf versions prior to v4 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > [email protected]: Glob versions prior to v9 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > glob > [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
warning eslint-import-resolver-typescript > [email protected]: Glob versions prior to v9 are no longer supported
error Couldn't find any versions for "@opensumi/ide-dev-tool" that matches "workspace:*"


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
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: 0

🧹 Outside diff range and nitpick comments (5)
packages/startup/entry/web-lite/lite-module/extension/index.ts (1)

Line range hint 26-71: 建议完善 ExtensionClientService 类的实现

目前该类中大多数方法都未实现,仅抛出错误。考虑到这是一个关键的扩展服务类,建议:

  1. 明确标注哪些方法计划在后续实现
  2. 添加相关的 TODO 注释说明实现计划
  3. 考虑是否需要添加临时的模拟实现以支持测试

如果需要帮助实现这些方法,我可以提供具体的实现建议。是否需要我创建一个 GitHub issue 来跟踪这项工作?

packages/extension/src/browser/vscode/contributes/localization.ts (2)

66-70: 优秀的异步控制流设计!

通过引入 _whenContributed 延迟对象和对应的 getter,使得其他组件可以等待本地化加载完成。这个设计很好地解决了语言包加载时序的问题。

这种模式在处理初始化依赖时特别有用,建议在其他类似场景中也考虑采用。


128-128: 建议增强错误处理机制

虽然添加 this._whenContributed.resolve() 很好地标记了贡献完成,但建议添加错误处理以确保在发生异常时能够正确通知等待方。

建议按照以下方式修改:

  async contribute() {
+   try {
      const promises: Promise<void>[] = [];
      // ... existing code ...
      await Promise.all(promises);
      this._whenContributed.resolve();
+   } catch (error) {
+     this._whenContributed.reject(error);
+     throw error;
+   }
  }
scripts/download.js (1)

161-164: 建议:确保语言包扩展的完整性

考虑到此PR的主要目标是修复语言包扩展的加载问题,建议:

  1. 在解压缩后验证语言包扩展的目录结构完整性
  2. 考虑添加对 package.nls.json 文件的特殊处理
  3. 为语言包扩展添加专门的验证步骤

这些改进将有助于确保语言包扩展能够正确加载和运行。

packages/extension/src/node/extension.service.client.ts (1)

249-249: 返回类型更改符合预期!

getLanguagePack 方法的返回类型更改为 Promise 是正确的,这样可以:

  • 确保与其他异步方法的一致性
  • 支持语言包的异步加载流程
  • 解决语言包扩展加载不正确的问题

建议添加 JSDoc 注释来说明该方法的异步性质:

+ /**
+  * 异步获取指定语言的语言包
+  * @param languageId 语言标识符
+  * @returns Promise<IExtensionLanguagePackMetadata[string] | undefined>
+  */
  public getLanguagePack(languageId: string) {
    const languagePacks = this.languagePackCache?.[languageId];
    return Promise.resolve(languagePacks);
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 5ddaa0b and 93b8fbf.

📒 Files selected for processing (8)
  • packages/extension/__mocks__/extension.service.client.ts (1 hunks)
  • packages/extension/src/browser/vscode/api/main.thread.localization.ts (1 hunks)
  • packages/extension/src/browser/vscode/contributes/configuration.ts (2 hunks)
  • packages/extension/src/browser/vscode/contributes/localization.ts (3 hunks)
  • packages/extension/src/common/extension.ts (1 hunks)
  • packages/extension/src/node/extension.service.client.ts (1 hunks)
  • packages/startup/entry/web-lite/lite-module/extension/index.ts (1 hunks)
  • scripts/download.js (2 hunks)
🔇 Additional comments (14)
packages/extension/__mocks__/extension.service.client.ts (1)

52-52: 修改正确,符合异步接口要求

此更改将 getLanguagePack 方法修改为返回 Promise,与实际服务实现保持一致,有助于解决语言包加载问题。

运行以下脚本验证真实实现的行为:

✅ Verification successful

Mock实现与真实服务行为一致,验证通过

从代码搜索结果可以看到,真实服务packages/extension/src/node/extension.service.client.ts中的getLanguagePack方法也是返回Promise.resolve(),与mock实现保持一致。对于未找到语言包的情况,两者都返回undefined,这是符合预期的行为。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证真实服务中 getLanguagePack 的实现

# 搜索 getLanguagePack 方法的所有实现
ast-grep --pattern 'getLanguagePack(languageId: string)' 

Length of output: 58


Script:

#!/bin/bash
# 搜索包含 getLanguagePack 的所有文件
rg "getLanguagePack" -l

# 查看包含 getLanguagePack 的具体实现
rg "getLanguagePack" -A 5

Length of output: 3883

packages/extension/src/browser/vscode/api/main.thread.localization.ts (2)

34-34: 异步调用修改正确!

getLanguagePack 的调用改为异步是正确的做法,这样可以确保语言包能够正确加载。


Line range hint 34-38: 建议验证语言回退逻辑

当前实现在获取语言包时使用 language || this.currentLanguage 作为回退机制。建议验证以下场景:

  1. 指定语言不存在时的回退行为
  2. 当前语言为空时的处理

运行以下脚本来验证语言包的处理逻辑:

✅ Verification successful

Let me gather more information about the language fallback logic.


根据收集到的信息,我可以生成最终响应了。

语言回退逻辑已经得到适当处理

代码分析显示当前实现是安全的:

  • currentLanguage 在初始化时通过 preferenceService 设置了默认值 'en'
  • 语言包获取逻辑使用 language || this.currentLanguage 确保了始终有一个有效值
  • 系统有完整的语言包缓存机制处理不存在的语言情况

建议:

  • 可以考虑添加单元测试来验证这些场景,但现有实现本身是合理的
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查语言包相关的实现和测试用例

# 检查语言包相关的测试用例
rg -A 5 "getLanguagePack" --type ts

# 检查语言相关的配置处理
ast-grep --pattern 'general.language'

Length of output: 2567


Script:

#!/bin/bash
# 检查当前语言的初始化和设置逻辑
ast-grep --pattern 'this.currentLanguage'

# 检查语言相关的错误处理
rg -A 3 "catch.*language" --type ts

# 检查语言包缓存的实现
rg -A 5 "languagePackCache" --type ts

# 检查语言相关的配置和默认值
rg "defaultLanguage|fallbackLanguage" --type ts

Length of output: 4101

🧰 Tools
🪛 Biome (1.9.4)

[error] 35-36: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/startup/entry/web-lite/lite-module/extension/index.ts (1)

33-35: ⚠️ Potential issue

需要实现 getLanguagePack 方法以解决语言包加载问题

方法签名的更改是正确的,将返回类型改为 Promise 有助于异步加载语言包。但是,目前的实现仍然抛出"未实现"错误,这需要被替换为实际的语言包加载逻辑。

运行以下脚本以验证其他相关组件是否已准备好配合此更改:

packages/extension/src/browser/vscode/contributes/localization.ts (2)

12-12: 导入更改看起来没有问题!

@opensumi/ide-core-common 导入 Deferred 是合理的,这将支持新的异步控制流机制。


Line range hint 84-127: 请验证并发加载场景

当前实现在处理多个扩展的语言包时可能存在竞态条件。建议验证以下场景:

  1. 多个语言包扩展同时加载
  2. 用户在加载过程中切换语言
✅ Verification successful

代码中已有适当的并发和语言切换处理机制

代码分析显示已经实现了合理的处理机制:

  1. 使用 Promise.all 来处理多个语言包的并发加载
  2. 在加载语言包前会检查语言ID是否匹配当前语言,避免加载不必要的语言包:
if (languageId !== getLanguageId()) {
  return;
}
  1. 通过 registerLocalizationBundle 注册语言包时会检查语言ID,防止重复注册:
if (!languageId) {
  return;
}
const existingMessages = this.getContents(languageId);
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查是否存在可能的竞态条件处理代码

# 搜索与语言切换相关的处理代码
rg -l "getLanguageId|updateLanguagePack" --type ts

# 检查是否有防止并发加载的机制
ast-grep --pattern 'class $_ {
  $$$
  private $_: Deferred<$_>
  $$$
}'

Length of output: 121752


Script:

#!/bin/bash
# 检查语言包加载相关的并发处理机制
ast-grep --pattern 'class $_ {
  $$$
  async contribute() {
    $$$
    const promises: Promise<void>[] = [];
    $$$
    await Promise.all(promises);
    $$$
  }
  $$$
}'

# 检查语言切换相关的处理
rg -A 5 "getLanguageId|normalizeLanguageId" --type ts

# 检查是否有防止重复加载的机制
rg -A 5 "registerLocalizationBundle" --type ts

Length of output: 62893

scripts/download.js (2)

68-70: 函数签名修改和临时文件命名逻辑优化!

函数签名的更改通过显式传递版本参数提高了代码的可读性和可维护性。临时文件命名逻辑的简化也降低了代码复杂度。


161-161: 验证版本参数在整个下载流程中的一致性

请确保版本参数在整个扩展下载流程中得到正确处理,特别是在重试逻辑中。

运行以下脚本以验证版本参数的使用:

✅ Verification successful

版本参数在下载流程中已正确实现

代码分析显示版本参数在整个下载流程中得到了完整且一致的处理:

  • 版本参数正确传递给 downloadExtension 函数
  • 在构建下载URL时正确使用了版本参数
  • 临时文件命名中包含了版本信息,确保不同版本互不干扰
  • 重试逻辑中版本参数保持一致性
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查版本参数在代码库中的使用情况

# 搜索版本参数的使用情况
rg "version['\"]?\s*[,:]\s*version" --type js

# 检查下载URL中版本参数的使用
rg "version=.*?['\"]" --type js

Length of output: 85


Script:

#!/bin/bash
# 检查downloadExtension函数的实现和版本参数的使用
ast-grep --pattern 'async function downloadExtension($_) {
  $$$
}'

# 检查与version相关的函数调用和参数传递
rg -A 5 "downloadExtension\(" --type js

# 检查下载URL的构建逻辑
rg -A 5 "downloadUrl.*=" --type js

Length of output: 1818

packages/extension/src/node/extension.service.client.ts (1)

249-249: 验证调用方的兼容性

需要确保所有调用 getLanguagePack 的地方都正确处理了返回的 Promise。

✅ Verification successful

已验证调用方正确处理了 Promise 返回值

通过代码分析可以看到:

  • getLanguagePack 方法在接口定义和所有实现中都一致地返回 Promise<IExtensionLanguagePack | undefined>
  • 唯一的调用方 main.thread.localization.ts 中正确使用了 await 来处理返回的 Promise
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 搜索所有调用 getLanguagePack 的地方
rg "getLanguagePack\(" -A 3 -B 3

# 特别关注是否有正确使用 await 的地方
ast-grep --pattern 'await $_.getLanguagePack($$$)'

Length of output: 3679

packages/extension/src/common/extension.ts (2)

Line range hint 127-131: 接口设计更加统一!

注意到 IExtensionNodeClientService 接口中的其他方法如 updateLanguagePacksetupNLSConfig 等都是返回 Promise 的异步方法,这次将 getLanguagePack 也改为异步返回使得整个接口的设计更加统一和一致。


131-131: 将 getLanguagePack 方法改为异步是正确的方向!

getLanguagePack 方法改为返回 Promise 有助于解决语言包加载的问题,这样可以确保语言包数据在需要时已完全加载。

让我们验证一下这个接口变更在代码库中的影响:

✅ Verification successful

所有 getLanguagePack 的调用处都已正确更新为异步模式!

通过代码分析,我们可以确认:

  • main.thread.localization.ts 中已使用 await 正确处理异步调用
  • extension.service.client.ts 中已实现返回 Promise
  • 相关的接口定义和 mock 文件都已更新为 Promise 返回类型
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查 getLanguagePack 方法的所有调用处是否都已更新为异步调用

# 搜索所有调用 getLanguagePack 的地方
rg "getLanguagePack" -A 3

# 特别检查是否有使用 await 关键字的异步调用
ast-grep --pattern 'await $_.getLanguagePack($_)'

Length of output: 2884

packages/extension/src/browser/vscode/contributes/configuration.ts (3)

15-16: 模块引入看起来良好

成功引入了 LocalizationsContributionPoint 模块,路径正确,无明显问题。


40-42: 依赖注入配置正确

通过 @Autowired 注解成功注入了 LocalizationsContributionPoint,依赖注入配置正确。


43-44: ⚠️ Potential issue

请验证将 contribute 方法修改为异步的影响

contribute 方法更改为 async,需要确保所有调用此方法的地方都已正确处理异步操作,以防止未处理的 Promise 或潜在的运行时错误。

运行以下脚本,检查对 contribute 方法的调用:

✅ Verification successful

Let me analyze the call sites of contribute method to verify if they properly handle async operations. I'll search for the implementation of the specific contribute method in the configuration file:


将 contribute 方法修改为异步是安全的

从代码分析结果来看,这个修改是安全的,原因如下:

  1. 在扩展系统中,contributePoint.contribute() 的调用已经使用 await 进行了正确的异步处理(见 packages/extension/src/common/extension.ts)
  2. ConfigurationContributionPoint 依赖于 LocalizationsContributionPoint 的贡献完成,因此需要等待其完成是合理的
  3. 其他继承自 VSCodeContributePoint 的贡献点也都遵循类似的异步模式
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:查找所有调用 `contribute` 方法的地方,确保已处理异步。

# 测试:搜索 `contribute` 方法的调用,期望:调用处应适当处理异步。

rg '\.contribute\(' -A 3

Length of output: 3680


Script:

#!/bin/bash
# Search for the class containing the contribute method in configuration.ts
ast-grep --pattern $'class $_ {
  $$$
  contribute() {
    $$$
  }
  $$$
}'  packages/extension/src/browser/vscode/contributes/configuration.ts

Length of output: 143


Script:

#!/bin/bash
# Let's check the implementation and usage of this specific configuration class
rg -A 10 "class.*Configuration.*" packages/extension/src/browser/vscode/contributes/configuration.ts

# Also check where this class is instantiated and used
rg "new.*Configuration" packages/extension/src/browser/vscode/contributes/

Length of output: 641


Script:

#!/bin/bash
# Let's check the base class and its usage pattern
rg -A 5 "class VSCodeContributePoint" packages/extension/src/browser/vscode/contributes/
rg -A 5 "extends VSCodeContributePoint" packages/extension/src/browser/vscode/contributes/

# And check how the contribute method is called in the extension system
rg -A 5 "contributePoint.contribute" packages/extension/src/

Length of output: 20882


Script:

#!/bin/bash
# Let's check the base class implementation to understand the async pattern
rg -A 10 "abstract class VSCodeContributePoint" packages/extension/src/browser/vscode/

# And check the implementation of ConfigurationContributionPoint's contribute method
rg -A 10 "contribute\(\)" packages/extension/src/browser/vscode/contributes/configuration.ts

Length of output: 632

@hacke2
Copy link
Member

hacke2 commented Nov 25, 2024

@xkaede CI 看一下

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 🎨 feature feature required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants