Skip to content

add support for nested gitignores #2751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
luuumine wants to merge 3 commits into from
Closed

add support for nested gitignores #2751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
af1a1c2
feat(tests): add tests about nested gitignore
luuumine Aug 26, 2026
6958a48
feat(walk): nested gitignore support
luuumine Aug 26, 2026
43723b9
refactor(tests): merge nested_gitignore tests
luuumine Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions bin/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ impl Check {
} else {
let all_ignores = [self.ignore.as_slice(), extra_ignores].concat();
let ignore = dirs::build_ignore_set(&all_ignores, &self.target, self.unrestricted)?;
let files = dirs::walk_nix_files(ignore, &self.target)?;
let files =
dirs::walk_nix_files(ignore, &self.target, &all_ignores, self.unrestricted)?;
Ok(vfs(&files.collect::<Vec<_>>()))
}
}
Expand Down Expand Up @@ -130,7 +131,8 @@ impl Fix {
} else {
let all_ignores = [self.ignore.as_slice(), extra_ignores].concat();
let ignore = dirs::build_ignore_set(&all_ignores, &self.target, self.unrestricted)?;
let files = dirs::walk_nix_files(ignore, &self.target)?;
let files =
dirs::walk_nix_files(ignore, &self.target, &all_ignores, self.unrestricted)?;
Ok(vfs(&files.collect::<Vec<_>>()))
}
}
Expand Down
116 changes: 88 additions & 28 deletions bin/src/dirs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,61 +13,118 @@ use ignore::{

#[derive(Debug)]
pub struct Walker {
dirs: Vec<PathBuf>,
dirs: Vec<(PathBuf, Vec<PathBuf>, Gitignore)>,
files: Vec<PathBuf>,
ignore: Gitignore,
extra_ignores: Vec<String>,
unrestricted: bool,
}

impl Walker {
pub fn new<P: AsRef<Path>>(target: P, ignore: Gitignore) -> io::Result<Self> {
pub fn new<P: AsRef<Path>>(
target: P,
ignore: Gitignore,
extra_ignores: Vec<String>,
unrestricted: bool,
) -> io::Result<Self> {
let target = target.as_ref().to_path_buf();
if !target.exists() {
Err(Error::new(
ErrorKind::NotFound,
format!("file not found: {}", target.display()),
))
} else if target.is_dir() {
let root_gitignore = target.join(".gitignore");
let gitignore_files = if !unrestricted && root_gitignore.exists() {
vec![root_gitignore]
} else {
vec![]
};
Ok(Self {
dirs: vec![target],
dirs: vec![(target, gitignore_files, ignore)],
files: vec![],
ignore,
extra_ignores,
unrestricted,
})
} else {
Ok(Self {
dirs: vec![],
files: vec![target],
ignore,
extra_ignores,
unrestricted,
})
}
}

fn build_ignore_for(
&self,
base: &Path,
gitignore_files: &[PathBuf],
) -> Result<Gitignore, IgnoreError> {
let mut builder = GitignoreBuilder::new(base);

if !self.unrestricted {
for gitignore in gitignore_files {
builder.add(gitignore);
}

builder.add_line(None, ".git")?;
}
for ignore in &self.extra_ignores {
builder.add_line(None, ignore.as_str())?;
}
builder.build()
}
}

impl Iterator for Walker {
type Item = PathBuf;
fn next(&mut self) -> Option<Self::Item> {
self.files.pop().or_else(|| {
while let Some(dir) = self.dirs.pop() {
if dir.is_dir()
&& let Match::None | Match::Whitelist(_) = self.ignore.matched(&dir, true)
{
let mut found = false;
for entry in fs::read_dir(&dir).ok()? {
let entry = entry.ok()?;
let path = entry.path();
if path.is_dir() {
self.dirs.push(path);
} else if path.is_file()
&& let Match::None | Match::Whitelist(_) =
self.ignore.matched(&path, false)
{
found = true;
self.files.push(path);
}
}
if found {
break;
while let Some((dir, mut gitignore_files, mut ignore)) = self.dirs.pop() {
if !dir.is_dir() {
continue;
}
let nested = dir.join(".gitignore");

if !self.unrestricted && nested.exists() && !gitignore_files.contains(&nested) {
gitignore_files.push(nested);

ignore = match self.build_ignore_for(&dir, &gitignore_files) {
Ok(ignore) => ignore,
Err(_) => continue,
};
}

if !matches!(
ignore.matched(&dir, true),
Match::None | Match::Whitelist(_)
) {
continue;
}

let mut found = false;

for entry in fs::read_dir(&dir).ok()? {
let entry = entry.ok()?;
let path = entry.path();

if path.is_dir() {
self.dirs
.push((path, gitignore_files.clone(), ignore.clone()));
} else if path.is_file()
&& matches!(
ignore.matched(&path, false),
Match::None | Match::Whitelist(_)
)
{
found = true;
self.files.push(path);
}
}

if found {
break;
}
}
self.files.pop()
})
Expand Down Expand Up @@ -106,7 +163,10 @@ pub fn build_ignore_set<P: AsRef<Path>>(
pub fn walk_nix_files<P: AsRef<Path>>(
ignore: Gitignore,
target: P,
extra_ignores: &[String],

@mightyiam mightyiam Aug 26, 2026

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.

Looking at this... I'm not sure why there would be distinct ignore and extra_ignores as opposed to a single ignores. Thoughts?

@luuumine luuumine Aug 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i guess we could probably refactor that another way to avoid having a "repeatition" but my idea was the following:

  • we need a Gitignore object (ignore element) at every step to know if the walker needs to traverse folders and files and everything
  • but we need to keep the full list of ignored stuff (extra_ignores element) to rebuild a new ignore object when encountering a new .gitignore file in a nested subfolder

so i think we can't do without both? i may be wrong there but i don't see how to avoid this issue

@mightyiam mightyiam Aug 28, 2026

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 discussion brings me back to thoughts I had before. Why do we have implementation of such common file traversal with gitignore logic in this project?

@luuumine luuumine Aug 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

what do you mean? surely we want to not check gitignored files, to avoid raising errors on files that are not relevant to the user

@mightyiam mightyiam Aug 28, 2026

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.

What I mean is, isn't there a crate that "just does this" exact behavior?

@luuumine luuumine Aug 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i didn't think about that earlier but i just saw that the regular ignore crate did..
I think we can just loop over their Walk iterator directly
https://docs.rs/ignore/latest/ignore/struct.Walk.html

This example shows the most basic usage of this crate. This code will recursively traverse the current directory while automatically filtering out files and directories according to ignore globs found in files like .ignore and .gitignore:

use ignore::Walk;

for result in Walk::new("./") {
    // Each item yielded by the iterator is either a directory entry or an
    // error, so either print the path or the error.
    match result {
        Ok(entry) => println!("{}", entry.path().display()),
        Err(err) => println!("ERROR: {}", err),
    }
}

@mightyiam mightyiam Aug 28, 2026

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.

Would you mind doing that, instead? Our test coverage should provide some confidence, right?

@luuumine luuumine Aug 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure, I'll try to look into it this weekend.
should I make a new PR?

@mightyiam mightyiam Aug 28, 2026

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.

Great! Whatever you find most convenient.

unrestricted: bool,
) -> Result<impl Iterator<Item = PathBuf>, io::Error> {
let walker = dirs::Walker::new(target, ignore)?;
Ok(walker.filter(|path: &PathBuf| matches!(path.extension(), Some(e) if e == "nix")))
let walker = dirs::Walker::new(target, ignore, extra_ignores.to_vec(), unrestricted)?;
Ok(walker
.filter(|path: &PathBuf| matches!(path.extension(), Some(extension) if extension == "nix")))
}
19 changes: 19 additions & 0 deletions bin/tests/walk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,25 @@ mod gitignored_files {

assert_eq!(report.paths, ["./linted.nix"]);
}

#[test]
fn multiple_gitignores() {
let report = Fixture::with_files(&[
("file.nix", CODE_THAT_TRIGGERS_A_LINT),
("a/.gitignore", "file.nix\nbuild/\n"),
("a/file.nix", CODE_THAT_TRIGGERS_A_LINT),
("a/build/inside.nix", CODE_THAT_TRIGGERS_A_LINT),
("b/file.nix", CODE_THAT_TRIGGERS_A_LINT),
("b/build/inside.nix", CODE_THAT_TRIGGERS_A_LINT),
])
.run_with_args(&[])
.unwrap();

assert_eq!(
report.paths,
["./file.nix", "./b/file.nix", "./b/build/inside.nix"]
);
}
}

mod unrestricted {
Expand Down