spec: bindable shortcut to copy editor file path (#10290)#10497
spec: bindable shortcut to copy editor file path (#10290)#10497lonexreb wants to merge 4 commits intowarpdotdev:masterfrom
Conversation
|
I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
github-actions
Bot
added
the
external-contributor
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This spec adds unbound editor copy-path actions and covers discoverability, project-root resolution, fallbacks, and validation coverage.
Concerns
- The
copy_path_with_linecontract does not define line/column numbering or when a column is included, so implementations could produce different clipboard strings for the same cursor state. - The outside-project fallback does not say whether
copy_path_with_linepreserves the line/column suffix when switching from relative to absolute paths. - Command Palette invocation behavior conflicts with the editor-focus requirement because the palette itself moves focus away from the editor; the spec needs to define which editor, if any, is targeted.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| - `editor.copy_absolute_path` — copies the absolute path of the active editor file to the clipboard. | ||
| - `editor.copy_relative_path` — copies the path relative to the project root. | ||
| - `editor.copy_path_with_line` — copies the relative path with `:line[:column]` suffix where `column` is appended only when the cursor has an explicit column position (e.g., `src/main.rs:42:7`; `src/main.rs:42` if no column). |
There was a problem hiding this comment.
column is considered explicit; editor cursors normally have a column, so implementers could emit either :line or :line:column for the same cursor state.
|
|
||
| ### B4. File outside project root | ||
|
|
||
| If the file resolves but is OUTSIDE the project root (e.g., a file opened from `/tmp` while a project is active), `editor.copy_relative_path` and `editor.copy_path_with_line` fall back to the absolute path and surface a toast: "File is outside project root — copied absolute path instead". `editor.copy_absolute_path` is unaffected. |
There was a problem hiding this comment.
editor.copy_path_with_line preserves the :line[:column] suffix when falling back to an absolute path; as written, both /tmp/foo.rs and /tmp/foo.rs:42 satisfy this sentence.
|
|
||
| ### B8. Command Palette discoverability | ||
|
|
||
| All three actions are visible in the Command Palette unconditionally and searchable by "copy path". When invoked from the Command Palette while the editor is not focused, B3's no-op-with-toast behavior applies. |
There was a problem hiding this comment.
|
Round-1 fixes for oz-for-oss review: (1) |
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines three bindable editor actions for copying absolute, relative, and line-qualified paths. The overall shape is clear, but several requirements need tightening before implementation because they conflict or rely on state the editor may not preserve.
Concerns
- Project root resolution is defined from the editor file's parent directory, which conflicts with the outside-active-project fallback behavior.
- The column-inclusion contract depends on cursor placement provenance and has edge cases that can produce contradictory outputs.
- Unicode column numbering says codepoints while also treating combining marks as part of the base character, which needs one model.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| "Project root" is the same root used by the file tree: | ||
|
|
||
| 1. The git toplevel (`git rev-parse --show-toplevel`) of the editor file's parent directory. | ||
| 2. If not in a git repo, the workspace folder containing the file. | ||
| 3. If neither resolves, the action falls back per B4. |
There was a problem hiding this comment.
/tmp file inside its own git repo would get that repo as root instead of being treated as outside the active project. Specify whether relative paths anchor to the active workspace/file-tree root or the opened file's containing repo, then align B4/A3/A8.
| `editor.copy_path_with_line` produces a deterministic clipboard string for a given cursor/selection state. The contract: | ||
|
|
||
| - **Line numbering**: 1-based — the first line is line `1`. This matches universal editor convention (vim, VS Code, IntelliJ). | ||
| - **Column numbering**: 1-based — the first column is column `1`. Columns count CHARACTERS (Unicode codepoints), NOT bytes. `<path>:N:K` always points to the K-th codepoint of line N (so a multi-byte UTF-8 character counts as one column; combining marks count as one column at their base codepoint). |
There was a problem hiding this comment.
| - **When column is included**: the column suffix is emitted ONLY when the cursor has an EXPLICIT column position. Specifically, include `:column` ONLY when the cursor was positioned via: | ||
| - mouse click, | ||
| - arrow-key navigation that establishes a column, | ||
| - or programmatic positioning that supplied a column. | ||
|
|
||
| EXCLUDE the column suffix (emit `<path>:N` only) when: | ||
| - the cursor is at the start of a line (col 1), | ||
| - the cursor is at the end of a line, | ||
| - the cursor was placed via a "go to line N" command without a column. |
There was a problem hiding this comment.
|
Addressed all 3 important concerns from the latest oz-for-oss review:
Commit: f613f77 |
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines three bindable editor actions for copying absolute, relative, and line-qualified file paths, with command palette discoverability, fallback behavior, and detailed cursor/column semantics.
Concerns
- The project-root resolution section is internally ambiguous about whether the workspace folder or git toplevel is the authoritative root. This affects the core relative-path output and should be clarified before implementation.
- The tests do not cover several root-resolution branches that the behavior contract requires, including workspace fallback, no-workspace fallback, and symlink canonicalization.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| Resolution order (the same root the file tree uses): | ||
|
|
||
| 1. **Active workspace folder** — the folder backing the currently active | ||
| Warp workspace (the same folder shown at the top of the file tree). | ||
| This is the canonical "project root". | ||
| 2. **Git toplevel of the workspace folder** — when the workspace folder is | ||
| itself inside a git working tree, run `git rev-parse --show-toplevel` | ||
| on the **workspace folder** (not the editor file's parent directory) | ||
| and use the toplevel as the project root. This widens the root for | ||
| monorepos where the workspace was opened on a sub-directory. |
There was a problem hiding this comment.
| Resolution order (the same root the file tree uses): | |
| 1. **Active workspace folder** — the folder backing the currently active | |
| Warp workspace (the same folder shown at the top of the file tree). | |
| This is the canonical "project root". | |
| 2. **Git toplevel of the workspace folder** — when the workspace folder is | |
| itself inside a git working tree, run `git rev-parse --show-toplevel` | |
| on the **workspace folder** (not the editor file's parent directory) | |
| and use the toplevel as the project root. This widens the root for | |
| monorepos where the workspace was opened on a sub-directory. | |
| Resolution algorithm (the same root the file tree uses): | |
| 1. Start with the **active workspace folder** — the folder backing the currently active | |
| Warp workspace (the same folder shown at the top of the file tree). | |
| 2. If that workspace folder is itself inside a git working tree, run | |
| `git rev-parse --show-toplevel` on the **workspace folder** (not the editor | |
| file's parent directory) and use the toplevel as the project root. This widens | |
| the root for monorepos where the workspace was opened on a sub-directory. | |
| 3. If git toplevel resolution fails, fall back to the active workspace folder. |
| ## Tests | ||
|
|
||
| - T1. `copy_absolute_path` produces the absolute path for the active editor file. | ||
| - T2. `copy_relative_path` produces the path relative to the git toplevel. |
There was a problem hiding this comment.
Two important concerns:
1. Project-root resolution authority. Promoted git toplevel to step 1
and made workspace folder the explicit fallback at step 2. Added
an authoritative-rule paragraph stating: 'project root = git
toplevel containing the active workspace folder when one exists,
otherwise the active workspace folder itself'. Removed earlier
'canonical' wording about the workspace folder. Documented why
git toplevel wins (monorepo sub-package case).
2. Test coverage for root-resolution branches. Added six new tests:
- T_root_git_toplevel_in_monorepo (B2.1 precedence)
- T_root_workspace_fallback_no_git (B2.2 no-repo fallback)
- T_root_workspace_fallback_git_failure (B2.2 git error fallback)
- T_root_no_workspace_fallback (B2.3 no-workspace path)
- T_root_symlink_canonicalization_inside / _outside / _failure
(canonicalization edge cases)
|
Addressed both round-N review concerns. 1. Project-root resolution authority (B2). Rewrote B2 to make git toplevel the authoritative project root and the workspace folder the explicit fallback. Added a single authoritative-rule paragraph: 'project root = git toplevel containing the active workspace folder when one exists, otherwise the active workspace folder itself'. Removed earlier 'canonical' wording about the workspace folder. Documented the rationale (monorepo sub-package case where users open a sub-package as workspace but expect repo-rooted relative paths). 2. Test coverage for root-resolution branches. Added six new tests covering every B2 branch:
|
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines three new bindable editor copy-path actions, including relative path resolution, Command Palette behavior, line/column suffix behavior, acceptance criteria, and tests.
Concerns
- The project-root cache can become stale when git metadata changes without a workspace switch.
- The line/column contract describes columns as grapheme clusters, but cursor columns are insertion points and end-of-line behavior is undefined.
HasExplicitColumnis described inconsistently as per-document and per-cursor state, leaving multi-cursor and selection behavior ambiguous.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| The resolved project root is cached per workspace and invalidated when the | ||
| active workspace changes or the workspace folder moves. |
There was a problem hiding this comment.
git init, .git is deleted or repaired, or a worktree is retargeted, the cached root can produce stale relative paths until the workspace changes; specify watching git/worktree state, a TTL, or resolving on each action.
| `editor.copy_path_with_line` produces a deterministic clipboard string for a given cursor/selection state. The contract: | ||
|
|
||
| - **Line numbering**: 1-based — the first line is line `1`. This matches universal editor convention (vim, VS Code, IntelliJ). | ||
| - **Column numbering**: 1-based — the first column is column `1`. Columns count **Unicode extended grapheme clusters** (per UAX #29 GraphemeBreakProperty), NOT bytes and NOT raw codepoints. `<path>:N:K` always points to the K-th grapheme cluster of line N. Concretely: |
There was a problem hiding this comment.
K as the K-th grapheme cluster, but cursor positions are insertion points; the tests expect cursor-after-first-grapheme to emit column 2, and end-of-line would be len + 1 with no K-th cluster. Define columns as insertion-point indices between grapheme clusters and explicitly cover end-of-line.
| - An emoji ZWJ sequence (e.g., 👨👩👧) is one column. | ||
| - Tabs (`\t`) count as one column. Width-aware tab expansion is NOT performed. | ||
| - This single grapheme-cluster model supersedes any earlier wording that said "codepoints" — the prior phrasing is replaced by this one. | ||
| - **When column is included** — driven by an explicit per-document boolean **`HasExplicitColumn`** that the editor maintains for each cursor position. The clipboard suffix is emitted iff `HasExplicitColumn == true`. The flag transitions are deterministic and source-of-input agnostic; provenance is not tracked at copy time: |
There was a problem hiding this comment.
HasExplicitColumn is described as a per-document boolean while also being maintained for each cursor position, and acceptance later says per-cursor. Specify whether the flag is per cursor/anchor/selection and how it transfers when cursors are added, merged, restored, or selected.
1 participant
Spec for #10290.
Summary
Add three new bindable Command Palette actions for the Warp editor —
editor.copy_absolute_path,editor.copy_relative_path, andeditor.copy_path_with_line— each rebindable in Settings → Keyboard Shortcuts and discoverable in the Command Palette without a binding. Defaults to unbound, matching Warp's pattern for optional new actions. Mirrors VS Code'seditor.action.copyPath/copyRelativeFilePathpattern users already know.Scope
Notes
editor.action.copyPathandcopyRelativeFilePath.ready-to-spec.Test plan
editor.copy_absolute_path/copy_relative_path/copy_path_with_line) align with existingeditor.*namespacecommand_palette.action_invokedis correct