Fix @source with folders that are ignored#20214
Conversation
more tests WIP tests
No need to check whether we're dealing with an auto source, or whether it's ignored when we know that ahead of time.
If the current `base` path for the `SourceEntry::Auto { base }` is git
ignored, then we promote it to a `SourceEntry::External { base }`
instead, bypassing the default git ignore rules.
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to promoting explicitly-referenced gitignored directories from Auto to External, affecting only @source entries that would previously have been silently excluded. The core fix is well-reasoned: walking ancestors bottom-up, caching gitignore matchers, and using two distinct stop conditions (git-root boundary and cwd boundary for non-git projects) covers the expected scenarios without risking out-of-tree gitignore lookups. The pattern-normalisation refactor in mod.rs is a no-op because optimize() already guarantees a leading /. Tests cover the new path at both unit and integration level. No files require special attention. Reviews (2): Last reviewed commit: "ensure we stop at the current working di..." | Re-trigger Greptile |
|
|
||
| // `Gitignore::new` roots the matcher at the directory | ||
| // containing the file, so patterns match relative to it. | ||
| path.is_file().then(|| Gitignore::new(&path).0) |
There was a problem hiding this comment.
.gitignore parse errors
Gitignore::new returns (Gitignore, Option<Error>) where the second element reports lines that could not be parsed. Discarding it with .0 means a malformed .gitignore will silently produce an incomplete matcher, potentially failing to promote an Auto source to External when it should be. Logging the error at WARN level (or at least DEBUG) would make diagnosing unexpected behaviour much easier without any cost to the happy path.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Hidden review stack artifactWalkthroughThis PR detects when auto-detected source directories are ignored by default rules or applicable .gitignore files and promotes those entries from Auto to External by walking ancestor directories and caching Gitignore matchers. It also adjusts the walker ignore negation pattern and Pattern construction, adds unit and CLI tests for allow-list .gitignore behavior, updates test utilities (type narrowing, --allow-empty commit, stack-trace capture), and adds a changelog entry. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
If we can't find a `.git` folder in any of the parents.
This PR fixes an issue where a
@sourcethat's pointing to a folder that is git ignored, is also ignored by the@sourceeven if it's explicitly added.Internally, we convert
@sourcedirectives fromPublicSourceEntrys toSourceEntrys where we have dedicated enum branches forAuto,Pattern,IgnoredandExternal.The
Autoone accepts abasepath, and will be used for auto content detection. However, these paths will make use of all the default auto content detection rules, which includes git ignore rules.We also have
Externalwhere we link to something that's "external" to the current repo. We can probably improve this name, but it's external in the sense that it won't show up on GitHub for example, aka ignored. We have some content dirs that we ignore by default, such as thenode_modulesfolder. When you do use@sourcewithnode_modulesin the path, then we will mark it as anexternalresource which does not look at thegitignorerelated rules and allowing it to be included this way.The idea with this is that, even though the folder is ignored by default, you can still include files from the folder by explicitly using the
@sourcedirective.The issue as seen in #19844 is using
vendor/instead ofnode_modules/which is not ignored by default. While we can addvendor/to this same ignored dirs list, it will result in a breaking change because this folder is often used by the Laravel community to store some resources in.This PR fixes this problem by not only looking at the content dirs we ignore by default, but also looking at the actual git ignore state of this folder. If it turns out that this is ignored, then we promote the
Autosource to anExternalsource.Fixes: #19844
Closes: #20057
Test plan
@sourcesilently ignores paths excluded by an allow-list.gitignore#19844. If we run the CLI with theDEBUG=*environment variable, the log file produces these results:We're checking some
.gitignorerelated files, so let's check on each OS [ci-all]