feat: local directory interactive mode by nrjdalal · Pull Request #59 · nrjdalal/gitpick

nrjdalal · 2026-03-22T11:37:02Z

Summary

gitpick -i browses the current directory interactively
gitpick -i target browses cwd, copies selected files to target
gitpick ./path -i target browses a specific path
Uses git ls-files to respect .gitignore when in a git repo
Falls back to manual walk (skipping .git only) outside git repos
Detects and errors on target-inside-source and same source/target
Gracefully skips unreadable directories and files
Full file preview with syntax highlighting works on local files

Test plan

gitpick -i - browse cwd
gitpick -i output - browse cwd, copy to output
gitpick . -i --dry-run - preview only
gitpick ./src -i dest - browse subfolder
gitpick -i . - should error (target inside source)
gitpick -i outside a git repo - falls back to manual walk
Verify 132 existing tests pass

🤖 Generated with Claude Code

Browse local directories with -i flag: - gitpick -i (browse cwd) - gitpick -i target (browse cwd, copy to target) - gitpick ./path -i target (browse path, copy to target) - gitpick . -i --dry-run (preview only) Skips node_modules and .git, reuses same picker UI with file preview and syntax highlighting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Detect local paths with dots (e.g. hello.txt) when -i is set - Skip common heavy dirs: .next, dist, build, .cache, coverage, etc. - Gracefully skip unreadable directories and files - Error when target is inside source directory - Better isLocalPath detection (allow dots in names) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Uses git ls-files --cached --others --exclude-standard to list files respecting .gitignore. Falls back to manual walk (skipping .git only) when not in a git repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nrjdalal

Overall PR score: 4.5/10.

Correctness: 4/10
UX/API design: 6/10
Test coverage: 3/10

The feature direction is strong, but I found two blocking correctness issues: the missing-source fallback can reinterpret the first positional as the target and ignore the explicit target, and individually selected symlinks are dereferenced instead of preserved. I also didn't find new tests covering the local interactive parsing/copy semantics.

nrjdalal · 2026-03-22T11:57:01Z

bin/index.ts

