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

chore(release): release v2.1.0 #196

Merged
merged 4 commits into from
Oct 10, 2024
Merged

chore(release): release v2.1.0 #196

merged 4 commits into from
Oct 10, 2024

Conversation

bytemain
Copy link
Member

@bytemain bytemain commented Oct 9, 2024

Summary by CodeRabbit

  • 新特性

    • 更新多个包的版本号至2.1.0。
    • 新增方法resolveStaticResourcePath以构建静态资源的URI。
  • Bug修复

    • 更新resolveFileType方法以更准确地识别文件类型。
  • 文档

    • 改进了变量命名以提高代码可读性。
  • 重构

    • 对多个方法进行了重命名和参数更新,以增强一致性和清晰度。

@bytemain
Copy link
Member Author

bytemain commented Oct 9, 2024

/create-merge-commit

Copy link

coderabbitai bot commented Oct 9, 2024

📝 Walkthrough

Walkthrough

该拉取请求主要涉及多个软件包的版本更新,将版本号从 "2.0.4" 更新到 "2.1.0"。此外,还对 createEditor.tsxindex.tsx 文件进行了修改,包括函数重命名和新方法的添加。其他文件中也进行了方法重构、类的添加以及接口的更新,涉及到静态资源路径的解析和文件类型的处理。

Changes

文件路径 更改摘要
lerna.json, packages/cli/package.json, packages/code-api/package.json, packages/code-service/package.json, packages/common/package.json, packages/core/package.json, packages/i18n/package.json, packages/plugin/package.json, packages/registry/package.json, packages/startup/package.json, packages/sumi-core/package.json, packages/toolkit/package.json 版本号更新:从 "2.0.4" 更新到 "2.1.0"。
packages/core/src/api/createEditor.tsx, packages/core/src/core/editor/index.tsx 函数 getDefaultAppConfig 重命名为 getDefaultEditorAppConfig;在 index.tsx 中添加新方法 resolveStaticResourcePath
packages/core/src/core/diff-viewer/internal/base.ts, packages/core/src/core/editor/editor.module.ts, packages/core/src/core/editor/modules.ts, packages/core/src/core/editor/types.ts 重构 normalizePath 方法;添加 EditorStaticResourceContribution 类;更新 ServerModuleCollection 的导入。
packages/sumi-core/src/common/types.ts, packages/sumi-core/src/server/core/app.ts, packages/sumi-core/src/server/file-service/encoding.ts, packages/sumi-core/src/server/file-service/file-service-watcher.ts, packages/sumi-core/src/server/index.ts, packages/sumi-core/src/server/search/base.ts 更新 resolveFileType 方法的返回类型;重命名变量以提高可读性;在 index.ts 中添加常量 editorDisabledModules

Possibly related PRs

  • chore: release v1.2.0 #112: 此拉取请求更新了 lerna.json 和多个 package.json 文件中的版本号,与主拉取请求中的版本更新相关。
  • fix: editor renderer not work #189: 此拉取请求包含对 createEditor.tsx 文件的更改,特别是将函数 getDefaultAppConfig 重命名为 getDefaultEditorAppConfig,与主拉取请求中的更改直接相关。

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.

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

@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: 6

🧹 Outside diff range and nitpick comments (9)
packages/sumi-core/src/server/index.ts (2)

39-39: 新增 editorDisabledModules 常量

引入 editorDisabledModules 常量是一个好主意,它允许灵活地禁用特定模块。使用 Set 数据结构确保了高效的查找操作。

建议添加注释说明此常量的用途,以及为什么要禁用 SearchModule。例如:

// 定义在编辑器中应该被禁用的模块
// SearchModule 被禁用是因为 [在此处填写原因]
const editorDisabledModules = new Set([SearchModule] as ModuleConstructor[]);

41-43: 新增 EditorServerModuleCollection 常量

