-
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: terminal search optimization #4384
Conversation
Walkthrough此次改动对终端搜索功能进行了重构和扩展。主要新增了基于 React 的 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 用户
participant TS as TerminalSearch组件
participant TSS as TerminalSearch服务
participant TC as TerminalClient
participant XT as XTerm搜索引擎
U->>TS: 输入搜索关键字和选项
TS->>TSS: 触发搜索请求
TSS->>TC: 调用 findNext/findPrevious 方法
TC->>XT: 传递搜索请求(包含搜索选项)
XT-->>TC: 返回搜索结果
TC-->>TSS: 传递搜索结果及事件通知
TSS-->>TS: 更新 UI 状态及显示结果
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
yarn install v1.22.22 ✨ Finishing Touches
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: 0
🧹 Nitpick comments (5)
packages/terminal-next/src/common/xterm.ts (1)
1-2
: 导入语句的顺序需要调整建议将内部依赖(@opensumi)放在外部依赖(@xterm)之后,以保持一致的导入顺序。
-import { ISearchOptions } from '@xterm/addon-search'; -import { IEvent, ITerminalOptions, ITheme, Terminal } from '@xterm/xterm'; +import { IEvent, ITerminalOptions, ITheme, Terminal } from '@xterm/xterm'; +import { ISearchOptions } from '@xterm/addon-search';packages/terminal-next/src/browser/terminal.search.ts (1)
63-70
: 搜索方法存在代码重复search、searchPrevious 和 searchNext 方法中有大量重复的搜索选项配置代码。建议提取共同的搜索选项逻辑。
+private getSearchOptions() { + return { + wholeWord: this.UIState.isWholeWord, + regex: this.UIState.isUseRegexp, + caseSensitive: this.UIState.isMatchCase, + }; +} @debounce(150) search() { - this.client?.findNext(this.text, { - wholeWord: this.UIState.isWholeWord, - regex: this.UIState.isUseRegexp, - caseSensitive: this.UIState.isMatchCase, - }); + this.client?.findNext(this.text, this.getSearchOptions()); } @debounce(150) searchPrevious() { - this.client?.findPrevious(this.text, { - wholeWord: this.UIState.isWholeWord, - regex: this.UIState.isUseRegexp, - caseSensitive: this.UIState.isMatchCase, - }); + this.client?.findPrevious(this.text, this.getSearchOptions()); } @debounce(150) searchNext(): void { - this.client?.findNext(this.text, { - wholeWord: this.UIState.isWholeWord, - regex: this.UIState.isUseRegexp, - caseSensitive: this.UIState.isMatchCase, - }); + this.client?.findNext(this.text, this.getSearchOptions()); }Also applies to: 73-79, 82-88
packages/terminal-next/src/common/client.ts (1)
120-124
: 需要为新增的搜索方法添加 JSDoc 注释建议为 findPrevious 方法和 onSearchResultsChange 事件添加 JSDoc 注释,保持文档的一致性。
findNext(text: string, searchOptions?: ISearchOptions): void; +/** + * 向上查找字符串 + * + * @param text 用户输入的字符串 + * @param searchOptions 搜索选项 + */ findPrevious(text: string, searchOptions?: ISearchOptions): void; +/** + * 搜索结果变更事件 + * 当搜索结果数量或当前结果索引发生变化时触发 + */ onSearchResultsChange: IEvent<{ resultIndex: number; resultCount: number }>;packages/terminal-next/src/browser/component/search.view.tsx (1)
11-11
: 优化组件 Props 类型定义。空对象类型
{}
作为 Props 类型不够明确。建议定义一个显式的接口来描述组件的 Props。-export const TerminalSearch: React.FC<{}> = React.memo((props) => { +interface TerminalSearchProps { + // 目前没有 props,但为未来扩展预留接口 +} +export const TerminalSearch: React.FC<TerminalSearchProps> = React.memo((props) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
packages/terminal-next/src/browser/component/search.module.less (1)
20-49
: .optionBtn 按钮样式细节优化建议
按钮尺寸、边距、居中对齐等属性设置得较为清晰,同时提供了 hover 状态和选中状态的视觉效果。
建议在&::before
伪元素中,对@option-size - 2
运算明确单位,以防止可能的解析歧义,推荐采用以下方式修改:- height: @option-size - 2; - line-height: @option-size - 2; + height: (@option-size - 2px); + line-height: (@option-size - 2px);这样能确保运算结果为预期的像素值,提高代码健壮性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/terminal-next/src/browser/component/search.module.less
(1 hunks)packages/terminal-next/src/browser/component/search.view.tsx
(1 hunks)packages/terminal-next/src/browser/component/terminal.module.less
(0 hunks)packages/terminal-next/src/browser/component/terminal.view.tsx
(3 hunks)packages/terminal-next/src/browser/terminal.client.ts
(2 hunks)packages/terminal-next/src/browser/terminal.search.ts
(3 hunks)packages/terminal-next/src/browser/xterm.ts
(1 hunks)packages/terminal-next/src/common/client.ts
(2 hunks)packages/terminal-next/src/common/controller.ts
(2 hunks)packages/terminal-next/src/common/xterm.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/terminal-next/src/browser/component/terminal.module.less
🧰 Additional context used
🪛 Biome (1.9.4)
packages/terminal-next/src/browser/component/search.view.tsx
[error] 11-11: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build-windows
🔇 Additional comments (16)
packages/terminal-next/src/common/xterm.ts (2)
17-17
: 搜索结果变更事件的类型定义很好使用 IEvent 类型和具体的结果数据结构,可以让搜索结果的状态变更得到很好的类型支持。
23-24
: 搜索方法的签名设计合理
- findNext 和 findPrevious 方法签名保持一致
- searchOptions 参数设为可选,保持了向后兼容性
- 返回类型为 boolean 表示搜索是否成功
packages/terminal-next/src/browser/terminal.search.ts (2)
16-20
: UIState 的初始状态设置合理所有搜索选项默认为 false,符合用户预期的默认行为。
90-96
: updateUIState 方法的实现很好
- 使用展开运算符实现部分更新
- 更新后立即触发搜索,保持搜索结果同步
packages/terminal-next/src/browser/component/terminal.view.tsx (1)
81-81
: 搜索组件的条件渲染实现合理使用 isVisible 状态控制 TerminalSearch 组件的渲染,符合 React 最佳实践。
packages/terminal-next/src/browser/component/search.view.tsx (3)
18-41
: 实现良好!useEffect 的实现考虑周到:
- 正确处理了清理函数
- 依赖项设置合理
- 关注点分离得当
43-91
: 回调函数实现优秀!
- 使用 useCallback 正确地缓存了回调函数
- 依赖项列表完整且合理
- 处理逻辑清晰
93-142
: 渲染逻辑结构清晰!
- 合理使用了 ValidateInput 组件
- 正确处理了样式类名组合
- 搜索选项和导航按钮布局合理
- 良好的国际化支持
packages/terminal-next/src/common/controller.ts (1)
118-144
: 接口定义清晰完整!
IUIState
接口完整地定义了搜索选项ISearchResult
接口合理地封装了搜索结果ITerminalSearchService
的更新保持了良好的类型安全packages/terminal-next/src/browser/xterm.ts (1)
180-198
: 搜索相关方法实现完善!
- 正确处理了搜索选项
- 复用 getFindColors 确保样式一致性
- 新增的向前搜索功能实现合理
packages/terminal-next/src/browser/terminal.client.ts (1)
734-746
: 终端客户端搜索功能实现完善!
- 正确检查了终端客户端就绪状态
- 与 XTerm 实现保持一致
- 搜索选项处理合理
packages/terminal-next/src/browser/component/search.module.less (5)
1-2
: 变量定义清晰
通过定义@option-size: 20px;
,方便了后续对按钮尺寸的统一管理。该变量化写法增强了样式的可维护性。
3-14
: .terminalSearch 容器样式布局合理
容器采用 Flex 布局和绝对定位,结合高 z-index 确保组件在终端上层显示。背景颜色通过 CSS 变量设置,有利于与整体主题的统一。注意:使用绝对定位时,若在响应式设计中遇到适配问题可能需要额外调整。
15-19
: .searchField 焦点状态设计得当
为搜索输入域在获得焦点时设置了边框颜色变化,这有助于提升用户交互体验。建议检查其他状态(例如:失去焦点或禁用状态)是否需要类似的视觉反馈。
52-55
: .panelBtn 样式设计简洁有效
为面板按钮设置了适当的内边距和cursor: pointer
,增强了交互性。样式简洁且符合预期,能够很好地融入整体布局。
57-60
: .searchResult 字体显示设置合理
设置了适当的外边距和字体大小,保证了搜索结果显示清晰。根据实际需求,后续可进一步微调间距或字号。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4384 +/- ##
==========================================
- Coverage 54.14% 54.09% -0.05%
==========================================
Files 1621 1622 +1
Lines 100331 100433 +102
Branches 21788 21817 +29
==========================================
+ Hits 54326 54334 +8
- Misses 38201 38280 +79
- Partials 7804 7819 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM
/next |
🎉 PR Next publish successful! 3.7.2-next-1739859371.0 |
@Ricbet 已验证 |
Types
Background or solution
Former terminal searching is poor.

Enrich searching behavior by using more apis from xterm search addon:
Changelog
support terminal searching options (regex/word/case) , searching results showing and prev/next button
Summary by CodeRabbit
新功能
样式改进