feat(navigator): Files / Changes 行に右クリックメニューでファイルパス copy を追加#640
Merged
Conversation
intent(navigator): copy file paths from Files / Changes panes — working tree as absolute path alone, snapshot / commit selection as "{hash}\n{abs path}"
decision(file-context-menu): module singleton usePopover shared by Filer / Changes, mounted once in NavigatorPane. relPath + active worktree dir join happens in the menu to avoid prop drilling
decision(changes): derive contextMenuHash from useGitGraphStore — workingTreeOnly / UNCOMMITTED_HASH fall back through compareHash so range-mode WT endpoints still copy a meaningful commit
rejected(shared): placement under src/shared/ blocked by isolateModules (shared→shared dep on popover/notification forbidden); placed under features/navigator/ since navigator already depends on filer + changes
learned(popover-auto-dismiss): right-click on popover="auto" closes immediately because mousedown reserves light-dismiss consumed on mouseup (whatwg/html#10905). delayed open via pointerup capture-once survives the cycle
📝 WalkthroughWalkthroughこのプルリクエストは、Filer および Changes ペイン内のファイルに対する右クリックコンテキストメニューを実装しています。新たに 🚥 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 docstrings
🧪 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/renderer/src/features/filer/FileTreeItem.vue`:
- Around line 392-410: onContextMenu (and the equivalent in ChangesTreeItem.vue)
currently waits for a pointerup listener before calling openContextMenu which
can miss keyboard-triggered contextmenu events; change onContextMenu to keep the
pointerup listener path but also create a fallback that opens the menu on the
next animation frame or microtask when no pointerup arrives (call
openContextMenu without x/y so the menu positions itself from the anchor), and
ensure useEventListener returns a cleanup function you store so you can cancel
the pending pointerup listener when the fallback runs (and cancel any scheduled
fallback if pointerup fires first); reference the onContextMenu function,
openContextMenu call, and the useEventListener cleanup to implement this.
🪄 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: 98bf5e39-e814-4406-81ae-4cde99c35860
📒 Files selected for processing (7)
apps/renderer/src/features/changes/ChangesPane.vueapps/renderer/src/features/changes/ChangesTreeItem.vueapps/renderer/src/features/filer/FileTreeItem.vueapps/renderer/src/features/navigator/FileContextMenu.vueapps/renderer/src/features/navigator/NavigatorPane.vueapps/renderer/src/features/navigator/index.tsapps/renderer/src/features/navigator/useFileContextMenu.ts
intent(navigator): apply review feedback — fix listener leak, input-source dependency, hash semantics in range mode, dependency direction inversion, missing inert leaf / folder guards decision(navigator): hoist popover open to NavigatorPane, children (FilerPane / ChangesPane / TreeItem) only emit contextMenu and bubble up. removes navigator import from filer / changes (keeps navigator → child as 1-way dependency) decision(navigator): defer showPopover via setTimeout(0) in NavigatorPane to escape the contextmenu mousedown / mouseup cycle (whatwg/html#10905). input-agnostic so keyboard contextmenu / programmatic dispatch also work, unlike the per-row useEventListener pointerup workaround which leaked on unmount decision(changes): contextMenuHash returns undefined in range mode (compareHash !== null). representing multi-commit diffs with a single hash misleads users; single commit selection still returns selectedHash matching Filer's snapshotHash semantics decision(filer): contextmenu on inertLeaf (submodule / snapshot symlink) falls through to OS menu — copying the working tree absolute path of a symlink / gitlink is misleading semantics decision(changes): contextmenu on folder rows falls through to OS menu — gozd has no folder-targeted menu action, do not steal the OS menu decision(filer): expose joinPath via the filer barrel and reuse it in FileContextMenu instead of inline `${dir}/${relPath}` to keep worktree-path joining as a single SSOT rejected(shared-context-menu): placing useFileContextMenu in src/shared/ still requires shared→shared deps (popover / notification) blocked by isolateModules; navigator-owned + event-bubble pattern matches the existing SidebarPane / WorktreeMenu architecture instead
…ix timer lifecycle intent(navigator): apply second-round review feedback — Filer / Changes copy hash asymmetry, joinPath responsibility overrun, directory row inconsistency, payload type duplication, anchor disconnect race, timer lifecycle leak decision(git-graph): introduce useGitGraphStore.contextMenuHash as the single source of truth for "what hash to copy". range mode returns undefined (multi-commit diff cannot be represented by one hash). NavigatorPane reads it at open time; child panes no longer compute or pass commit hash decision(navigator): export FileContextMenuPayload from useFileContextMenu.ts and have every emit site `import type` it. eliminates the 5-place duplicate definition while keeping the import as type-only (no runtime dependency back to navigator) decision(navigator): pendingOpenTimers set + onScopeDispose clearTimeout, and `req.anchorEl.isConnected` guard inside the deferred callback. covers timer leak on unmount and dir-switch races where the anchor element is gone before showPopover fires. disconnected case writes a debug log instead of silent dropping decision(filer): FileTreeItem early-returns for directory rows so the OS context menu shows instead of "Copy file path" with a directory path. mirrors the Changes folder-row behavior. inert leaf guard kept for submodule / snapshot symlink decision(worktree): add joinAbsRel(dir, relPath) to features/worktree/pathUtils.ts as the SSOT for absolute-dir + worktree-relative joining. filerUtils.joinPath stays scoped to worktree-relative concatenation (`""` parent case is its defining contract). FileContextMenu uses joinAbsRel rejected(payload-in-shared): putting FileContextMenuPayload in src/shared/popover mixes file-specific concerns into a generic popover module. type-only re-export from navigator keeps the type close to its only consumer (NavigatorPane) without runtime coupling
…outFn
intent(navigator): apply third-round review feedback — close worktree-switch race during defer, replace handwritten timer queue with useTimeoutFn, harden joinAbsRel against trailing-slash dir, sync NavigatorPane <doc> with current behavior
decision(navigator): snapshot worktreeStore.dir and gitGraphStore.contextMenuHash synchronously when contextMenu fires and freeze them into the popover context. defer-time / menu-show-time store reads were a structural race: switching worktrees mid-defer would compose the old relPath with the new dir. FileContextMenu now reads context.dir instead of the live store
decision(navigator): swap the handwritten pendingOpenTimers Set + onScopeDispose for VueUse useTimeoutFn(_, 0, {immediate:false}). gives scope-bound cleanup, start() cancels the prior pending (last right-click wins, matches popover singleton's openState overwrite semantics)
decision(worktree): joinAbsRel strips trailing slashes from dir and treats stripped "/" as root. Swift URL.path can emit trailing-slash absolute paths; the function now produces "/abs/dir/rel" / "/rel" / "/" deterministically. tests cover trailing slash, repeated slash, root-only, root + relPath, root + empty
decision(worktree): drop the "filer の joinPath とは責務が別" comparison from joinAbsRel docstring. the function name and contract describe the responsibility on their own; the cross-feature reference was leftover review context, not durable documentation
…extMenu
intent(navigator): apply fourth-round review feedback — remove duplicate spec text from FileContextMenu and NavigatorPane <doc> blocks, document intentional silence of pending-cancel and no-active-worktree paths
decision(navigator): make useFileContextMenu.ts docstring the single source of truth for popover semantics (defer / snapshot / disconnect guard / context shape). FileContextMenu.vue <doc> and NavigatorPane.vue <doc> now state only the component-local responsibility and point readers at the SSOT
decision(navigator): document why useTimeoutFn.start silently cancels prior pending — keeping user right-click bursts noise-free; matches popover singleton's last-write-wins openState semantics
decision(navigator): document why "no active worktree" path stays at notification.debug — FilerPane never renders tree items when dir is undefined ("waiting for open command..." instead), so this branch is defensive-only and not user-visible. info toast would conflate it with normal-state messaging
…irectory rows intent(navigator): two user-reported regressions after applying reviewer guidance — menu instantly closing on mouse release (light-dismiss not avoided) and Filer directory rows no longer opening the menu decision(navigator): drop useTimeoutFn / setTimeout(0) defer and go back to pointerup capture listener. WebKit (WebPage) does not let any task / microtask defer escape popover="auto" light-dismiss; only running showPopover after the next pointerup is consumed reliably keeps the menu open. verified on the initial commit (238821e) and confirmed regressed under setTimeout decision(navigator): keep the pointerup listener at setup top-level via useEventListener (no { once: true }) and stash the request in a pending ref. setup-level registration binds it to the effect scope so it cleans up on unmount / HMR, addressing the per-row useEventListener leak that prompted the earlier refactor without re-introducing it decision(filer): restore Filer directory rows as menu targets. directories carry a real filesystem path and copying the absolute path is meaningful. the prior symmetry with Changes folder rows was a false equivalence: Changes folder rows are chain-compressed presentation (e.g. `.github/workflows`) with no single canonical path, hence still excluded decision(navigator): document the unbreakable invariant — `setTimeout(0)` / `requestAnimationFrame` / `queueMicrotask` are all confirmed unable to escape WebKit's popover light-dismiss; only the pointerup capture listener works. Shift+F10 / Apps key / programmatic dispatch produce no pointerup so the menu does not open via those paths; that responsibility is deferred to a future keybinding-system route rather than papered over with a defer trick
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/renderer/src/features/filer/FileTreeItem.vue`:
- Around line 392-416: The onContextMenu handler currently prevents default and
emits a custom contextMenu for directories as well as files; add a guard so only
file leaves trigger the custom menu: inside function onContextMenu, before
calling event.preventDefault() and emit("contextMenu", ...), check and return
early if the node is a directory (e.g. use an existing flag like isDirectory or
node.type === 'directory' — referenced symbols: onContextMenu, isInertLeaf,
emit("contextMenu"), props.path); apply the same directory-guard change to the
other context-menu handler mentioned (the handler at the other occurrence around
line 435) so directories fall through to the OS menu.
🪄 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: 530558d9-1d20-4f16-95d1-e81f39b708cc
📒 Files selected for processing (12)
apps/renderer/src/features/changes/ChangesPane.vueapps/renderer/src/features/changes/ChangesTreeItem.vueapps/renderer/src/features/filer/FileTreeItem.vueapps/renderer/src/features/filer/FilerPane.vueapps/renderer/src/features/git-graph/useGitGraphStore.tsapps/renderer/src/features/navigator/FileContextMenu.vueapps/renderer/src/features/navigator/NavigatorPane.vueapps/renderer/src/features/navigator/index.tsapps/renderer/src/features/navigator/useFileContextMenu.tsapps/renderer/src/features/worktree/index.tsapps/renderer/src/features/worktree/pathUtils.test.tsapps/renderer/src/features/worktree/pathUtils.ts
✅ Files skipped from review due to trivial changes (1)
- apps/renderer/src/features/worktree/index.ts
…ight-click intent(navigator): user-reported regression — Changes folder rows do not open the menu. user expects the menu on every row in Files / Changes, not just file leaves decision(changes): expose displayPath (deepest folder fullPath after chain compression) on folder nodes and emit it as relPath on right-click. user clicks the row showing `.github/workflows`, the menu copies `.github/workflows` — the chain-compressed display always resolves to one concrete deepest folder, so the earlier "path is ambiguous" argument was wrong decision(filer): drop the "Changes folder is excluded by responsibility" comment now that both sides include folder rows. asymmetry was a misread on the reviewer's part, not a real responsibility split decision(navigator): pointerup capture handler filters event.button === 2 so only right-click releases consume the pending. left clicks / middle clicks elsewhere on screen no longer race-fire the menu while a pending is parked decision(navigator): docstring of useFileContextMenu.ts gains a note that the light-dismiss invariant block is duplicated in NavigatorPane.vue. they must be updated together. the duplication is intentional (the NavigatorPane copy sits where the next implementer reads first), but the cross-link makes the contract explicit
intent(navigator): reviewer flagged that event.button === 2 filter breaks macOS WebKit control + click (bugzilla 52174 — control + click dispatches as button=0), and that the new displayPath path on Changes folder rows had no test coverage decision(navigator): remove the event.button check from the pointerup capture listener. pending ref already acts as the "we just had a contextmenu" flag, consuming the next pointerup and nulling itself. multi-button race (right-click held while clicking elsewhere) is an accepted edge case, the realistic right-click → release path remains correct decision(navigator): document the bug in the invariant block — "do not filter by event.button" added to the list alongside "do not defer with setTimeout / rAF" and "do not change capture / pointer phase" decision(changes): add changesTree.test.ts covering displayPath for the five chain-compression boundaries — no compression, single-chain, multi-chain, file/folder mix interrupting the chain, single-file-only branch. displayPath drives the relPath used by Copy file path, so regressing the chain-walk produces incorrect copy contents without compile-time signal
intent(navigator): apply low-severity reviewer feedback (34/35/36) — defend pointerup-only design against pointerdown reset proposals, exercise buildChangesTree error contracts, group tests by responsibility decision(navigator): expand the invariant block with why pointerdown reset cannot be added. right-click pointerdown happens before onFileContextMenu sets pending, so the reset target does not exist yet; pairing reset with subsequent left-click pointerdown then erases pending unexpectedly. keeps future refactors from chasing the same dead-end button-filter detour decision(changes): add throw-contract tests (`a//b.ts`, `a/b.ts/`, `""`) so the invalid-path contract that ChangesPane catches via tryCatch + notify.error is exercised. previously only the happy paths existed decision(changes): nest describe blocks (`buildChangesTree` > `displayPath` / `error contracts`) to keep future additions grouped by responsibility instead of flat
…g note intent(navigator): apply low-severity reviewer feedback (37/38) — rewrite the pointerdown-reset prohibition so its logic reads in one direction, and record why buildChangesTree tests are grouped by responsibility decision(navigator): restructure the docstring sentence so the "right-click sequence wouldn't break" observation comes first as a possible counter-argument, then the actual race (an unrelated pointerdown erasing pending) is the prohibition's load-bearing reason. removes the internal contradiction the reviewer pointed out decision(changes): add a top-level describe comment explaining the nested grouping is for future responsibility-scoped additions (displaySegments / anchorPath etc.), so flattening back is recognized as a regression instead of an aesthetic preference
…not toggle intent(navigator): user reported control+click on Filer/Changes folders toggles them open while the context menu opens. control+click on macOS is meant to be a context menu trigger only, not a click decision(navigator): early-return from both toggle (Filer) and onClick (Changes) when event.ctrlKey is true. WebKit dispatches control+click as button=0 with a regular click event in addition to contextmenu, so both handlers fire; gating click on ctrlKey leaves contextmenu alone but stops the unwanted toggle/select
intent(navigator): apply low-severity reviewer feedback (39/41) — make the control+click rationale traceable and call out the macOS-only assumption baked into the ctrlKey gate decision(navigator): cite webkit bugzilla 52174 in the click-handler comments so the "control+click is dispatched as button=0" claim is verifiable. matches the citation used in NavigatorPane's invariant block decision(navigator): document that gozd is macOS-only (root CLAUDE.md) so ctrlKey === control+click is a safe identity here. note explicitly that cross-platform support would require an OS check before this early return, since Ctrl+Click on Linux/Windows carries different semantics
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.
概要
Filer (Files) / Changes パネルの行に右クリック / control+click でメニューを追加し、ファイルやフォルダの絶対パスを clipboard に copy できるようにする。snapshot / commit 選択中は
{commit hash}\n{絶対パス}の 2 行形式で copy する。背景
これまで「いま見ているファイルやフォルダの絶対パスを別ツールに貼りたい」「過去 commit のあるファイル参照を hash 付きで控えたい」という日常的な copy 要求に応える手段がなかった。Filer / Changes はどちらも active worktree の行を表示しているので、両方の行に統一的な右クリック経路を追加するのが自然な置き場所になる。
設計判断の過程で実機検証と設計レビューを通じて見つかった構造的制約、そのリファクタの経緯:
popover="auto"を contextmenu 同サイクル内でshowPopoverすると mousedown が light-dismiss を予約し、続く mouseup で即閉じる ( whatwg/html issue 10905 )。setTimeout(0)/requestAnimationFrame等の defer は WebKit (WebPage) では light-dismiss を抜けないことが実機で判明したため、pointerupcapture listener を NavigatorPane の setup 直下に常設し、子から bubble する contextmenu event を pending ref に積んで次の pointerup で showPopover する経路にしたbutton=0+ click event として dispatch する ( webkit bugzilla 52174 ) ため、contextmenu と通常 click の両方が発火する。click handler 側でevent.ctrlKey早期 return することで folder の展開 / file の select 経路を抑止し、contextmenu 経路のみが動くようにしたsrc/shared/に置こうとするとisolateModuleslint に引っかかるため、features/navigator/に閉じ込め、子 pane (FilerPane / ChangesPane / TreeItem) は contextMenu event を bubble で navigator まで上げる構造に倒した。子は payload 型のみ type-only import で受け、runtime 依存は navigator → 子の 1 方向に保つdirとcommitHashは 右クリック時に snapshot して popover context に焼き付け、menu 側は live store を読み直さないuseGitGraphStore.contextMenuHashの SSOT に集約 (range mode は undefined、UNCOMMITTED_HASH も undefined、それ以外で selectedHash)。両 pane で同 file の copy 結果が非対称にならないよう統一変更内容
新規モジュール ( features/navigator/ )
useFileContextMenu.ts—usePopover<{ dir, relPath, commitHash?, x?, y? }>()の module singleton。FileContextMenuPayload(子から bubble する event payload 型) を SSOT として export。docstring に light-dismiss 回避の不変条件 (setTimeout/rAF/queueMicrotask不可、{ capture: true }外すな、event.buttonで filter するな、pointerdownで reset するな ) を集約FileContextMenu.vue— Copy file path 1 アクションのポップオーバー。context のdir(右クリック時 snapshot) とrelPathをjoinAbsRelで結合した絶対パスを clipboard に書く。commitHash指定時は{hash}\n{abs path}形式NavigatorPane
<FileContextMenu />を 1 個マウント (singleton state)useEventListener(window, "pointerup", ..., { capture: true })を setup 直下で 1 度だけ登録 (effect scope 連動で auto cleanup)pendingOpenref に dir / hash を snapshot で積み、pointerup で消化 + null 化 (連打時は最後の右クリックだけが menu を開く)anchorEl.isConnectedで defer 中 unmount race をガードFiler
FileTreeItem.vue: 行 button に@contextmenuを追加し、inertLeaf (submodule / snapshot symlink) 以外で preventDefault + emit。toggle内でevent.ctrlKey早期 return (control+click で folder 展開を抑止)FilerPane.vue: 配下から bubble してくる contextMenu を NavigatorPane に bubbleselect-noneで右クリック時のテキスト選択を抑制Changes
ChangesTreeItem.vue: file leaf / folder どちらも menu 対象。folder は chain 圧縮の最深 fullPath (displayPath) を relPath に。onClick内でevent.ctrlKey早期 returnChangesPane.vue: contextMenu を NavigatorPane に bubbleselect-noneでテキスト選択抑制Git graph store
useGitGraphStore.contextMenuHash(computed) を追加。「右クリックで copy する commit hash」の SSOT。range mode は undefined、UNCOMMITTED_HASH も undefined、それ以外で selectedHashChanges tree
ChangesTreeNode.folderにdisplayPath(chain 圧縮の最深 folder fullPath) を追加。chain 圧縮された.github/workflowsのような行でも 1 つの実体 path に決まるため menu 対象にできるchangesTree.test.ts新規追加 —displayPathの境界 5 ケース (圧縮なし / 1 段 / 多段 / file 混在 / root 直下) と throw 契約 3 ケース (重複// 末尾// 空 path) を覆うWorktree pathUtils
joinAbsRel(dir, relPath)を追加。絶対 dir + worktree 相対 path → 絶対 path の SSOT/(SwiftURL.path経由) を defensive strip、dir === "/"の root ケースも safe に処理pathUtils.test.tsに境界テスト 4 件追加Type / 依存方向
FileContextMenuPayloadは navigator が SSOT として export、子 pane はimport typeで参照 (type-only import なので runtime 依存は navigator → 子の 1 方向で閉じる)確認事項
{hash}\n{abs path}が copy される