Skip to content

fix: resolve editor paths from repo root#347

Merged
benvinegar merged 1 commit into
modem-dev:mainfrom
duarteocarmo:fix/open-editor-paths
May 22, 2026
Merged

fix: resolve editor paths from repo root#347
benvinegar merged 1 commit into
modem-dev:mainfrom
duarteocarmo:fix/open-editor-paths

Conversation

@duarteocarmo
Copy link
Copy Markdown
Contributor

@duarteocarmo duarteocarmo commented May 22, 2026

@benvinegar - there is a bug when launching the editor from a place that is not the repo root.
This PR should fix it.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

Greptile Summary

This PR fixes the e editor shortcut when Hunk is launched from a repo subdirectory by resolving the selected diff file's path against the VCS repo root (sourceLabel) instead of the process working directory.

  • A new exported helper resolveEditableFilePath(filePath, basePath?) wraps node:path's resolve with a process.cwd() default, replacing the inline resolve(process.cwd(), file.path) call in openSelectedFileInEditor.
  • App.tsx derives basePath from bootstrap.changeset.sourceLabel (the repo root) when input.kind is vcs, show, or stash-show, and passes undefined otherwise so the pre-existing cwd fallback is preserved for diff, patch, and difftool inputs.
  • A unit test for the new helper is added to openInEditor.test.ts.

Confidence Score: 5/5

This is a focused, well-scoped bug fix with correct logic and no regressions introduced.

The input-kind guard in App.tsx correctly limits repo-root resolution to the three VCS-backed input kinds where sourceLabel is always an absolute repo root path; the cwd fallback is preserved for all other input types. The resolve call in the helper handles already-absolute paths safely. No edge cases or data-flow issues were found.

No files require special attention.

Important Files Changed

Filename Overview
src/ui/lib/openInEditor.ts Extracts path resolution into a new exported helper resolveEditableFilePath that accepts an optional basePath (defaulting to process.cwd()), and threads that parameter through openSelectedFileInEditor; logic is correct and the fallback preserves prior behaviour for non-VCS inputs.
src/ui/App.tsx Computes basePath from bootstrap.changeset.sourceLabel (the repo root) when the input kind is vcs, show, or stash-show, and correctly passes undefined for the remaining kinds (diff, patch, difftool) so the cwd fallback is used; both new deps are added to the useCallback array.
src/ui/lib/openInEditor.test.ts Adds a test for resolveEditableFilePath; the test verifies the correct argument order and that basePath is honoured, though it only covers the explicit-path happy path.
CHANGELOG.md User-visible changelog entry added for the subdirectory editor-shortcut fix.

Sequence Diagram

sequenceDiagram
    participant App as App.tsx
    participant OE as openInEditor.ts
    participant FS as filesystem

    App->>App: User presses 'e'
    App->>App: Determine basePath
    note over App: sourceLabel if vcs/show/stash-show, else undefined
    App->>OE: "openSelectedFileInEditor({ basePath, file, … })"
    OE->>OE: resolveEditableFilePath(file.path, basePath)
    note over OE: resolve(basePath ?? cwd, file.path)
    OE->>FS: existsSync(absolutePath)
    alt file exists
        OE->>OE: buildEditorCommand(editor, absolutePath, line)
        OE->>OE: Bun.spawnSync([editor, …args])
    else file not found
        OE-->>App: Cannot edit …: file does not exist on disk.
    end
Loading

Reviews (1): Last reviewed commit: "fix: resolve editor paths from repo root" | Re-trigger Greptile

@duarteocarmo duarteocarmo force-pushed the fix/open-editor-paths branch from a38640f to bf35853 Compare May 22, 2026 11:57
@benvinegar
Copy link
Copy Markdown
Member

Makes sense, thanks for the follow up!

@benvinegar benvinegar merged commit 290f596 into modem-dev:main May 22, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants