feat: add notebook variable inspector panel #4186
Conversation
Walkthrough此拉取请求引入了多个新本地化字符串，主要用于笔记本变量面板的用户界面，增强了对变量检查的支持。具体修改包括添加成功和失败的刷新操作消息、搜索变量名称的占位符以及查看详细信息的提示。此外，移除了与内核面板相关的多个组件和类，更新了模块的注册逻辑，并引入了新的变量管理界面和样式。整体结构保持不变，但功能得到了扩展。 Changes
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (13)
packages/notebook/src/browser/variables/index.less (2)
1-61: 建议改进容器样式的响应性和可访问性以下几点建议：
- 建议避免使用固定的高度计算值（40px），考虑使用更灵活的方式
- 缺少键盘焦点状态样式，影响可访问性
- 变量名称过长时可能存在显示问题
建议应用以下修改：
.libro-variable-content { - height: calc(100% - 40px); + height: 100%; + padding-top: var(--mana-libro-header-height, 40px); + box-sizing: border-box; .libro-variable-list { .ant-list-item { &:hover, + &:focus-visible { border-radius: 6px; - background-color: var(--mana-libro-variable-background-hover-color); + background-color: var(--mana-libro-variable-interactive-color); } .libro-variable-name { color: var(--mana-libro-variable-name-color); + word-break: break-word; } } } }
95-102: 优化空状态样式的实现方式当前实现存在以下问题：
- 使用了
!important标记，这通常不是最佳实践- 空状态缺少适当的间距和文本样式定义
建议按照以下方式优化：
.libro-variable-empty { display: flex; flex-direction: column; align-items: center; justify-content: center; height: 100%; - margin: unset !important; + margin: 0; + padding: 16px; + color: var(--mana-libro-variable-description-color); + font-size: var(--mana-libro-font-size); }packages/notebook/src/browser/variables/libro-variable-color-registry.ts (3)
8-8: 请移除未使用的注释代码代码中存在被注释掉的依赖注入语句，如果不再需要建议直接删除，以保持代码整洁。
- // @inject(OpensumiInjector) injector: Injector;
11-130: 建议完善颜色注册的文档和结构发现以下需要改进的地方：
- 所有颜色定义都缺少描述信息，建议添加清晰的说明。
- 颜色分组需要更清晰的结构和注释。
- 部分颜色值使用了硬编码的十六进制值。
建议按照以下方式重构：
- 为每个颜色添加有意义的描述：
{ id: 'libro.variable.search.background.color', defaults: { dark: '#ffffff0a', light: Color.rgba(0, 10, 26, 0.04), }, - description: '', + description: '变量搜索框背景颜色', },
- 使用清晰的分组注释：
+ // #region 变量面板基础颜色 { id: 'libro.variable.search.background.color', ... }, + // #endregion + // #region 变量标签颜色 { id: 'libro.variable.tag.background.color', ... }, + // #endregion
- 考虑将常用的颜色值提取为常量：
const DARK_BACKGROUND = '#ffffff0a'; const LIGHT_TEXT = Color.rgba(0, 10, 26, 0.26);
12-12: region 注释不完整代码中只有 region 开始注释，但缺少对应的 endregion。
- // #region antd variable + // #region antd variables + // ... color definitions ... + // #endregionpackages/notebook/src/browser/variables/variable-panel.tsx (1)
30-44: 提取重复代码以提高代码可维护性在处理
editorService.currentResource和editorService.onActiveResourceChange时，存在重复的逻辑。建议将重复的部分提取成一个独立的函数，以减少代码冗余，提升代码的可读性和维护性。可以将重复的逻辑提取为一个函数，例如：
const handleResourceChange = (resource) => { if (resource?.uri.path.ext === `.${LIBRO_COMPONENTS_SCHEME_ID}`) { libroOpensumiService.getOrCreateLibroView(resource.uri).then((libro) => { const viewManager = manaContainer.get(ViewManager); viewManager .getOrCreateView<LibroVariablePanelView>(LibroVariablePanelView, { id: resource.uri.toString(), }) .then((libroVariablePanelView) => { libroVariablePanelView?.pause(); libroVariablePanelView.parent = libro; libroVariablePanelView.update(); setLibroVariablePanelView(libroVariablePanelView); }); }); } else { setLibroVariablePanelView(undefined); } };然后在需要的地方调用：
- if (editorService.currentResource?.uri.path.ext === `.${LIBRO_COMPONENTS_SCHEME_ID}`) { - // 原有逻辑 - } + handleResourceChange(editorService.currentResource); editorService.onActiveResourceChange((e) => { - if (e?.uri.path.ext === `.${LIBRO_COMPONENTS_SCHEME_ID}`) { - // 原有逻辑 - } else { - setLibroVariablePanelView(undefined); - } + handleResourceChange(e); });这样可以减少代码重复，提升维护效率。
Also applies to: 48-61
packages/notebook/src/browser/libro.contribution.tsx (1)
132-134: 建议使用本地资源替换远程图标 URL在
iconClass中，使用了远程图标的 URL。如果这些外部资源不可用，可能导致图标无法显示。建议将图标资源本地化，或者确保外部资源的高可用性和稳定性。packages/notebook/src/browser/variables/variable-view.tsx (3)
60-60: 修正组件名称的拼写错误
DesrendeItem可能是拼写错误，建议修改为更具意义的名称，例如DescRenderItem，以提高代码可读性。-const DesrendeItem = () => <Tooltip title={item.varContent}>{item.varContent}</Tooltip>; +const DescRenderItem = () => <Tooltip title={item.varContent}>{item.varContent}</Tooltip>;
118-123: 简化过滤条件逻辑在
filteredList方法中，过滤器内的if条件检查是多余的，建议直接返回过滤结果，以简化代码。return this.list.filter((item) => { - if (!this.searchValue) { - return true; - } return item.varName.includes(this.searchValue); });可以进一步简化为：
return this.list.filter((item) => item.varName.includes(this.searchValue));
252-282: 提取重复的代码以减少代码重复在
handleQueryResponse方法中，处理'execute_result'和'display_data'消息类型的代码逻辑相似，建议提取公共部分到一个辅助函数，以提高代码的可维护性。示例：
private parseResponseContent(response: KernelMessage.IIOPubMessage): void { const payload = response as KernelMessage.IExecuteResultMsg; let content: string = payload.content.data['text/plain'] as string; if (content.startsWith("'") || content.startsWith('"')) { content = content.slice(1, -1).replace(/\\"/g, '"').replace(/\\'/g, "'"); } this.list = JSON.parse(content) as IVariable[]; } protected handleQueryResponse = (response: KernelMessage.IIOPubMessage) => { const msgType = response.header.msg_type; switch (msgType) { case 'execute_result': case 'display_data': this.parseResponseContent(response); break; default: break; } };packages/notebook/src/browser/variables/inspector-script.ts (3)
327-329: 发现未完成的 'widgetQueryCommand' 实现在 R 语言配置中，
widgetQueryCommand被设置为'TODO'，这表示该功能尚未实现。为了确保变量检查器的完整功能，需要提供 R 语言的widgetQueryCommand实现。您是否需要帮助来实现此功能？我可以协助编写相应的命令，或者为此创建一个新的 GitHub issue。
334-336: 发现未完成的 Scala 语言配置项在 Scala 配置中，
matrixQueryCommand、widgetQueryCommand和deleteCommand当前为空，且标有// TODO。这表明这些功能尚未实现。您是否需要帮助来完善这些配置项？我可以协助编写相应的命令，或者为此创建一个新的 GitHub issue。
340-348: 建议将getScript方法改为同步函数
getScript方法中的操作是同步的，没有异步过程。建议将其改为同步函数，直接返回对应的LanguageModel，以简化代码并提高性能。示例修改：
- public static getScript(lang: string): Promise<LanguageModel> { - return new Promise((resolve, reject) => { - if (lang in Languages.scripts) { - resolve(Languages.scripts[lang]); - } else { - reject('Language ' + lang + ' not supported yet!'); - } - }); - } + public static getScript(lang: string): LanguageModel { + if (lang in Languages.scripts) { + return Languages.scripts[lang]; + } else { + throw new Error('Language ' + lang + ' not supported yet!'); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
packages/i18n/src/common/en-US.lang.ts(1 hunks)packages/i18n/src/common/zh-CN.lang.ts(1 hunks)packages/notebook/src/browser/index.ts(0 hunks)packages/notebook/src/browser/kernel-panel/index.ts(0 hunks)packages/notebook/src/browser/kernel-panel/kernel.panel.contribution.ts(0 hunks)packages/notebook/src/browser/libro.contribution.tsx(7 hunks)packages/notebook/src/browser/variables/index.less(1 hunks)packages/notebook/src/browser/variables/inspector-script.ts(1 hunks)packages/notebook/src/browser/variables/libro-variable-color-registry.ts(1 hunks)packages/notebook/src/browser/variables/libro-variable-module.ts(1 hunks)packages/notebook/src/browser/variables/variable-panel.tsx(1 hunks)packages/notebook/src/browser/variables/variable-protocol.ts(1 hunks)packages/notebook/src/browser/variables/variable-view.tsx(1 hunks)
💤 Files with no reviewable changes (3)
- packages/notebook/src/browser/index.ts
- packages/notebook/src/browser/kernel-panel/index.ts
- packages/notebook/src/browser/kernel-panel/kernel.panel.contribution.ts
✅ Files skipped from review due to trivial changes (2)
- packages/notebook/src/browser/variables/libro-variable-module.ts
- packages/notebook/src/browser/variables/variable-protocol.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/notebook/src/browser/variables/inspector-script.ts
[error] 9-349: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/notebook/src/browser/variables/variable-view.tsx
[error] 26-26: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (4)
packages/i18n/src/common/zh-CN.lang.ts (1)
1277-1282: 变量检查器相关的翻译内容清晰完整！
这组翻译符合以下要求：
- 术语使用准确，如"变量检查器"而不是直译的"变量检视器"
- 提示信息简洁明了
- 操作反馈信息清晰
- 用语符合中文用户习惯
packages/i18n/src/common/en-US.lang.ts (1)
1516-1521: 新增的笔记本变量面板本地化字符串看起来不错！
这些字符串清晰地描述了变量检查器面板的各个功能,包括:
- 面板标题
- 文件格式不支持的提示
- 刷新操作的成功/失败消息
- 变量名搜索占位符
- 查看详情的按钮文本
字符串的命名和格式与现有的本地化模式保持一致。
packages/notebook/src/browser/libro.contribution.tsx (2)
147-152: 确认组件注册时 weight 和 priority 属性的必要性
在注册 VariablePanel 时，同时设置了 weight: 2 和 priority: 1。请确认这两个属性在组件排序或显示中的作用，是否需要同时设置，或者是否存在冲突。
128-160: 新增的 registerComponent 方法实现正确
registerComponent 方法成功地将 KernelPanel 和 VariablePanel 注册到了组件注册表，符合预期功能。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4186 +/- ##
==========================================
- Coverage 54.06% 54.05% -0.02%
==========================================
Files 1612 1616 +4
Lines 97925 97959 +34
Branches 20043 20046 +3
==========================================
+ Hits 52942 52950 +8
- Misses 37372 37397 +25
- Partials 7611 7612 +1
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.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
packages/notebook/src/browser/variables/variable-panel.tsx (2)
17-20: 建议将内联样式移至样式表建议将
collapsePanelContainerStyle对象移至样式表中，以保持代码的一致性和可维护性。这样可以：
- 提高代码的可读性
- 便于统一管理样式
- 支持样式的复用
+ // index.less + .collapse-panel-container { + width: 100%; + height: 100%; + } // variable-panel.tsx - const collapsePanelContainerStyle = { - width: viewState.width || '100%', - height: viewState.height, - }; + const containerStyle = { + ...viewState, + };
68-84: 建议简化渲染逻辑当前的渲染逻辑包含了不必要的片段包装和冗余的返回语句，可以进行简化以提高代码的可读性。
- if (libroVariablePanelView) { - return ( - <> - <div style={collapsePanelContainerStyle}> - {<ViewRender view={libroVariablePanelView}></ViewRender>} - </div> - </> - ); - } else { - return ( - <> - <Empty - image={Empty.PRESENTED_IMAGE_SIMPLE} - description={localize('notebook.variable.panel.unsupported')} - className='libro-variable-empty' - /> - </> - ); - } + return libroVariablePanelView ? ( + <div style={collapsePanelContainerStyle}> + <ViewRender view={libroVariablePanelView} /> + </div> + ) : ( + <Empty + image={Empty.PRESENTED_IMAGE_SIMPLE} + description={localize('notebook.variable.panel.unsupported')} + className='libro-variable-empty' + /> + );packages/notebook/src/browser/libro.contribution.tsx (2)
18-19: 导入语句的组织需要优化建议将相关的导入语句进行分组整理，提高代码的可读性：
- 将变量面板相关的导入（VariablePanel、VARIABLE_ID等）集中放置
- 将核心组件相关的导入（ComponentContribution、ComponentRegistry等）集中放置
建议按以下方式重组导入语句：
import { AppConfig, ClientAppContextContribution, ClientAppContribution, CommandContribution, CommandRegistry, + ComponentContribution, + ComponentRegistry, Disposable, Domain, IClientApp, IOpenerService, OpenerContribution, Schemes, URI, + localize, } from '@opensumi/ide-core-browser'; // 变量面板相关导入 +import { LibroVariableModule } from './variables/libro-variable-module'; +import { VariablePanel } from './variables/variable-panel'; +import { VARIABLE_ID } from './variables/variable-protocol';Also applies to: 27-27, 42-42, 52-54
65-65: 模块注册顺序建议调整当前模块注册的顺序可能需要根据依赖关系进行优化。建议将核心模块放在前面，扩展模块放在后面。
- modules={[ManaAppPreset, LibroJupyterNoEditorModule, LibroTOCModule, LibroOpensumiModule, LibroVariableModule]} + modules={[ + // 核心模块 + ManaAppPreset, + LibroJupyterNoEditorModule, + // 扩展模块 + LibroOpensumiModule, + LibroTOCModule, + LibroVariableModule, + ]}packages/notebook/src/browser/variables/variable-view.tsx (1)
252-282: 建议重构重复的响应处理逻辑
handleQueryResponse方法中处理 'execute_result' 和 'display_data' 的逻辑存在重复。建议提取共同逻辑到单独的方法：
protected parseContent(content: string): IVariable[] { if (content.slice(0, 1) === "'" || content.slice(0, 1) === '"') { content = content.slice(1, -1); content = content.replace(/\\"/g, '"').replace(/\\'/g, "'"); } return JSON.parse(content); } protected handleQueryResponse = (response: KernelMessage.IIOPubMessage) => { const msgType = response.header.msg_type; if (msgType === 'execute_result' || msgType === 'display_data') { const payload = response as KernelMessage.IExecuteResultMsg; const content = payload.content.data['text/plain'] as string; this.list = this.parseContent(content); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/notebook/src/browser/libro.contribution.tsx(7 hunks)packages/notebook/src/browser/variables/variable-panel.tsx(1 hunks)packages/notebook/src/browser/variables/variable-view.tsx(1 hunks)
🔇 Additional comments (2)
packages/notebook/src/browser/libro.contribution.tsx (1)
74-75: 常量命名需要修正
此问题在之前的评审中已经提出，常量 NOTE_BOOK_PANEL_ID 的命名仍需要修正。
packages/notebook/src/browser/variables/variable-view.tsx (1)
87-95: 接口定义清晰完整
IVariable 接口的属性定义明确，类型完整，能够很好地描述变量的各个特征。
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/notebook/src/browser/variables/variable-view.tsx (1)
251-281: 建议提取重复代码到独立函数
handleQueryResponse方法中存在重复的代码逻辑，建议重构以提高可维护性：
- 提取字符串处理逻辑
- 合并相似的消息处理逻辑
+private processContent(content: string): string { + if (content.slice(0, 1) === "'" || content.slice(0, 1) === '"') { + content = content.slice(1, -1); + return content.replace(/\\"/g, '"').replace(/\\'/g, "'"); + } + return content; +} +private updateList(content: string): void { + const processedContent = this.processContent(content); + this.list = JSON.parse(processedContent) as IVariable[]; +} protected handleQueryResponse = (response: KernelMessage.IIOPubMessage) => { const msgType = response.header.msg_type; switch (msgType) { case 'execute_result': { const payload = response as KernelMessage.IExecuteResultMsg; - let content: string = payload.content.data['text/plain'] as string; - if (content.slice(0, 1) === "'" || content.slice(0, 1) === '"') { - content = content.slice(1, -1); - content = content.replace(/\\"/g, '"').replace(/\\'/g, "'"); - } - const update = JSON.parse(content) as IVariable[]; - this.list = update; + this.updateList(payload.content.data['text/plain'] as string); break; } case 'display_data': { const payloadDisplay = response as KernelMessage.IExecuteResultMsg; - let contentDisplay: string = payloadDisplay.content.data['text/plain'] as string; - if (contentDisplay.slice(0, 1) === "'" || contentDisplay.slice(0, 1) === '"') { - contentDisplay = contentDisplay.slice(1, -1); - contentDisplay = contentDisplay.replace(/\\"/g, '"').replace(/\\'/g, "'"); - } - const updateDisplay = JSON.parse(contentDisplay) as IVariable[]; - this.list = updateDisplay; + this.updateList(payloadDisplay.content.data['text/plain'] as string); break; } default: break; } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/notebook/src/browser/variables/libro-variable-color-registry.ts(1 hunks)packages/notebook/src/browser/variables/variable-view.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/notebook/src/browser/variables/libro-variable-color-registry.ts
🔇 Additional comments (3)
packages/notebook/src/browser/variables/variable-view.tsx (3)
44-52: 建议避免使用硬编码的图片 URL
建议将图片资源本地化，避免依赖外部 CDN。
242-249: 建议添加空值检查而不是使用非空断言
在使用 this.languageModel 时直接使用非空断言 ! 可能导致运行时错误。
298-310: 建议增强错误处理
fetch 方法中的错误处理可以更加完善，特别是在处理内核连接和执行请求时的异常情况。
* feat: add notebook variable inspector panel * fix: dispose event * chore: simplify create logic Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: lint * fix: missing deps --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Types
Background or solution
支持快速 Notebook 文件内的变量
Changelog
feat: add notebook variable inspector panel
Summary by CodeRabbit
新功能
VariablePanel组件，用于在笔记本界面中管理和显示变量。LibroVariableComponent组件，提供变量的显示和交互功能。LibroVariableModule，整合多个变量管理组件。样式
文档