Skip to content

refactor: support watch by unrecursive config#4201

Closed
Aaaaash wants to merge 3 commits intomainfrom
refactor/support-watch-by-unrecursive-config
Closed

refactor: support watch by unrecursive config#4201
Aaaaash wants to merge 3 commits intomainfrom
refactor/support-watch-by-unrecursive-config

Conversation

@Aaaaash
Copy link
Copy Markdown
Member

@Aaaaash Aaaaash commented Dec 9, 2024

Types

  • 🎉 New Features
  • 🐛 Bug Fixes
  • 📚 Documentation Changes
  • 💄 Code Style Changes
  • 💄 Style Changes
  • 🪚 Refactors
  • 🚀 Performance Improvements
  • 🏗️ Build System
  • ⏱ Tests
  • 🧹 Chores
  • Other Changes

Background or solution

Changelog

Summary by CodeRabbit

  • 新功能

    • 添加了可选属性 unRecursiveDirectories,用于配置不递归处理的目录路径。
    • 更新了 watchFileChanges 方法,允许指定是否递归监视文件变化。
  • 改进

    • 文件监视逻辑进行了重构,明确区分递归和非递归监视功能。
    • 增强了 FileServiceDiskFileSystemProvider 类的可配置性。
  • 文档

    • 更新了相关方法的注释,以提高可读性和理解性。

@opensumi opensumi Bot added the ⚙️ refactor Refactor code label Dec 9, 2024
@Aaaaash
Copy link
Copy Markdown
Member Author

Aaaaash commented Dec 9, 2024

/next

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 9, 2024

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

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 eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options.
warning eslint > @humanwhocodes/config-array@0.13.0: Use @eslint/config-array instead
warning eslint > @humanwhocodes/config-array > @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
warning eslint > file-entry-cache > flat-cache > rimraf@3.0.2: Rimraf versions prior to v4 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > glob@7.2.3: Glob versions prior to v9 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > glob > inflight@1.0.6: 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 > glob@7.2.3: Glob versions prior to v9 are no longer supported
error Couldn't find any versions for "@opensumi/ide-dev-tool" that matches "workspace:*"

Walkthrough

该拉取请求引入了一些对文件监控功能的修改,包括在AppConfig接口中添加一个新的可选属性unRecursiveDirectories,以及更新FileSystemProvider接口中的watch方法签名以支持递归选项。此外,FileServiceClientDiskFileSystemProvider类中的文件监控逻辑也进行了相应的调整,以便更好地处理递归和非递归目录的监控需求。

Changes

文件路径 更改摘要
packages/core-browser/src/react-providers/config-provider.tsx 添加了可选属性unRecursiveDirectories?: string[]AppConfig接口。
packages/core-common/src/types/file.ts 更新FileSystemProvider接口中的watch方法签名,添加了可选属性recursive?: boolean
packages/file-service/src/browser/file-service-client.ts 更新watchFileChanges方法,添加逻辑以根据unRecursiveDirectories配置决定是否应用递归监控。
packages/file-service/src/node/disk-file-system.provider.ts 重构DiskFileSystemProvider类,替换watcherServer属性为recursiveFileSystemWatcherunrecursiveFileSystemWatcher,并更新相关方法。
packages/file-service/src/node/file-service.ts 更新watchFileChanges方法签名,添加可选的recursive布尔参数。
packages/file-service/src/node/recursive/file-service-watcher.ts 更新setClient方法中的条件逻辑,修改了对client的检查。
packages/file-service/src/node/un-recursive/file-service-watcher.ts 更新setClient方法中的条件逻辑,确保只有在已有客户端的情况下才会赋值。

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant FileService
    participant FileSystemProvider
    participant DiskFileSystemProvider

    User->>FileService: watchFileChanges(uri, options)
    FileService->>FileSystemProvider: watch(uri, options)
    FileSystemProvider->>DiskFileSystemProvider: watch(uri, options)
    DiskFileSystemProvider->>DiskFileSystemProvider: getWatcherServer(recursive)
    DiskFileSystemProvider->>DiskFileSystemProvider: initWatcherServer(excludes)
    DiskFileSystemProvider-->>FileSystemProvider: return watcher
    FileSystemProvider-->>FileService: return watch result
    FileService-->>User: return watch result
Loading

Possibly related PRs

  • feat: use async in watch file changes #4026: 此PR修改了FileSystemWatcherServer类,与主PR中的unRecursiveDirectories属性引入的文件监控功能相关。两个PR中的更改都涉及如何配置和管理文件监控,特别是关于递归和非递归选项。

Suggested labels

🎨 feature

Suggested reviewers

  • Ricbet

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4dc6a9f and 0ebfd9c.

📒 Files selected for processing (2)
  • packages/file-service/src/node/recursive/file-service-watcher.ts (1 hunks)
  • packages/file-service/src/node/un-recursive/file-service-watcher.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/file-service/src/node/un-recursive/file-service-watcher.ts
  • packages/file-service/src/node/recursive/file-service-watcher.ts

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

🧹 Outside diff range and nitpick comments (3)
packages/core-browser/src/react-providers/config-provider.tsx (1)

315-319: 属性命名建议修改为 nonRecursiveDirectories

为更符合命名惯例,建议将属性 unRecursiveDirectories 重命名为 nonRecursiveDirectories,以提升代码可读性和一致性。

packages/file-service/src/node/file-service.ts (1)

98-107: 建议使用可选链操作符优化代码

当前代码在处理 options 时可以使用可选链操作符来简化逻辑。

建议修改为:

-      recursive: options && options.recursive !== undefined ? options.recursive : this.options.recursive,
+      recursive: options?.recursive ?? this.options.recursive,
🧰 Tools
🪛 Biome (1.9.4)

[error] 105-105: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/file-service/src/browser/file-service-client.ts (1)

Line range hint 342-362: 建议添加功能说明文档

新增的非递归目录配置功能缺少相关文档说明。

建议在代码中添加以下注释:

+    // 通过 unRecursiveDirectories 配置控制特定目录的文件监听是否递归
+    // 当路径匹配 unRecursiveDirectories 中的目录时,将禁用递归监听
     const unRecursiveDirectories = this.appConfig.unRecursiveDirectories || [];
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4c710fe and 4dc6a9f.

📒 Files selected for processing (7)
  • packages/core-browser/src/react-providers/config-provider.tsx (1 hunks)
  • packages/core-common/src/types/file.ts (1 hunks)
  • packages/file-service/src/browser/file-service-client.ts (2 hunks)
  • packages/file-service/src/node/disk-file-system.provider.ts (6 hunks)
  • packages/file-service/src/node/file-service.ts (1 hunks)
  • packages/file-service/src/node/recursive/file-service-watcher.ts (1 hunks)
  • packages/file-service/src/node/un-recursive/file-service-watcher.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/file-service/src/node/file-service.ts

[error] 105-105: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (3)
packages/file-service/src/node/disk-file-system.provider.ts (2)

71-75: 属性定义正确

新增的 recursiveFileSystemWatcherunrecursiveFileSystemWatcher 属性定义清晰,类型明确,有助于区分递归和非递归的文件监视器。


105-105: 确保初始化 WatcherServer

在构造函数中调用 this.initWatcherServer();,确保了文件监视器服务器的正确初始化。

packages/core-common/src/types/file.ts (1)

160-160: 更新 watch 方法接口,增加递归选项

FileSystemProvider 接口的 watch 方法中新增了 recursive 可选参数,增强了对文件或目录监视的灵活性,允许呼叫者指定是否递归监视子目录。

Comment on lines +147 to +149
if (!watcherServer) {
return -1;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

避免在 Promise 中返回错误的数值

当没有可用的 watcherServer 时,watch 方法返回 -1。建议抛出异常或拒绝 Promise,而不是返回特定的错误值,以符合 Promise 的错误处理惯例。

建议修改如下:

if (!watcherServer) {
-   return -1;
+   throw new Error('No suitable watcher server found');
}
📝 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
if (!watcherServer) {
return -1;
}
if (!watcherServer) {
throw new Error('No suitable watcher server found');
}

Comment thread packages/file-service/src/node/un-recursive/file-service-watcher.ts Outdated
Comment thread packages/file-service/src/node/recursive/file-service-watcher.ts Outdated
Comment thread packages/file-service/src/browser/file-service-client.ts
@opensumi
Copy link
Copy Markdown
Contributor

opensumi Bot commented Dec 9, 2024

🎉 PR Next publish successful!

3.6.1-next-1733711618.0

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 54.03%. Comparing base (4c710fe) to head (0ebfd9c).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...file-service/src/node/disk-file-system.provider.ts 92.30% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4201      +/-   ##
==========================================
- Coverage   54.04%   54.03%   -0.02%     
==========================================
  Files        1616     1616              
  Lines       98006    98005       -1     
  Branches    20056    20062       +6     
==========================================
- Hits        52966    52954      -12     
- Misses      37420    37428       +8     
- Partials     7620     7623       +3     
Flag Coverage Δ
jsdom 49.62% <87.50%> (-0.01%) ⬇️
node 15.55% <87.50%> (+<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.

@Aaaaash
Copy link
Copy Markdown
Member Author

Aaaaash commented Dec 11, 2024

#4202

@Aaaaash Aaaaash closed this Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚙️ refactor Refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant