fix: restored notebook have invalid keybinding #4174
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 [email protected]: 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: 1
๐งน Outside diff range and nitpick comments (1)
packages/notebook/src/browser/libro-keybind-contribution.ts (1)
Line range hint
89-119: ไฟฎๅค้ฎ็ปๅฎไธญ็ฉบ็ๅฝไปคๅญๆฎตๅจ
registerKeybindingsๆนๆณไธญ๏ผ'f1'ใ'f8'ใ'f9' ้ฎ็ปๅฎ็commandๅญๆฎตไธบ็ฉบ๏ผๅฏ่ฝๅฏผ่ด้ฎ็ปๅฎๆ ๆณ็ๆใ่ฏทไธบ่ฟไบ้ฎ็ปๅฎๆๅฎๆญฃ็กฎ็ๅฝไปคใ่ฏทๅบ็จไปฅไธไฟฎๆน๏ผ
{ keybinding: 'f1', - command: '', + command: '้่ฆๆง่ก็ๅฝไปคID', when: 'libroNotebookFocused', }, { keybinding: 'f8', - command: '', + command: '้่ฆๆง่ก็ๅฝไปคID', when: 'libroNotebookFocused', }, { keybinding: 'f9', - command: '', + command: '้่ฆๆง่ก็ๅฝไปคID', when: 'libroNotebookFocused', },่ฏทๅฐ
'้่ฆๆง่ก็ๅฝไปคID'ๆฟๆขไธบๅฎ้ ็ๅฝไปคๆ ่ฏ็ฌฆใ
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
๐ Files selected for processing (2)
packages/notebook/src/browser/libro-keybind-contribution.ts(2 hunks)packages/notebook/src/browser/libro/libro-opensumi-content-contribution.ts(2 hunks)
๐ Additional comments (7)
packages/notebook/src/browser/libro/libro-opensumi-content-contribution.ts (2)
16-16: ่กจ่พพๅผๆ ผๅผไผๅ
ๆฌๅท็ๆทปๅ ๆ้ซไบๆกไปถ่กจ่พพๅผ็ๅฏ่ฏปๆง๏ผ่ฟๆฏไธไธชๅฅฝ็ๆน่ฟใ
36-37: ้ช่ฏๆไปถๅๅค็็ๆนๅจๅฝฑๅ
ๅฐ uri.path.name ๆนไธบ uri.path.base ๆฏไธไธช้่ฆๅๆด๏ผ้่ฆ็กฎไฟๅจๆๆๅบๆฏไธ้ฝ่ฝๆญฃ็กฎๅค็ๆไปถๅใๅฆๅค๏ผTODO ๆณจ้ๆๅฐ็็ฌ่ฎฐๆฌๆ น็ฎๅฝ้ฎ้ข้่ฆ่ฟไธๆญฅ่ฎจ่ฎบใ
่ฟ่กไปฅไธ่ๆฌๆฅ้ช่ฏ็ธๅ ณไปฃ็ ๏ผ
โ Verification successful
Let me gather more information about the URI path handling in the codebase.
ๆไปถๅๅค็ๆนๅจๅทฒๅพๅฐ้ช่ฏ
ไปฃ็ ๅบไธญๅทฒๅนฟๆณไฝฟ็จ uri.path.base ไฝไธบ่ทๅๆไปถๅ็ๆ ๅๆนๅผ๏ผๅ
ๆฌ็ผ่พๅจใๆไปถๆ ใ่ฐ่ฏ็ญๅคไธชๆ ธๅฟๆจกๅใๅฐ uri.path.name ๆนไธบ uri.path.base ๆฏ็ฌฆๅ้กน็ฎ่ง่็ๆญฃ็กฎๆนๅจใ
ๅ ณไบ็ฌ่ฎฐๆฌๆ น็ฎๅฝ็ TODO ๆณจ้๏ผ
- ่ฟๆฏไธไธชๅทฒ็ฅ็ๅฑ้ๆง๏ผ็ฑไบ Jupyter ๆชๆไพ่ทๅๆ น็ฎๅฝ็ API
- ็ฎๅ็ๅฎ็ฐๆฏไธไธชๅ็็ไธดๆถๆนๆก
- ๅปบ่ฎฎๅจ Jupyter ๆไพ็ธๅ ณ API ๅๅไผๅๆญคๅค็้ป่พ
๐ Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ๆ่ฟฐ๏ผๆฃๆฅไปฃ็ ๅบไธญๆฏๅฆๅญๅจๅ
ถไปไฝฟ็จ uri.path.name ็ๅฐๆน๏ผไปฅๅ็ฌ่ฎฐๆฌๆ น็ฎๅฝ็็ธๅ
ณๅฎ็ฐ
# ๆ็ดข uri.path.name ็ไฝฟ็จ
rg "uri\.path\.name"
# ๆ็ดข็ฌ่ฎฐๆฌๆ น็ฎๅฝ็ธๅ
ณ็ๅฎ็ฐ
rg -i "notebook.*root.*dir|notebook.*root.*path"
Length of output: 508
Script:
#!/bin/bash
# Check the implementation and usage of uri.path.base
rg "uri\.path\.base"
# Check the context of workspace-variable-contribution.ts where uri.path.name is used
rg -A 5 -B 5 "uri\.path\.name" packages/workspace/src/browser/workspace-variable-contribution.ts
# Check the context of resource.ts where uri.path.name is used
rg -A 5 -B 5 "uri\.path\.name" packages/core-browser/src/contextkey/resource.ts
Length of output: 2292
packages/notebook/src/browser/libro-keybind-contribution.ts (5)
1-1: ๅฏผๅ
ฅๆจกๅๆญฃ็กฎ
ๅฟ ่ฆ็ๆจกๅๅทฒๆญฃ็กฎๅฏผๅ ฅ๏ผ็กฎไฟไบๅ่ฝ็ๆญฃๅธธไฝฟ็จใ
7-14: ๆฐๅขๆฅๅฃๅฏผๅ
ฅ็กฎไฟๅ่ฝๆฉๅฑ
ๅฏผๅ
ฅไบ CommandContributionใKeybindingContribution ็ญๆฅๅฃ๏ผๆปก่ถณไบๅ็ปญๅฎ็ฐ็ธๅบๅ่ฝ็้ๆฑใ
21-22: ็ฑปๅฃฐๆๆดๆฐ็ฌฆๅ่ฎพ่ฎก่ฆๆฑ
LibroKeybindContribution ็ฑป็ฐๅจๅฎ็ฐไบ ClientAppContributionใKeybindingContribution ๅ CommandContribution ๆฅๅฃ๏ผ็กฎไฟ็ฑปๅ
ทๅค็ธๅบ็ๅ่ฝ๏ผๅฎ็ฐไบๆ้็ๅฅ็บฆใ
36-36: ๆๅๅๅปบไธไธๆ้ฎ
ๅๅปบไบไธไธๆ้ฎ libroNotebookFocused๏ผๅๅงๅผ่ฎพไธบ false๏ผๆๅฉไบๅจไธๅ็ปไปถ้ดๅ
ฑไบซ็ถๆใ
31-33: ็กฎ่ฎค็ๅฝๅจๆๆนๆณ็ๆญฃ็กฎๆฟๆข
onStart() ๆนๆณ่ขซๆฟๆขไธบ initialize()ใ่ฏท็กฎไฟๆญคๆดๆน็ฌฆๅๆกๆถๅฏน็ๅฝๅจๆๆนๆณ็ๆๆ๏ผไปฅ้ฟๅ
ๅๅงๅ้ฎ้ขใ
ไธบ็กฎ่ฎคๆกๆถ้ขๆ็็ๅฝๅจๆๆนๆณ๏ผๅปบ่ฎฎ่ฟ่กไปฅไธ่ๆฌ๏ผ
โ Verification successful
Let me gather more information about the lifecycle methods in the codebase.
็ๅฝๅจๆๆนๆณๆฟๆขๆฏๆญฃ็กฎ็
ๆ นๆฎ ClientAppContribution ๆฅๅฃ็ๅฎไน๏ผinitialize() ๆฏๆกๆถ้ขๆ็ๅฏ้็ๅฝๅจๆๆนๆณ๏ผ็จไบๅจๅบ็จๅฏๅจๆถใๅจๅฝไปคใๆ้ฎ็ปๅฎๅ่ๅๅๅงๅไนๅๆง่กใ่ฟไธชๆฟๆขๆฏ็ฌฆๅๆกๆถ่ฎพ่ฎก็ใ
๐ Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ๆฃๆฅๅฎ็ฐไบ ClientAppContribution ๆฅๅฃ็็ฑป๏ผไฝฟ็จไบๅชไบ็ๅฝๅจๆๆนๆณ
rg 'class .* implements .*ClientAppContribution' -A 5 | rg 'initialize\(\)|onStart\(\)'
Length of output: 455
Script:
#!/bin/bash
# ๆฃๆฅ ClientAppContribution ๆฅๅฃ็ๅฎไน
rg "interface.*ClientAppContribution" -A 10
# ๆฃๆฅๆดๅคไฝฟ็จ initialize() ็็คบไพ
ast-grep --pattern 'initialize() {
$$$
}'
# ๆฃๆฅๆดๅคไฝฟ็จ onStart() ็็คบไพ
ast-grep --pattern 'onStart() {
$$$
}'
Length of output: 1116
| registerCommands(commands: CommandRegistry) { | ||
| commands.registerCommand(NotebookCommands['EnterCommandMode'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['EnterCommandMode'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['RunCell'], { | ||
| commands.registerCommand(NotebookCommands['RunCell'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['RunCell'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['RunCellAndSelectNext'], { | ||
| commands.registerCommand(NotebookCommands['RunCellAndSelectNext'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['RunCellAndSelectNext'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['RunCellAndInsertBelow'], { | ||
| commands.registerCommand(NotebookCommands['RunCellAndInsertBelow'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['RunCellAndInsertBelow'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['SplitCellAntCursor'], { | ||
| commands.registerCommand(NotebookCommands['SplitCellAntCursor'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['SplitCellAntCursor'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(LibroSearchToggleCommand.ShowLibroSearch, { | ||
| commands.registerCommand(LibroSearchToggleCommand.ShowLibroSearch, { |
There was a problem hiding this comment.
๐ ๏ธ Refactor suggestion
ๅปบ่ฎฎไผๅๅฝไปคๆณจๅไปฅๅๅฐ้ๅคไปฃ็
registerCommands ๆนๆณไธญๅคๆฌก้ๅคๆณจๅๅฝไปค๏ผๅปบ่ฎฎๆๅๅ
ฌๅ
ฑ้ป่พ๏ผไฝฟ็จๅพช็ฏๆๆ ๅฐๆฅ็ฎๅไปฃ็ ๏ผๆๅๅฏ่ฏปๆงๅๅฏ็ปดๆคๆงใ
ๅฏไปฅ่่ๅฆไธไฟฎๆน๏ผ
registerCommands(commands: CommandRegistry) {
const notebookCommands = [
NotebookCommands['EnterCommandMode'],
NotebookCommands['RunCell'],
NotebookCommands['RunCellAndSelectNext'],
NotebookCommands['RunCellAndInsertBelow'],
NotebookCommands['SplitCellAntCursor'],
LibroSearchToggleCommand.ShowLibroSearch,
];
notebookCommands.forEach((command) => {
commands.registerCommand(command, {
execute: () => {
this.libroCommandRegistry.executeCommand(command.id);
},
});
});
}
Codecov ReportAll modified and coverable lines are covered by tests โ
Additional details and impacted files@@ Coverage Diff @@
## main #4174 +/- ##
=======================================
Coverage 54.07% 54.07%
=======================================
Files 1611 1611
Lines 97846 97846
Branches 20039 20031 -8
=======================================
Hits 52914 52914
Misses 37327 37327
Partials 7605 7605
Flags with carried forward coverage won't be shown. Click here to find out more. โ View full report in Codecov by Sentry. |
Types
Background or solution
Changelog
Summary by CodeRabbit
ๆฐๅ่ฝ
ไฟฎๅค