Skip excluded directory subtrees during copy#176
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
Walkthroughcopy_directories now detects exclude patterns that target direct children and, when present, copies a directory by enumerating and copying only non-excluded direct children. Post-copy exclusion still runs for deeper patterns; whole-directory fast copy is retained when no subdir-level excludes are detected. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Client
participant CD as copy_directories
participant SEL as _selective_copy_dir
participant FAST as _fast_copy_dir
participant APPLY as _apply_directory_excludes
participant FS as Filesystem
Caller->>CD: request copy of includeDirs (with excludeDirs)
CD->>CD: evaluate excludes for dir_path
alt subdir-level excludes detected
CD->>SEL: perform selective copy for dir_path
SEL->>SEL: enumerate direct children
SEL->>FAST: for each child not excluded -> copy child
FAST->>FS: copy child subtree
SEL->>APPLY: apply deeper excludes under copied children
APPLY->>FS: remove deeper excluded targets if present
else no subdir excludes
CD->>FAST: fast whole-directory copy of dir_path
FAST->>FS: copy entire dir_path
CD->>APPLY: apply excludes post-copy
APPLY->>FS: remove excluded subtrees
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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)
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 the current code and only fix it if needed.
Inline comments:
In `@lib/copy.sh`:
- Around line 356-371: The code in _has_subdir_excludes currently strips
exclude_pattern to its first segment (pattern_prefix) which misses excludes like
exclude_pattern="vendor/bundle/cache" when dir_path="vendor/bundle"; change the
logic so when exclude_pattern contains a slash you directly test whether the
exclude_pattern is under dir_path (e.g. use a case test like case
"$exclude_pattern" in "$dir_path"/*) return 0 ;; esac) instead of comparing only
the first segment; keep the existing .git exclusions and other checks but
replace the pattern_prefix handling with this prefix-with-slash match using the
existing exclude_pattern and dir_path variables.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 636ff3ee-691f-435e-9b8b-aaf4ae64b451
📒 Files selected for processing (3)
CHANGELOG.mdlib/copy.shtests/copy_safety.bats
helizaga
force-pushed
the
codex/skip-excluded-copy-subtrees
branch
from
16abe9e to
f82ddc4
Compare
averyjennings
approved these changes
Apr 28, 2026
Contributor
averyjennings
left a comment
averyjennings
left a comment
There was a problem hiding this comment.
Looks good — no blockers found.
The selective copy path is a clean optimization for the .claude + .claude/worktrees style configs. The follow-up commit f82ddc4 neatly resolved the multi-segment include-path gap by extracting _directory_exclude_suffix and using it for both the new predicate and the pre-existing cleanup, and also fixed a latent dest_parent -> dst_root path-doubling bug for multi-segment includes. New bats coverage is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/copy.sh`:
- Around line 413-415: The check uses child_rel literally so excludes with or
without a trailing slash don't match; update the exclusion logic to normalize
trailing slashes before comparison by passing a slash-stripped child path (or by
normalizing entries inside is_excluded). Specifically, change the call site that
invokes is_excluded (the if is_excluded "$child_rel" "$excludes" check) to use a
normalized child identifier (e.g., strip any trailing slash from child_rel) or
modify is_excluded to treat patterns as equivalent whether they end with a
slash, ensuring ".claude/worktrees" and ".claude/worktrees/" are treated the
same.
- Around line 281-300: The prefix-match branch currently returns on the first
match causing early termination and incorrect suffixes; instead, when the case
"$dir_path" in $prefix) branch matches, capture the current suffix into a
variable (e.g., matched_suffix="$suffix") and continue the loop so the prefix is
refined further; after the loop ends, print the deepest matched_suffix (if set)
and return 0 so _apply_directory_excludes sees the correctly refined suffix (use
variables dir_path, prefix, suffix and the matched_suffix accumulator).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e7fbbb04-504e-4d9e-97a3-7460798fb326
📒 Files selected for processing (3)
CHANGELOG.mdlib/copy.shtests/copy_safety.bats
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/copy_safety.bats
helizaga
force-pushed
the
codex/skip-excluded-copy-subtrees
branch
from
f82ddc4 to
971269c
Compare
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
2 participants
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
includeDirsvendor/bundle/cacheWhy
copy_directoriespreviously cloned an included parent directory first and deleted excluded subdirectories afterward. For configs like:that meant copying large excluded trees before immediately removing them. The new selective path detects excludes beneath an included directory and copies only the direct children that are not excluded.
The exclude matching now also handles include paths with
/, glob-prefix patterns like*/bundle/cache/tmp, and trailing-slash excludes like.claude/worktrees/.Fixes #175.
Validation
git diff --checkshellcheck bin/gtr bin/git-gtr lib/*.sh lib/commands/*.sh adapters/editor/*.sh adapters/ai/*.sh./scripts/generate-completions.sh --checkbats tests/copy_safety.batsbats tests/cmd_copy.batsbats tests/Summary by CodeRabbit
Bug Fixes
Tests
Documentation