Skip to content

feat: support VSCode API: WindowState.active#4233

Merged
bk1012 merged 9 commits into
opensumi:mainfrom
quxingkai:WindowState
Dec 18, 2024
Merged

feat: support VSCode API: WindowState.active#4233
bk1012 merged 9 commits into
opensumi:mainfrom
quxingkai:WindowState

Conversation

@quxingkai
Copy link
Copy Markdown
Contributor

@quxingkai quxingkai commented Dec 16, 2024

Types

  • 🎉 New Features

Background or solution

Changelog

Summary by CodeRabbit

  • 新功能

    • 引入了 WindowActivityTimer 类,用于监控用户在窗口中的活动并管理非活动计时器。
    • WindowState 接口中添加了 active 属性,以指示窗口当前是否处于活动状态。
    • 更新了窗口状态管理方法,新增 $onDidChangeWindowFocus$onDidChangeWindowActive 方法,以分别处理焦点和活动状态的变化。
  • Bug 修复

    • 改进了资源管理和窗口活动跟踪能力,确保在实例处置时正确清理资源。

@opensumi opensumi Bot added the 🎨 feature feature required label Dec 16, 2024
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 16, 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:*"

概述

演练

此次更改主要增强了窗口状态管理功能,引入了更精细的窗口活动和焦点跟踪机制。新增了 WindowActivityTimer 类来监控用户活动,并在 WindowState 接口中添加了 active 属性。修改了窗口状态相关的方法,从单一的状态设置转变为分别处理窗口焦点和活动状态的方法。

变更

文件路径 变更摘要
packages/extension/src/browser/vscode/api/main.thread.window-state.ts 引入 DisposableCollectionWindowActivityTimer,更新窗口状态处理方法,新增 onActiveStateChanged 方法,修改 dispose 方法以清理资源。
packages/extension/src/common/vscode/ext-types.ts WindowState 接口中添加 active 属性。
packages/extension/src/common/vscode/window.ts 移除 $setWindowState 方法,新增 $onDidChangeWindowFocus$onDidChangeWindowActive 方法。
packages/extension/src/hosted/api/vscode/ext.host.window-state.ts 重命名 $setWindowState 方法为 $onDidChangeWindowFocus,新增 $onDidChangeWindowActive 方法,添加 active 属性。
packages/types/vscode/typings/vscode.d.ts WindowState 接口中添加 readonly active: boolean; 属性。
packages/extension/src/browser/vscode/api/window-activity/window-activity-timer.ts 新增 WindowActivityTimer 类,用于监控用户活动并管理不活动计时器。

序列图

sequenceDiagram
    participant User
    participant WindowActivityTimer
    participant ExtHostWindowState
    
    User->>WindowActivityTimer: 用户交互
    WindowActivityTimer->>WindowActivityTimer: 重置不活动计时器
    WindowActivityTimer->>ExtHostWindowState: 触发活动状态变更
    ExtHostWindowState->>ExtHostWindowState: 更新窗口活动状态
    ExtHostWindowState->>ExtHostWindowState: 发出活动状态事件
Loading

可能相关的 PR

建议的审阅者

  • Ricbet
  • bk1012

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e48836a and eb11c4f.

📒 Files selected for processing (1)
  • packages/extension/src/browser/vscode/api/window-activity/window-activity-timer.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/extension/src/browser/vscode/api/window-activity/window-activity-timer.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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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: 3

🧹 Nitpick comments (3)
packages/extension/src/browser/vscode/api/window-activity/window-activity-tracker.ts (2)

77-81: 优化 inactivityCounter 的重置逻辑

当检测到用户不活跃时,将 inactivityCounter 重置为 0,可以确保在用户再次交互后正确重新计时。

建议在 checkInactivity 方法中修改如下:

  private checkInactivity = (): void => {
    this.inactivityCounter++;
    if (this.inactivityCounter >= this.inactivityLimit) {
      this.activeState = false;
      this.stopTracking();
+     this.inactivityCounter = 0;
    }
  };

52-55: 考虑监听更多的用户交互事件

当前只监听了 mousedownkeydowntouchstart 事件,建议考虑添加 mousemovewheel 事件,以更全面地检测用户的活动状态。

建议修改如下:

  win.addEventListener('mousedown', this.resetInactivity, eventListenerOptions);
  win.addEventListener('keydown', this.resetInactivity, eventListenerOptions);
  win.addEventListener('touchstart', this.resetInactivity, eventListenerOptions);
+ win.addEventListener('mousemove', this.resetInactivity, eventListenerOptions);
+ win.addEventListener('wheel', this.resetInactivity, eventListenerOptions);
packages/extension/src/browser/vscode/api/main.thread.window-state.ts (1)

35-36: 方法命名建议

onActiveStateChanged 方法目前被声明为私有方法,但从命名上看,更像是一个事件处理器。考虑将其命名为 handleActiveStateChanged 或者 handleWindowActiveStateChanged,以提高代码可读性。

建议修改如下:

- private onActiveStateChanged(isActive: boolean): void {
+ private handleActiveStateChanged(isActive: boolean): void {
    this.proxy.$onDidChangeWindowActive(isActive);
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f10b81d and af57a6c.

📒 Files selected for processing (6)
  • packages/extension/src/browser/vscode/api/main.thread.window-state.ts (1 hunks)
  • packages/extension/src/browser/vscode/api/window-activity/window-activity-tracker.ts (1 hunks)
  • packages/extension/src/common/vscode/ext-types.ts (1 hunks)
  • packages/extension/src/common/vscode/window.ts (1 hunks)
  • packages/extension/src/hosted/api/vscode/ext.host.window-state.ts (1 hunks)
  • packages/types/vscode/typings/vscode.d.ts (1 hunks)
🔇 Additional comments (5)
packages/extension/src/hosted/api/vscode/ext.host.window-state.ts (2)

22-27: 确保 $onDidChangeWindowActive 方法的一致性

新添加的 $onDidChangeWindowActive 方法与 $onDidChangeWindowFocus 方法逻辑相似,但请确保在事件触发和状态更新时的一致性,以避免潜在的状态同步问题。

请检查是否在所有相关地方正确调用了 $onDidChangeWindowActive 方法,以保证窗口活动状态的准确性。


32-37: 初始化 WindowStateImpl 时确保状态正确

在插件进程重启时,将 focusedactive 默认设置为 true,请确认这与实际窗口状态一致,以避免插件在启动时状态不匹配的问题。

建议在插件启动时获取实际的窗口状态,或者确保默认值与实际状态一致。

packages/extension/src/browser/vscode/api/main.thread.window-state.ts (1)

30-33: 确保 WindowActivityTracker 正常工作

在构造函数中初始化了 WindowActivityTracker,请确保它正确地监听了窗口的活动状态变化,并且事件处理函数 onActiveStateChanged 能够正确传递状态。

可以通过实际测试窗口的激活和未激活状态,验证事件是否正确触发和处理。

packages/extension/src/common/vscode/ext-types.ts (1)

1641-1641: 代码实现正确!

active 属性的类型定义和实现都很规范。这个布尔类型属性可以准确反映窗口的活动状态。

packages/types/vscode/typings/vscode.d.ts (1)

1057-1062: 文档注释非常完整!

active 属性添加的文档注释解释得很清楚:

  • 清晰说明了属性的用途
  • 详细描述了状态变化的时机
  • 注释格式规范,便于生成API文档

Comment thread packages/extension/src/common/vscode/window.ts
@bk1012 bk1012 changed the title feat: support VSCode API: WindowState.active WIP: feat: support VSCode API: WindowState.active Dec 16, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 10.60606% with 59 lines in your changes missing coverage. Please review.

Project coverage is 53.92%. Comparing base (a1919f1) to head (eb11c4f).
Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
...scode/api/window-activity/window-activity-timer.ts 8.16% 40 Missing and 5 partials ⚠️
...src/browser/vscode/api/main.thread.window-state.ts 18.18% 9 Missing ⚠️
...ion/src/hosted/api/vscode/ext.host.window-state.ts 16.66% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4233      +/-   ##
==========================================
- Coverage   53.99%   53.92%   -0.07%     
==========================================
  Files        1616     1618       +2     
  Lines       98129    98295     +166     
  Branches    20080    20113      +33     
==========================================
+ Hits        52986    53010      +24     
- Misses      37503    37623     +120     
- Partials     7640     7662      +22     
Flag Coverage Δ
jsdom 49.52% <10.60%> (-0.07%) ⬇️
node 15.51% <0.00%> (-0.03%) ⬇️

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.

@bk1012 bk1012 changed the title WIP: feat: support VSCode API: WindowState.active feat: support VSCode API: WindowState.active Dec 17, 2024
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: 0

🧹 Nitpick comments (5)
packages/extension/src/browser/vscode/api/main.thread.window-state.ts (1)

21-24: blurHandler 和 focusHandler 中改为调用 $onDidChangeWindowFocus 透传焦点事件,可更好地与上层扩展或宿主同步窗口焦点状态。若后续需要扩展其他窗口事件(如最小化、最大化等),可以采用类似思路。

packages/extension/src/browser/vscode/api/window-activity/window-activity-timer.ts (4)

3-4: 建议:使用更具可读性的常量命名方式
使用更能体现含义的常量名可以提高可读性,例如使用 MAX_IDLE_TICKS、IDLE_CHECK_MS 等来表示含义。


16-23: 建议:在构造函数中添加错误处理或边界检查
目前的构造函数仅接收参数并设置默认值,如果外界传入的 inactivityLimit 或 checkInterval 值无效(如负数),可能导致意外行为。可以考虑添加简单的边界判断。


64-71: 提示:考虑在 checkInactivity 中记录更多元数据
目前仅有计数递增并与最大值比较,可根据需要在此处记录与分析用户行为,如最后一次活动时间戳,有助于后续统计。


81-87: 建议:对计时器为 undefined 的情况保持一致性
stopInactivityCheck 中对 inactivityCheckTimer 进行判空操作并清理后赋值 undefined,这里的模式非常好,注意在其他调用计时器的地方也保持一致。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af57a6c and e48836a.

📒 Files selected for processing (2)
  • packages/extension/src/browser/vscode/api/main.thread.window-state.ts (1 hunks)
  • packages/extension/src/browser/vscode/api/window-activity/window-activity-timer.ts (1 hunks)
🔇 Additional comments (14)
packages/extension/src/browser/vscode/api/main.thread.window-state.ts (6)

3-3: 引入了 DisposableCollection,有助于集中管理清理逻辑,避免分散的手动 dispose 错误。此改动在后续维护中显得更安全和直观。


7-7: 新增了对 WindowActivityTimer 的依赖,可以实现在窗口未聚焦及超时不活动情况下同步对应状态。与 VSCode API 要求的 “active” 状态保持一致,契合 PR 需求。


15-16: toDispose 实例化为 DisposableCollection,能方便地在 dispose 中一次性释放所有资源,特别适合与多个可挂载事件或 Disposable 实例配合使用,推荐使用方式恰当。


30-33: 在构造函数里创建 WindowActivityTimer 并将其 push 到 toDispose 中,使得活跃状态监听器统一释放,防止内存泄漏。此实现简洁明了。


35-36: onActiveStateChanged 在活跃状态变化时调用代理的 $onDidChangeWindowActive,和 blur/focus 事件配合使用,确保活跃和焦点两种状态都能正确区分。这个逻辑对实现 VSCode API 的 WindowState.active 十分关键。


42-42: 在 dispose 中调用 this.toDispose.dispose(),集中销毁 tracker、事件监听器和其他资源,避免遗漏。清理逻辑一致且便于后续维护。

packages/extension/src/browser/vscode/api/window-activity/window-activity-timer.ts (8)

5-11: 认可:继承 Disposable 并保存必要属性的设计
此处继承自 Disposable 并定义必要属性是合理且易于理解的结构,便于后续扩展和资源释放。


12-14: 认可:使用 Emitter 提供事件机制
通过 Emitter 来暴露活动状态改变事件是一个好做法,有助于在其他模块中进行监听与订阅。


26-29: 认可:onDidChangeActiveState 的事件封装便于订阅
通过只读访问器暴露事件实例非常清晰,可防止外部模块直接操作内部 Emitter。


30-36: 赞同:分离 setActiveState 方法以确保逻辑集中
将激活状态的切换与事件通知集中在单独的方法中,方便后续添加更多业务逻辑。


38-52: 提醒:注意事件监听器的多次绑定情况
在 setupActivityListeners 中为同一个 window 添加多个事件监听。确认在重复调用 setupActivityListeners 时不会重复注册监听器并造成多重触发或内存泄漏。


54-62: 认可:通过 resetInactivityTimer 重置计时并保持活跃状态
在用户有活动时立刻将计数归零并切回活跃状态符合预期。


73-80: 认可:startInactivityCheck 与 clearInterval 逻辑拆分清晰
在启动前先停止,避免重复计时器,是常见且正确的写法,逻辑简洁。


89-107: 认可:dispose 方法完整地清理了所有资源
这里不仅停止计时器,还释放了 Emitter,移除了事件监听器,保证了不会造成内存泄漏,且与 Disposable 的设计理念相符。

@bk1012 bk1012 merged commit b5efb7c into opensumi:main Dec 18, 2024
@quxingkai quxingkai deleted the WindowState branch January 3, 2025 01:59
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