Skip to content

fix: Prune dotfiles rather than ignore, make --prune inject default ignores into prune list#413

Open
effigies wants to merge 9 commits into
bids-standard:mainfrom
effigies:fix/prune-git
Open

fix: Prune dotfiles rather than ignore, make --prune inject default ignores into prune list#413
effigies wants to merge 9 commits into
bids-standard:mainfrom
effigies:fix/prune-git

Conversation

@effigies

@effigies effigies commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Initially narrowly scoped to prune .git, this PR now includes the following changes:

  • Dotfiles and directories are now pruned during walk, rather than ignored after the walk. This saves descent into .git/ or other potentially deep hidden directories (.venv/, for example).
  • --prune now injects the default ignore list into the prune list, which is a more principled way to decide what gets pruned.
  • To preserve the original intended behavior of --prune, derivatives/ is now ignored.
  • To avoid pruning .bidsignore and causing --prune to un-ignore everything, .bidsignore is now ignored through a carve-out in the BIDSFile.ignored property. As a validator feature and not a BIDS feature, this seems reasonable to me.
  • The file tree builders now all construct a default prune list for consistency.

Open issues (probably for separate PRs):

  • Generating the default ignore list from schema. There's a bit of a chicken and egg problem where if the ignore list comes from a newer schema than is declared in dataset_description.json, we're not respecting that, but that's already the case. We already don't load the schema based on dataset_description.json, so as long as we load it from --schema, we're still improving.
  • The file count skips ignored files... With significant confounds like .git now being pruned instead of ignored, I think it may be time to revisit this and count ignored files for a more accurate description of dataset size.

Closes #412.

@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.57143% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.35%. Comparing base (1d21e18) to head (c70a5d6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/main.ts 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #413      +/-   ##
==========================================
- Coverage   87.37%   87.35%   -0.02%     
==========================================
  Files          65       65              
  Lines        4777     4794      +17     
  Branches      781      780       -1     
==========================================
+ Hits         4174     4188      +14     
- Misses        510      513       +3     
  Partials       93       93              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@effigies

effigies commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@yarikoptic Care to give this branch a try?

Comment thread src/main.ts Outdated
Comment thread src/main.ts Outdated
@effigies

effigies commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Well, this turned into a rabbit hole. Will try again later.

@effigies effigies marked this pull request as draft June 3, 2026 17:25
@effigies effigies changed the title fix: Prune git directory rather than just ignoring it fix: Prune dotfiles rather than ignore, make --prune inject default ignores into prune list Jul 2, 2026
@effigies effigies marked this pull request as ready for review July 2, 2026 11:24
@effigies effigies requested review from nellh, rwblair and yarikoptic July 2, 2026 12:30
@effigies

effigies commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

PR description updated. I cast the review net a bit wider, since this is a fairly significant change.

@rwblair rwblair left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm waffling on how transparent we should be with what exactly is pruned when the --prune option isn't used or is false. The defaults are sensible so I don't think we'll surprise any one, but my lack of imagination has tied my shoelaces together before.

Should the contents of ignoreDefaults be more visible in help text or documentation?

Do you think a logger.info readout of pruned files would be useful to anyone?

The nomeclature/semantics of --prune feels strange sometimes, but something like --extended-prune feels worse, so I'm fine with keeping it for now.

Comment thread src/main.ts
if (options.prune) {
// Opaque BIDS directories are ignored by default but still included to allow lookups,
// including size estimates and symlink resolution in git trees.
// Pruning is an extreme measure to reduce memory usage and IO ops and may have unintended consequences.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may need to be added to the help text for the flag.

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.

Talks to git too much?! takes forever on slimmed down clone

3 participants