Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 11 additions & 4 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ mod walk;

use std::env;
use std::io::IsTerminal;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::Arc;

use anyhow::{Context, Result, anyhow, bail};
Expand Down Expand Up @@ -98,7 +98,7 @@ fn run() -> Result<ExitCode> {
.map(|pat| build_pattern_regex(pat, &opts))
.collect::<Result<Vec<String>>>()?;

let config = construct_config(opts, &pattern_regexps)?;
let config = construct_config(opts, &pattern_regexps, &search_paths)?;

ensure_use_hidden_option_for_leading_dot_pattern(&config, &pattern_regexps)?;

Expand Down Expand Up @@ -227,7 +227,11 @@ fn check_path_separator_length(path_separator: Option<&str>) -> Result<()> {
}
}

fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result<Config> {
fn construct_config(
mut opts: Opts,
pattern_regexps: &[String],
search_paths: &[PathBuf],
) -> Result<Config> {
// The search will be case-sensitive if the command line flag is set or
// if any of the patterns has an uppercase character (smart case).
let case_sensitive = !opts.ignore_case
Expand Down Expand Up @@ -295,7 +299,10 @@ fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result<Config
ignore_hidden: !(opts.hidden || opts.rg_alias_ignore()),
read_fdignore: !(opts.no_ignore || opts.rg_alias_ignore()),
read_vcsignore: !(opts.no_ignore || opts.rg_alias_ignore() || opts.no_ignore_vcs),
require_git_to_read_vcsignore: !opts.no_require_git,
require_git_to_read_vcsignore: walk::should_require_git_to_read_vcsignore(
search_paths,
opts.no_require_git,
),
read_parent_ignore: !opts.no_ignore_parent,
read_global_ignore: !(opts.no_ignore
|| opts.rg_alias_ignore()
Expand Down
33 changes: 32 additions & 1 deletion src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,9 +696,27 @@ pub fn scan(paths: &[PathBuf], patterns: Vec<Regex>, config: Config) -> Result<E
WorkerState::new(patterns, config).scan(paths)
}

/// Whether the `ignore` crate should require a `.git` directory to apply gitignore rules.
///
/// Jujutsu repositories use a `.jj` directory and still rely on `.gitignore` files. When a
/// search path is inside such a repository, treat it like `--no-require-git` unless the user
/// explicitly passes `--require-git`.
pub fn should_require_git_to_read_vcsignore(
search_paths: &[PathBuf],
no_require_git: bool,
) -> bool {
if no_require_git {
return false;
}

!search_paths
.iter()
.any(|path| path.ancestors().any(|ancestor| ancestor.join(".jj").is_dir()))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Resolve search paths before scanning ancestors for .jj

path.ancestors() only walks lexical components of the provided search path, so for the common default ./ (or other relative paths) it never reaches parent directories above the current working directory. That means running fd from a subdirectory inside a Jujutsu repo (where .jj is at the repo root) will not be detected here, and .gitignore rules still won’t apply unless the user is at the root or passes an absolute path, which defeats the new feature for many real invocations.

Useful? React with 👍 / 👎.

Comment on lines +708 to +714
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Honor explicit --require-git in Jujutsu repositories

This helper cannot distinguish “default behavior” from an explicit --require-git, because it only receives no_require_git; as soon as .jj is found it always returns false (do not require git). In a Jujutsu repo this makes --require-git ineffective, even though the intended behavior and existing flag semantics rely on explicit override precedence.

Useful? React with 👍 / 👎.

}

#[cfg(test)]
mod tests {
use super::search_str_for_entry;
use super::{search_str_for_entry, should_require_git_to_read_vcsignore};
use std::path::{Path, PathBuf};

#[test]
Expand Down Expand Up @@ -735,4 +753,17 @@ mod tests {
PathBuf::from("bar")
);
}

#[test]
fn should_require_git_in_jujutsu_repo() {
let temp = tempfile::tempdir().unwrap();
let root = temp.path();
std::fs::create_dir(root.join(".jj")).unwrap();

assert!(!should_require_git_to_read_vcsignore(
&[root.join("src")],
false
));
assert!(!should_require_git_to_read_vcsignore(&[root.join("src")], true));
}
}
19 changes: 19 additions & 0 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,25 @@ fn test_custom_ignore_precedence() {
te.assert_output(&["--no-ignore", "foo"], "inner/foo");
}

/// Respect .gitignore in Jujutsu repositories with a `.jj` directory (fixes #1817)
#[test]
fn test_jujutsu_repo_respects_gitignore() {
let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);

fs::remove_dir(te.test_root().join(".git")).unwrap();
fs::create_dir(te.test_root().join(".jj")).unwrap();

te.assert_output(
&["foo"],
"a.foo
one/b.foo
one/two/c.foo
one/two/C.Foo2
one/two/three/d.foo
one/two/three/directory_foo/",
);
}

/// Don't require git to respect gitignore (--no-require-git)
#[test]
fn test_respect_ignore_files() {
Expand Down
Loading