-
Notifications
You must be signed in to change notification settings - Fork 403
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
feat: partially supported the VSCode TerminalExitReason API #4195
Conversation
Walkthrough此次更改涉及对终端相关类的测试套件进行多项修改,包括将 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
warning eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options. 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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 (3)
packages/extension/__tests__/hosted/api/vscode/ext.host.terminal.test.ts (1)
Line range hint
193-208
: 建议增加退出原因的测试覆盖当前测试只验证了退出码设置为 2,但没有验证退出原因是否正确设置。建议添加对
exitStatus.reason
的断言。const mockSetStatus = jest.spyOn(terminal4, 'setExitStatus'); // 要等待前台创建完 terminal 示例后,pty 事件绑定完再 fire setTimeout(() => { closeEmitter.fire(2); // 要等待事件 fire 后能监听到 setTimeout(() => { expect(mockTerminalExit).toHaveBeenCalledWith(terminalId, 2); expect(mockSetStatus).toHaveBeenCalled(); expect(terminal4.exitStatus).toBeDefined(); expect(terminal4.exitStatus?.code).toBe(2); + expect(terminal4.exitStatus?.reason).toBe(TerminalExitReason.Process); defered.resolve(); }, 0); }, 0);
packages/extension/src/common/vscode/ext-types.ts (1)
2872-2878
: 建议添加 JSDoc 文档枚举值的定义正确且符合 VSCode API 设计。建议添加 JSDoc 注释说明每个退出原因的具体含义。
+/** + * 定义终端退出的原因。 + */ export enum TerminalExitReason { + /** 未知原因 */ Unknown = 0, + /** 系统关闭 */ Shutdown = 1, + /** 进程退出 */ Process = 2, + /** 用户操作 */ User = 3, + /** 扩展操作 */ Extension = 4, }packages/types/vscode/typings/vscode.d.ts (1)
2962-2990
: 枚举定义完整且文档清晰!
TerminalExitReason
枚举很好地覆盖了所有常见的终端退出场景。建议在文档中添加一个示例代码,展示如何使用这些枚举值来处理不同的终端退出情况。建议添加如下示例代码到文档中:
// 示例:处理不同的终端退出原因 terminal.onDidClose(e => { switch (e.exitStatus?.reason) { case TerminalExitReason.Process: console.log('终端进程已退出'); break; case TerminalExitReason.User: console.log('用户关闭了终端'); break; // ... 处理其他情况 } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
packages/extension/__tests__/hosted/api/vscode/ext.host.terminal.test.ts
(1 hunks)packages/extension/src/common/vscode/ext-types.ts
(1 hunks)packages/extension/src/hosted/api/vscode/ext.host.terminal.ts
(4 hunks)packages/types/vscode/typings/vscode.d.ts
(1 hunks)
🔇 Additional comments (2)
packages/extension/src/hosted/api/vscode/ext.host.terminal.ts (1)
755-757
: 实现正确且类型安全
setExitStatus
方法的实现正确,使用 Object.freeze
确保状态对象不可变,并且参数类型定义完善。
packages/types/vscode/typings/vscode.d.ts (1)
2955-2959
: 代码变更正确且合理!
新增的 reason
属性增强了终端退出状态的报告能力,通过提供退出原因使 API 更加完整。使用 readonly
修饰符确保了属性的不可变性,这是一个很好的 API 设计选择。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4195 +/- ##
=======================================
Coverage 54.05% 54.06%
=======================================
Files 1616 1616
Lines 97963 97970 +7
Branches 20055 20056 +1
=======================================
+ Hits 52955 52964 +9
+ Misses 37396 37393 -3
- Partials 7612 7613 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Types
兼容 vscode TerminalExitReason API,目前适配了 onProcessExit 的场景,其他场景采用兼容 API 的实现,确保插件的正常工作。
Summary by CodeRabbit
TerminalExitReason
枚举,提供了终端退出的多种原因。setExitStatus
取代setExitCode
,并增加了退出原因参数。