创建 EditorServerModuleCollection 是一个很好的做法,它允许为编辑器服务器提供一个定制的模块集合。过滤逻辑简洁有效。

考虑使用 Array.prototype.filter 的简写形式来稍微优化代码:

export const EditorServerModuleCollection = ServerModuleCollection.filter(
  (m) => !editorDisabledModules.has(m)
);

这种写法更加简洁,并且保持了相同的功能。

packages/core/src/core/editor/types.ts (1)

53-54: 新方法添加正确,建议添加文档注释

新添加的 resolveStaticResourcePath 方法看起来是一个有用的扩展,可以为代码文档模型提供静态资源路径解析功能。这个可选方法的添加不会破坏现有的接口实现。

为了提高代码的可读性和可维护性,建议为新方法添加文档注释,解释其用途、参数和返回值。例如:

/**
 * 解析静态资源路径
 * @param documentModel 代码文档模型
 * @returns 解析后的 URI
 */
resolveStaticResourcePath?(documentModel: CodeDocumentModel): URI;

这将帮助其他开发者更好地理解和使用这个新方法。

packages/core/src/api/createEditor.tsx (1)

33-33: 函数重命名提高了代码清晰度

getDefaultAppConfig重命名为getDefaultEditorAppConfig是一个很好的改进,它更明确地表示了这个配置是针对编辑器应用的。这种改变提高了代码的可读性和可维护性。

考虑在函数上方添加一个简短的JSDoc注释,解释这个函数的用途和返回值类型。例如:

/**
 * 获取默认的编辑器应用配置
 * @returns {IAppOpts} 默认的编辑器应用配置对象
 */
const getDefaultEditorAppConfig = (): IAppOpts => ({
  // ... existing implementation ...
});

这将进一步提高代码的文档性和可读性。

packages/core/src/core/diff-viewer/internal/base.ts (3)

Line range hint 88-93: 路径规范化方法的改进

这个改动简化了路径规范化的过程,提高了代码的可读性。然而,我们可以进一步优化这个方法:

  1. 考虑使用 path.relative() 来获取相对路径,这可能会更加健壮。
  2. 添加一个参数来控制是否保留开头的斜杠,以增加灵活性。

建议如下:

normalizePath(path: string, keepLeadingSlash = false): string {
  const result = path.relative(this.appConfig.workspaceDir, path);
  return keepLeadingSlash ? result : result.replace(/^\//, '');
}

这样的实现可能会更加通用和可配置。


251-251: 获取所有标签方法的更新

这个改动与之前的修改保持一致,使用 codeUri.path 替代 codeUri.fsPath。这种一致性有助于维护代码的可读性和可预测性。

为了进一步优化这个方法,我们可以考虑以下建议:

  1. 使用 map 方法代替 forEach,这样可以直接返回结果,无需创建中间数组。
  2. 考虑缓存 normalizePath 的结果,特别是如果这个方法被频繁调用的话。

建议的优化如下:

getAllTabs = (): IDiffViewerTab[] => {
  const editorGroup = this.workbenchEditorService.editorGroups[0];
  return editorGroup.resources.map((editor, idx) => ({
    index: idx,
    filePath: this.normalizePath(editor.uri.codeUri.path),
  }));
};

这个优化可以略微提高性能,特别是在处理大量标签时。


Line range hint 359-372: 初始化方法中路径处理的更新

这些修改确保了整个初始化过程中路径处理的一致性,这是一个很好的改进。然而,我们可以通过以下方式进一步提高代码的可读性和维护性:

  1. 考虑将路径规范化的逻辑抽取到一个单独的辅助函数中,以减少重复代码。
  2. 添加注释解释为什么需要规范化路径,特别是对于 newPath 的处理。

建议的改进如下:

private normalizeAndGetIndex(path: string): { normalizedPath: string, index: number } {
  const normalizedPath = this.normalizePath(path);
  return {
    normalizedPath,
    index: this.getFileIndex(normalizedPath)
  };
}

// 在 initialize 方法中使用
if (newPath) {
  const { normalizedPath, index } = this.normalizeAndGetIndex(newPath);
  newPath = normalizedPath;
  currentIndex = index;
}

这样可以使代码更加模块化,并且更容易理解路径处理的意图。

packages/core/src/core/editor/contributions/static-resource.contribution.ts (2)

48-50: 提供协助解决 'FIXME' 注释中的问题

注释中提到由于 resolveStaticResource 是同步方法,这里硬编码了 workspaceManagerService 的实现。这可能导致代码的可维护性和可扩展性问题。建议考虑使用异步方法或重构相关逻辑。我可以协助提出解决方案,您是否希望我为此创建一个新的 GitHub issue?


51-54: 纠正关于修剪前导空白的误导性注释

代码实际是在修剪路径的前导分隔符,而不是修剪前导空白字符。建议将注释修改为更准确的描述,例如“修剪前导路径分隔符”。

建议的修改:

- // trim leading whitespace
+ // 修剪前导路径分隔符
📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 302756f and 21e7a88.

📒 Files selected for processing (26)
  • lerna.json (1 hunks)
  • packages/cli/package.json (1 hunks)
  • packages/code-api/package.json (1 hunks)
  • packages/code-service/package.json (1 hunks)
  • packages/common/package.json (1 hunks)
  • packages/core/package.json (1 hunks)
  • packages/core/src/api/createEditor.tsx (2 hunks)
  • packages/core/src/core/diff-viewer/internal/base.ts (5 hunks)
  • packages/core/src/core/editor/contributions/static-resource.contribution.ts (1 hunks)
  • packages/core/src/core/editor/editor.module.ts (2 hunks)
  • packages/core/src/core/editor/modules.ts (2 hunks)
  • packages/core/src/core/editor/types.ts (2 hunks)
  • packages/core/src/core/utils.ts (2 hunks)
  • packages/i18n/package.json (1 hunks)
  • packages/plugin/package.json (1 hunks)
  • packages/registry/package.json (1 hunks)
  • packages/startup/package.json (1 hunks)
  • packages/startup/src/editor/index.tsx (5 hunks)
  • packages/sumi-core/package.json (1 hunks)
  • packages/sumi-core/src/common/types.ts (2 hunks)
  • packages/sumi-core/src/server/core/app.ts (6 hunks)
  • packages/sumi-core/src/server/file-service/encoding.ts (0 hunks)
  • packages/sumi-core/src/server/file-service/file-service-watcher.ts (1 hunks)
  • packages/sumi-core/src/server/index.ts (2 hunks)
  • packages/sumi-core/src/server/search/base.ts (1 hunks)
  • packages/toolkit/package.json (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/sumi-core/src/server/file-service/encoding.ts
✅ Files skipped from review due to trivial changes (12)
  • lerna.json
  • packages/cli/package.json
  • packages/code-api/package.json
  • packages/code-service/package.json
  • packages/common/package.json
  • packages/core/package.json
  • packages/i18n/package.json
  • packages/plugin/package.json
  • packages/registry/package.json
  • packages/startup/package.json
  • packages/sumi-core/package.json
  • packages/toolkit/package.json
🧰 Additional context used
🪛 Biome
packages/sumi-core/src/common/types.ts

[error] 216-218: Don't use 'Boolean' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead

(lint/complexity/noBannedTypes)

🔇 Additional comments (22)
packages/sumi-core/src/server/search/base.ts (1)

10-10: 符号名称更改看起来不错,但请验证其影响。

符号名称从 'ContentSearchService' 更改为 'IContentSearchServer' 使其与接口名称更加一致,这是一个很好的改进。然而,这种更改可能会影响代码库中其他引用此符号的部分。

请运行以下脚本来验证更改的影响:

✅ Verification successful

符号名称更改已成功验证。

符号名称从 'ContentSearchService' 更改为 'IContentSearchServer' 已在整个代码库中正确应用,没有发现任何对旧符号名称的引用。

所有相关部分现在都使用新的符号名称 'IContentSearchServer',确保命名的一致性和代码的稳定性。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 IContentSearchServer 符号的所有使用是否已更新

# 测试:搜索旧的符号名称。预期:没有匹配项
echo "搜索旧的符号名称 'ContentSearchService':"
rg --type typescript "Symbol\('ContentSearchService'\)"

# 测试:搜索新的符号名称。预期:只有在 base.ts 中有一个匹配项
echo "搜索新的符号名称 'IContentSearchServer':"
rg --type typescript "Symbol\('IContentSearchServer'\)"

# 测试:搜索 IContentSearchServer 的所有引用。预期:所有引用都使用新的符号名称
echo "搜索 IContentSearchServer 的所有引用:"
rg --type typescript "IContentSearchServer"

Length of output: 509


Script:

#!/bin/bash
# 描述:验证 IContentSearchServer 符号的所有使用是否已更新

# 测试:搜索旧的符号名称。预期:没有匹配项
echo "搜索旧的符号名称 'ContentSearchService':"
rg --glob '*.ts' --glob '*.tsx' "Symbol\('ContentSearchService'\)"

# 测试:搜索新的符号名称。预期:只有在 base.ts 中有一个匹配项
echo "搜索新的符号名称 'IContentSearchServer':"
rg --glob '*.ts' --glob '*.tsx' "Symbol\('IContentSearchServer'\)"

# 测试:搜索 IContentSearchServer 的所有引用。预期:所有引用都使用新的符号名称
echo "搜索 IContentSearchServer 的所有引用:"
rg --glob '*.ts' --glob '*.tsx' "IContentSearchServer"

Length of output: 1424

packages/sumi-core/src/server/index.ts (1)

37-37: 类型注解改进了代码质量

为 ServerModuleCollection 添加 as ModuleConstructor[] 类型注解是一个很好的改进。这增强了类型安全性,提高了代码的可读性,并与从 '@opensumi/ide-core-browser' 导入的 ModuleConstructor 类型保持一致。

packages/core/src/core/editor/modules.ts (2)

69-69: 模块集合更新与导入更改一致。

getModules 函数中使用 EditorServerModuleCollection 替代 ServerModuleCollection 是正确的,与导入语句的更改保持一致。

为确保功能完整性,请验证是否所有必要的模块都包含在 EditorServerModuleCollection 中。可以运行以下脚本来比较新旧集合的内容:

#!/bin/bash
# 描述:比较 ServerModuleCollection 和 EditorServerModuleCollection 的内容

# 搜索 ServerModuleCollection 的定义
echo "ServerModuleCollection 定义:"
rg --type typescript -A 10 'export const ServerModuleCollection'

# 搜索 EditorServerModuleCollection 的定义
echo "EditorServerModuleCollection 定义:"
rg --type typescript -A 10 'export const EditorServerModuleCollection'

如果发现任何差异,请考虑是否需要显式包含某些模块。


29-29: 导入语句更改看起来合理。

ServerModuleCollection 更改为 EditorServerModuleCollection 似乎是为了使用更具体的模块集合。这个改动看起来是正确的。

请运行以下脚本以验证整个代码库中的一致性:

packages/core/src/api/createEditor.tsx (2)

65-65: 函数调用更新与重命名一致

createEditor函数中对getDefaultAppConfig的调用已正确更新为getDefaultEditorAppConfig。这个更改与之前的函数重命名保持一致,确保了代码的连贯性。


Line range hint 1-110: 总体评审意见

本次更改主要涉及函数重命名,从getDefaultAppConfiggetDefaultEditorAppConfig。这个改动提高了代码的清晰度和特定性,更好地反映了该函数与编辑器应用的关系。相应的函数调用也已更新,保持了代码的一致性。

这些更改是非功能性的,不会影响现有的行为或引入新功能。它们主要改善了代码的可读性和可维护性,这对于长期项目维护是有益的。

建议在未来的更改中,考虑为关键函数添加简洁的JSDoc注释,以进一步提高代码的文档性。

packages/sumi-core/src/common/types.ts (2)

11-11: 导入语句添加正确

新增的 EditorFileType 导入语句位置恰当,与其他导入保持一致。这个导入是为了支持 resolveFileType 方法签名的更新。


216-216: 方法签名更新和类型使用建议

  1. resolveFileType 方法签名的更新增加了 EditorFileType 返回类型,这提高了文件类型解析的灵活性,是个很好的改进。

  2. 建议验证这个变更对现有代码的影响,确保所有使用 resolveFileType 的地方都能正确处理新增的 EditorFileType 类型。

  3. 根据静态分析工具的提示,建议将第 216 行的 Boolean 类型改为小写的 boolean,以保持一致性和遵循 TypeScript 的最佳实践。

您是否需要我生成一个脚本来检查 resolveFileType 的使用情况?

🧰 Tools
🪛 Biome

[error] 216-218: Don't use 'Boolean' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead

(lint/complexity/noBannedTypes)

packages/sumi-core/src/server/core/app.ts (5)

114-114: 变量重命名提高了代码可读性

disposeCollection 重命名为 _disposables 是一个很好的改进。这个更改:

  1. 使用下划线前缀明确表示这是一个私有成员,符合常见的命名约定。
  2. 新名称更简洁,同时仍然清晰地表达了其用途。

这有助于提高代码的可读性和一致性。


Line range hint 152-156: 改进了资源清理逻辑

将事件监听器的移除逻辑添加到 _disposables 集合中是一个很好的改进。这个更改:

  1. 确保了在 ServerApp 被销毁时,事件监听器会被正确清理。
  2. 集中了清理逻辑,使得资源管理更加统一和可靠。

这种做法有助于防止内存泄漏,并使代码更易于维护。


194-196: 优化了文件系统资源管理

initializeHomeFileSystem 的结果添加到 _disposables 集合中是一个很好的改进。这个更改:

  1. 确保了在初始化主目录文件系统时分配的资源能够被正确清理。
  2. 与之前的更改保持一致,进一步集中了资源清理逻辑。

这种做法有助于防止资源泄漏,并使整个应用的生命周期管理更加健壮。


226-227: 改进了通道处理器的命名和资源管理

这些更改进一步优化了代码:

  1. commonChannelPathHandler 重命名为 pathHandler,提高了简洁性和可读性。
  2. 清理逻辑现在正确地集成到 _disposables 集合中,确保了资源的proper清理。
  3. 这些更改与类中早先的修改保持一致,提高了整体代码的一致性。

这些改进使得代码更易于理解和维护,同时确保了资源的正确管理。

Also applies to: 239-239, 241-243, 255-255


Line range hint 114-255: 总体代码质量显著提升

这次更新对 ServerApp 类进行了一系列改进:

  1. 统一了命名约定,如将 disposeCollection 改为 _disposables,使私有成员更易识别。
  2. 优化了资源管理,将清理逻辑集中到 _disposables 集合中,包括事件监听器和文件系统资源。
  3. 提高了代码的一致性和可读性,如将 commonChannelPathHandler 简化为 pathHandler

这些更改虽然没有改变功能,但显著提高了代码的可维护性、可读性和健壮性。它们符合 TypeScript/JavaScript 开发的最佳实践,有助于防止资源泄漏并简化了未来的维护工作。

总的来说,这是一次高质量的重构,值得赞赏。

packages/core/src/core/diff-viewer/internal/base.ts (2)

Line range hint 1-487: DiffViewerContribution 类的整体改进

本次修改主要集中在路径处理和规范化方面,这些变更在整个文件中保持了一致性。主要的改进包括:

  1. 简化了 normalizePath 方法,使其更加通用。
  2. 统一使用 codeUri.path 替代 codeUri.fsPath
  3. 在初始化过程中保持了一致的路径处理方式。

这些改动提高了代码的可读性和一致性。然而,我们需要注意以下几点:

  1. 跨平台兼容性:由于移除了特定操作系统的处理,需要进行全面的跨平台测试。
  2. 性能优化:考虑在频繁调用的方法中实施建议的性能优化。
  3. 代码模块化:可以进一步抽取共用的路径处理逻辑,提高代码的可维护性。

总的来说,这些修改是积极的,但建议进行全面的测试,特别是在不同操作系统上,以确保功能的正确性和一致性。


242-242: 编辑器文件路径获取方法的更新

这个改动将 fsPath 替换为 path,与新的路径处理方式保持一致。这是一个好的改进,但我们需要注意以下几点:

  1. 确保在所有支持的操作系统上都能正确工作,特别是在 Windows 系统上。
  2. 考虑添加单元测试,以验证在不同操作系统上的行为是否一致。

建议添加一个注释,解释为什么使用 path 而不是 fsPath,以便future维护者理解这个决定。

为了验证这个改动在不同操作系统上的行为,我们可以运行以下脚本:

这个脚本将帮助我们识别可能需要额外注意的地方,以确保跨平台兼容性。

packages/core/src/core/editor/contributions/static-resource.contribution.ts (2)

1-11: 导入语句正确

导入的模块和类型均符合需要。


26-65: registerStaticResolver 方法的实现正确

代码逻辑清晰,满足功能需求。

packages/core/src/core/utils.ts (1)

71-80: 接口定义清晰

SCMUriQueryParamsSCMUriParams 接口的定义清晰,结构合理,符合业务需求。

packages/startup/src/editor/index.tsx (2)

15-18: 模块引入正确,符合需求

新引入的模块 URICodeDocumentModelgetFileTypeByExt 在代码中正确使用,满足功能需求。


36-36: 确认在 main 数组中添加图片路径是否合适

main 数组中添加了图片路径 'packages/design/src/browser/style/images/design-background-light.png',请确认这是否符合预期。如果 main 用于列举代码文件,包含图片路径可能不合适。

packages/core/src/core/editor/editor.module.ts (2)

63-63: 引入新类EditorStaticResourceContribution

成功导入了EditorStaticResourceContribution,这将有助于扩展编辑器的静态资源功能。


744-744: EditorStaticResourceContribution添加到providers数组

已将EditorStaticResourceContribution添加到EditorSpecialModuleproviders列表中,确保该贡献者正确注册。

Comment on lines +143 to +148
while (!(await fse.pathExists(uri.codeUri.path)) && times <= count) {
uri = uri.parent;
times++;
}
if (await fse.pathExists(uri.codeUri.fsPath)) {
return uri.codeUri.fsPath;
if (await fse.pathExists(uri.codeUri.path)) {
return uri.codeUri.path;
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

发现路径处理方式使用不一致,需统一规范

在代码库中发现多个文件同时使用了fsPathpath,包括file-service-watcher.ts文件。这表明路径处理方式存在不一致,可能导致潜在的问题或意外行为。

  • 请检查以下文件中的路径处理方式,确保使用一致的属性:

    • packages/sumi-core/src/server/file-service/file-service-watcher.ts
    • packages/sumi-core/src/server/file-scheme/file-scheme-doc.service.ts
    • packages/code-service/src/static-resource.contribution.ts
    • 以及其他列出的文件。
  • 如果决定统一使用path,请确保所有相关部分都进行相应的修改,并在不同操作系统上进行充分测试。

🔗 Analysis chain

请确认路径处理方式的变更及其影响

lookup函数中,您将uri.codeUri.fsPath更改为uri.codeUri.path。这个变更可能会影响路径的处理方式,特别是在不同的操作系统上。

  1. 能否解释一下进行这项更改的原因?
  2. 这个变更是否已经在不同的操作系统(如Windows、macOS和Linux)上进行了充分的测试?
  3. 是否考虑过这个改动可能对其他依赖此函数的代码产生的影响?

为了验证这个更改的影响,我建议运行以下脚本来检查整个代码库中fsPathpath的使用情况:

这将帮助我们了解这个更改是否与整个代码库的使用模式一致,以及是否需要在其他地方进行类似的更改。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
echo "Checking usage of 'fsPath':"
rg --type typescript 'fsPath'

echo "\nChecking usage of 'path' in URI context:"
rg --type typescript 'uri.*\.path'

Length of output: 308


Script:

#!/bin/bash
echo "Checking usage of 'fsPath' in TypeScript files:"
rg 'fsPath' --glob '*.ts' --glob '*.tsx'

echo "\nChecking usage of 'path' in URI context in TypeScript files:"
rg 'uri.*\.path' --glob '*.ts' --glob '*.tsx'

Length of output: 7218

Comment on lines +82 to +97
export function fromSCMUri(uri: URI): SCMUriParams {
const query = uri.getParsedQuery();

const result: SCMUriParams = {
platform: query.scheme,
repo: query.authority,
path: query.path,
ref: query.ref,
};

if (query.branch) {
result.branch = query.branch;
}

return result;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

修正对 URI 属性的访问方式

fromSCMUri 函数中,当前从 query 对象中获取 schemeauthoritypath 等属性可能不正确。这些属性实际上应该直接从 uri 对象中获取,而不是从 query。例如,应该使用 uri.scheme 而非 query.scheme

可以应用以下修改来修正属性的获取:

export function fromSCMUri(uri: URI): SCMUriParams {
  const query = uri.getParsedQuery();

  const result: SCMUriParams = {
-   platform: query.scheme,
-   repo: query.authority,
-   path: query.path,
+   platform: uri.scheme,
+   repo: uri.authority,
+   path: uri.path,
    ref: query.ref,
  };

  if (query.branch) {
    result.branch = query.branch;
  }

  return result;
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function fromSCMUri(uri: URI): SCMUriParams {
const query = uri.getParsedQuery();
const result: SCMUriParams = {
platform: query.scheme,
repo: query.authority,
path: query.path,
ref: query.ref,
};
if (query.branch) {
result.branch = query.branch;
}
return result;
}
export function fromSCMUri(uri: URI): SCMUriParams {
const query = uri.getParsedQuery();
const result: SCMUriParams = {
platform: uri.scheme,
repo: uri.authority,
path: uri.path,
ref: query.ref,
};
if (query.branch) {
result.branch = query.branch;
}
return result;
}

Comment on lines +99 to +114
export function toSCMUri(uriParams: SCMUriParams) {
const query: SCMUriQueryParams = {
ref: uriParams.ref,
};

if (uriParams.branch) {
query.branch = uriParams.branch;
}

return URI.from({
scheme: uriParams.platform,
authority: uriParams.repo,
path: uriParams.path,
query: JSON.stringify(query),
});
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

避免将查询参数序列化为 JSON 字符串

toSCMUri 函数中,将查询参数使用 JSON.stringify 序列化为 JSON 字符串存储在 URI 的查询部分可能不是最佳实践。这可能会导致 URI 在其他环境中解析出现问题。建议使用标准的查询字符串格式编码查询参数。

可以应用以下修改,使查询参数以标准格式编码:

export function toSCMUri(uriParams: SCMUriParams) {
  const query: SCMUriQueryParams = {
    ref: uriParams.ref,
  };

  if (uriParams.branch) {
    query.branch = uriParams.branch;
  }

  return URI.from({
    scheme: uriParams.platform,
    authority: uriParams.repo,
    path: uriParams.path,
-   query: JSON.stringify(query),
+   query: new URLSearchParams(query as Record<string, string>).toString(),
  });
}

这样,生成的 URI 查询部分将是标准的 key=value&key2=value2 格式,更易于解析和兼容。

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function toSCMUri(uriParams: SCMUriParams) {
const query: SCMUriQueryParams = {
ref: uriParams.ref,
};
if (uriParams.branch) {
query.branch = uriParams.branch;
}
return URI.from({
scheme: uriParams.platform,
authority: uriParams.repo,
path: uriParams.path,
query: JSON.stringify(query),
});
}
export function toSCMUri(uriParams: SCMUriParams) {
const query: SCMUriQueryParams = {
ref: uriParams.ref,
};
if (uriParams.branch) {
query.branch = uriParams.branch;
}
return URI.from({
scheme: uriParams.platform,
authority: uriParams.repo,
path: uriParams.path,
query: new URLSearchParams(query as Record<string, string>).toString(),
});
}

Comment on lines +53 to +54
const staticServicePath = `https://raw.githubusercontent.com`;

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议将 staticServicePath 配置化

目前将 staticServicePath 硬编码为 https://raw.githubusercontent.com,建议将其提取为可配置项,以提高代码的灵活性和可维护性。

Comment on lines +179 to +183
const ext = path.split('.').pop();
if (!ext) {
return 'text';
}
return getFileTypeByExt(ext);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

改进扩展名提取的方法

使用 path.split('.').pop() 提取文件扩展名在一些情况下可能会不准确,例如文件名包含多个点(如 archive.tar.gz),会导致扩展名解析错误。建议使用更可靠的方法来获取扩展名,以确保文件类型解析的准确性。

可以考虑以下改进:

resolveFileType(path) {
-  const ext = path.split('.').pop();
+  const ext = path.substring(path.lastIndexOf('.') + 1);
   if (!ext) {
     return 'text';
   }
   return getFileTypeByExt(ext);
}

或者,使用正则表达式来获取扩展名:

const ext = path.match(/\.([^.]+)$/)?.[1] || '';

Comment on lines +207 to +211
resolveStaticResourcePath(documentModel: CodeDocumentModel) {
return URI.parse(
`${staticServicePath}/${documentModel.owner}/${documentModel.name}/${documentModel.ref}/${documentModel.filepath}`,
);
},
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

确保构建的 URL 正确地进行编码

resolveStaticResourcePath 方法中,直接拼接 URL 可能导致在 ownernamereffilepath 包含特殊字符时出现问题。建议对这些参数进行 URL 编码,以确保生成的 URI 有效且安全。

可以考虑如下修改:

resolveStaticResourcePath(documentModel: CodeDocumentModel) {
  return URI.parse(
-    `${staticServicePath}/${documentModel.owner}/${documentModel.name}/${documentModel.ref}/${documentModel.filepath}`,
+    `${staticServicePath}/${encodeURIComponent(documentModel.owner)}/${encodeURIComponent(documentModel.name)}/${encodeURIComponent(documentModel.ref)}/${encodeURIComponent(documentModel.filepath)}`,
  );
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
resolveStaticResourcePath(documentModel: CodeDocumentModel) {
return URI.parse(
`${staticServicePath}/${documentModel.owner}/${documentModel.name}/${documentModel.ref}/${documentModel.filepath}`,
);
},
resolveStaticResourcePath(documentModel: CodeDocumentModel) {
return URI.parse(
`${staticServicePath}/${encodeURIComponent(documentModel.owner)}/${encodeURIComponent(documentModel.name)}/${encodeURIComponent(documentModel.ref)}/${encodeURIComponent(documentModel.filepath)}`,
);
},

@bytemain
Copy link
Member Author

bytemain commented Oct 9, 2024

/merge-this

1 similar comment
@bytemain
Copy link
Member Author

/merge-this

@opensumi
Copy link
Contributor

opensumi bot commented Oct 10, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant