feat(desktop): Quick Open で :行番号 入力時に指定行へジャンプ#384
Merged
Conversation
…umn] Closes #383. When using Cmd+P (Quick Open), typing `foo.ts:123` or `foo.ts:123:45` now opens the file and scrolls to the specified line/column, matching VS Code's Quick Open behavior. Changes: - Extract `parseQuickOpenQuery()` to split `path:line[:col]` from user input - Wire parsed line/column through useCommandPalette → onSelectFile → addFileViewerPane (v1) or openFilePane (v2) - Extend v2 FilePaneData with line/column/cursorRequestId fields - Add revealPosition effect in v2 CodeEditor that fires on cursorRequestId - 12 unit tests for the parser 💘 Generated with Crush Assisted-by: GLM-5 via Crush <crush@charm.land>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 Walkthroughウォークスルーコマンドパレットで「ファイル:行:列」構文の解析を導入し、カーソル位置データ(行、列、cursorRequestId)をコマンドパレットからファイルペイン、ビューレンダラー、CodeView、CodeEditorコンポーネントを通じてスレッド化します。CodeEditorは要求されたカーソル位置を表示します。 変更内容
シーケンス図sequenceDiagram
actor User
participant CommandPalette
participant useCommandPalette
participant page.tsx as Workspace Page
participant FilePane
participant CodeEditor
User->>CommandPalette: 入力「file.ts:123:45」
CommandPalette->>useCommandPalette: クエリ送信
useCommandPalette->>useCommandPalette: parseQuickOpenQuery実行
Note over useCommandPalette: searchQuery="file.ts"<br/>line=123, column=45
useCommandPalette->>User: onSelectFile(filePath, line, column)
User->>page.tsx: openFilePane(filePath, displayName, location)
page.tsx->>page.tsx: cursorRequestId生成<br/>FilePaneData作成
page.tsx->>FilePane: initialLine, initialColumn,<br/>cursorRequestId渡す
FilePane->>CodeEditor: ViewProps経由で渡す
CodeEditor->>CodeEditor: viewReady待機
Note over CodeEditor: エディタ初期化完了
CodeEditor->>CodeEditor: revealPosition(123, 45)実行
CodeEditor->>User: カーソルを指定位置に表示
推定コード確認工数🎯 4 (複雑) | ⏱️ 約45分 関連の可能性があるPR
ポエム
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…adapter The revealPosition effect was creating a new CodeMirrorAdapter on each fire without disposing it. Replace with direct EditorView dispatch using EditorSelection.cursor, matching the adapter's internal implementation. 💘 Generated with Crush Assisted-by: GLM-5 via Crush <crush@charm.land>
![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
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx (1)
241-244:crypto.randomUUID()に統一するとより簡潔かつ衝突耐性も高くなります。同ファイル内（例: Line 437, 559, 607–608）では一意 ID 生成に
crypto.randomUUID()を使っており、こちらのcursorRequestIdも揃えるとコードベース全体で一貫します。♻️ 修正案
- const cursorRequestId = - location?.line !== undefined - ? `${Date.now()}:${Math.random().toString(36).slice(2, 10)}` - : undefined; + const cursorRequestId = + location?.line !== undefined ? crypto.randomUUID() : undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx around lines 241 - 244, Replace the ad-hoc ID generation for cursorRequestId (the ternary that uses Date.now() + Math.random().toString(36).slice(...)) with crypto.randomUUID() so it matches other usages in this file; update the assignment where cursorRequestId is declared to return crypto.randomUUID() when location?.line is defined and undefined otherwise, preserving the existing conditional logic and variable name.apps/desktop/src/renderer/screens/main/components/CommandPalette/useCommandPalette.ts (1)
18-18: 同一モジュール内からの再エクスポートは不要な可能性があります。
parseQuickOpenQueryはindex.ts経由でも再エクスポートされているとサマリにあります。useCommandPalette.tsのパブリック面にparseQuickOpenQueryを混在させると、CommandPaletteフックを import する側に無関係なシンボルが露出し、インポート経路が複数になって一貫性が崩れます。テスト等での利用であれば、./parseQuickOpenQueryまたは./indexから直接 import する方が自然です。♻️ 提案
-export { parseQuickOpenQuery } from "./parseQuickOpenQuery"; -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/CommandPalette/useCommandPalette.ts` at line 18, Remove the unnecessary re-export of parseQuickOpenQuery from useCommandPalette.ts: delete the line exporting parseQuickOpenQuery so the CommandPalette hook file only exposes the hook, and update any consumers to import parseQuickOpenQuery directly from ./parseQuickOpenQuery or from the module index.ts; this keeps useCommandPalette (the CommandPalette hook) API focused and avoids multiple import paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 250-266: The update branch is overwriting an existing pane
displayName with undefined when reopening via CommandPalette; in the
state.setPaneData call (related symbols: activeData, displayName,
state.setPaneData, active.pane.id, FilePaneData) change the data assignment so
displayName uses a fallback to the existing activeData.displayName (e.g.
displayName ?? activeData.displayName) instead of writing the raw displayName,
and keep the shouldUpdateData condition as-is (or optionally ensure it still
triggers for location changes) so you don't lose previously stored display names
when displayName is undefined.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 241-244: Replace the ad-hoc ID generation for cursorRequestId (the
ternary that uses Date.now() + Math.random().toString(36).slice(...)) with
crypto.randomUUID() so it matches other usages in this file; update the
assignment where cursorRequestId is declared to return crypto.randomUUID() when
location?.line is defined and undefined otherwise, preserving the existing
conditional logic and variable name.
In
`@apps/desktop/src/renderer/screens/main/components/CommandPalette/useCommandPalette.ts`:
- Line 18: Remove the unnecessary re-export of parseQuickOpenQuery from
useCommandPalette.ts: delete the line exporting parseQuickOpenQuery so the
CommandPalette hook file only exposes the hook, and update any consumers to
import parseQuickOpenQuery directly from ./parseQuickOpenQuery or from the
module index.ts; this keeps useCommandPalette (the CommandPalette hook) API
focused and avoids multiple import paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e3c0111-f0a2-4ad8-b6bd-0b69e44ff29c
📒 Files selected for processing (10)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/CodeView/CodeView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/registry/views/CodeView/components/CodeEditor/CodeEditor.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types.tsapps/desktop/src/renderer/screens/main/components/CommandPalette/index.tsapps/desktop/src/renderer/screens/main/components/CommandPalette/parseQuickOpenQuery.test.tsapps/desktop/src/renderer/screens/main/components/CommandPalette/parseQuickOpenQuery.tsapps/desktop/src/renderer/screens/main/components/CommandPalette/useCommandPalette.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1254cd2ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Use crypto.randomUUID() for cursorRequestId (consistent with file) - Remove unnecessary re-export of parseQuickOpenQuery from useCommandPalette - Preserve existing displayName when reopening via CommandPalette - Format Math.max to single line per biome 💘 Generated with Crush Assisted-by: GLM-5 via Crush <crush@charm.land>
index.ts was still re-exporting parseQuickOpenQuery from useCommandPalette instead of the extracted module. 💘 Generated with Crush Assisted-by: GLM-5 via Crush <crush@charm.land>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
foo.ts:123やfoo.ts:123:45のように入力した際、ファイルを開いた後指定行/列に自動スクロール・ジャンプする機能を追加:line[:column]記法に対応変更点
parseQuickOpenQuery(): 入力テキストからpath:line[:column]を分離する純粋関数（テスト 12 件）useCommandPalette: クエリをパースし、ファイル検索はパス部分のみで実行、選択時に行/列をコールバックに渡すaddFileViewerPaneにline/columnを渡すよう修正（既存のinitialLine/initialColumn機構が消費）FilePaneDataにline/column/cursorRequestIdを追加し、CodeEditorのrevealPositionに接続cursorRequestIdは一意 ID で、同じファイルに連続で異なる行を指定した場合でも React が effect を再実行する仕組みTest plan
parseQuickOpenQueryのユニットテスト 12 件（全通過）foo.ts:42入力 → ファイルが開き 42 行目にジャンプsrc/bar.tsx:10:5入力 → 10 行 5 列にジャンプfoo.ts単独 → 従来どおりファイルを開くのみCloses #383
💘 Generated with Crush
Assisted-by: GLM-5 via Crush crush@charm.land
Summary by CodeRabbit
新機能