+    if (isLocalPath && options.interactive) {
+      // If url doesn't look like a real path, it's the target (e.g. `gitpick -i hello`)
+      if (
+        !fs.existsSync(path.resolve(url.startsWith("~/") ? url.replace("~", os.homedir()) : url))


Score: 4/10 for correctness in this area. This fallback silently rewrites a nonexistent first positional into target = url; url = ".". With two args, a typo like gitpick -i mising-dir out stops meaning "source + target" and instead browses the current working directory, writes into mising-dir, and ignores out. This should only be allowed for the single-positional shorthand; when both source and target are present, a missing source should error.

nrjdalal · 2026-03-22T11:57:01Z

bin/index.ts

+      for (const sel of selected) {
+        const src = path.join(resolvedSource, sel)
+        const dest = path.join(targetDir, sel)
+        const stat = await fs.promises.stat(src).catch(() => null)


Score: 5/10 for filesystem fidelity in this area. This copy path uses stat() and then copyFile() for non-directories, which follows symlinks. So selecting a symlink by itself copies the linked file contents instead of recreating the symlink. That is inconsistent with directory selection, where copyDir() preserves symlinks. Using lstat() here and handling symlinks explicitly would keep the local interactive flow faithful to the source tree.

…in copy - gitpick -i missing-dir out now errors instead of silently rewriting - Individual symlink selection preserves the symlink instead of dereferencing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nrjdalal

Updated review for commit 5e59e36.

Overall PR score: 7.0/10.

Correctness: 7/10
UX/API design: 8/10
Test coverage: 5/10

The two earlier blockers look fixed in this revision: missing explicit sources now error correctly, and individually selected symlinks are no longer dereferenced. I only found one remaining issue in the new local copy path: symlink creation failures are swallowed and still counted as copied, which can hide stale output in overwrite cases. Full CLI suite is currently green locally (bun test tests/cli.test.ts).

nrjdalal · 2026-03-22T12:02:49Z

bin/index.ts

+        await fs.promises.mkdir(path.dirname(dest), { recursive: true })
+        if (lstat.isSymbolicLink()) {
+          const linkTarget = await fs.promises.readlink(src)
+          await fs.promises.symlink(linkTarget, dest).catch(() => {})


Updated score for this area: 6/10. Preserving symlinks is the right fix, but swallowing symlink() errors here makes the result look successful even when the destination was not updated. In overwrite scenarios an existing destination entry will cause EEXIST, this branch will still increment copiedFiles, and the command can report success while leaving stale output behind. It would be safer to either remove/replace the destination when overwrite is enabled or let the error surface instead of ignoring it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nrjdalal

Updated review for commit bb103bd.

Overall PR score: 8.0/10.

Correctness: 8/10
UX/API design: 8/10
Test coverage: 6/10

The previous correctness blockers look addressed in this revision, and the full CLI suite is green locally (bun test tests/cli.test.ts). I only found one residual issue in the local interactive symlink branch: failures are still swallowed, so the command can silently produce a partial result instead of surfacing the copy error.

Findings:

[P2] Local interactive symlink copy still suppresses failures and can leave partial output without warning.

nrjdalal · 2026-03-22T12:06:58Z

bin/index.ts

+            await fs.promises.rm(dest, { force: true })
+            await fs.promises.symlink(linkTarget, dest)
+            copiedFiles++
+          } catch {}


[P2] Updated score for this area: 7/10. This is better than the previous version because it no longer counts failed symlink copies as successful, but the empty catch {} still suppresses the error entirely. If rm() or symlink() fails here, the command continues and can report an apparently successful copy with some selected entries missing. Returning a clear error would make partial-copy failures much easier to detect and debug.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nrjdalal

Updated review for commit 1852d29.

Overall PR score: 9.0/10.

Correctness: 9/10
UX/API design: 9/10
Test coverage: 7/10

I don't have any new blocking findings on this revision. The earlier symlink-copy issue is in a much better place now that failures are surfaced as warnings instead of being swallowed silently, and the full CLI suite is green locally (bun test tests/cli.test.ts).

Residual risk: I still don't see dedicated tests for the new local interactive path itself, so most of the confidence here is coming from code review plus the broader existing suite rather than direct coverage of the new branch.

nrjdalal and others added 3 commits March 22, 2026 07:47

nrjdalal commented Mar 22, 2026

View reviewed changes

fix: remove existing dest before symlink creation, don't count failures

bb103bd

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nrjdalal commented Mar 22, 2026

View reviewed changes

fix: warn on symlink copy failure instead of swallowing silently

1852d29

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nrjdalal commented Mar 22, 2026

View reviewed changes

nrjdalal merged commit 78e63dd into main Mar 22, 2026
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: local directory interactive mode#59

feat: local directory interactive mode#59
nrjdalal merged 6 commits intomainfrom
feat/local-interactive

nrjdalal commented Mar 22, 2026

Uh oh!

nrjdalal left a comment

Uh oh!

nrjdalal Mar 22, 2026

Uh oh!

nrjdalal Mar 22, 2026

Uh oh!

nrjdalal left a comment

Uh oh!

nrjdalal Mar 22, 2026

Uh oh!

nrjdalal left a comment

Uh oh!

nrjdalal Mar 22, 2026

Uh oh!

nrjdalal left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

nrjdalal commented Mar 22, 2026

Summary

Test plan

Uh oh!

nrjdalal left a comment

Choose a reason for hiding this comment

Uh oh!

nrjdalal Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

nrjdalal Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

nrjdalal left a comment

Choose a reason for hiding this comment

Uh oh!

nrjdalal Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

nrjdalal left a comment

Choose a reason for hiding this comment

Uh oh!

nrjdalal Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

nrjdalal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant