Skip to content

Add docs and tests for ignoring .git #1403

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.

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

Merged
merged 4 commits into from
Oct 23, 2023
Merged

Conversation

tmccombs
Copy link
Collaborator

See #1396.

doc/fd.1 Outdated
Comment on lines 46 to 47
is also used. The .git directory is excluded, because that is usually what is wanted if excluding vcs
ignored files, and is more consistent with the output of most git commands.
Copy link
Contributor

Choose a reason for hiding this comment

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

I usually expect the program behavior to be explained in man pages, but not the reasons behind the behavior/choices. I'd say this is not unnecessary.

tests/tests.rs Outdated
Comment on lines 2558 to 2580
/// Test behavior of .git directory with various flags
#[test]
fn test_git_dir() {
let te = TestEnv::new(
&[".git/one"],
&[".git/one/foo.a", ".git/.foo", ".git/a.foo"],
);

te.assert_output(&["--hidden", "foo"], "");
te.assert_output(&["--no-ignore", "foo"], "");
te.assert_output(
&["--hidden", "--no-ignore", "foo"],
".git/one/foo.a
.git/.foo
.git/a.foo",
);
te.assert_output(
&["--hidden", "--no-ignore-vcs", "foo"],
".git/one/foo.a
.git/.foo
.git/a.foo",
);
}
Copy link
Contributor

@marcospb19 marcospb19 Oct 22, 2023

Choose a reason for hiding this comment

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

Could you edit this test (or add a new one) to cover this example (or an equivalent one)?

let te = TestEnv::new(
    &["path/to/another/repository/.git/file1"],
    &["path/to/another/repository/.git/folder/file2"],
    &["a_repository/.git/file1"],
    &["a_repository/.git/folder/file2"],
    ...
);

Because the current tests only covers .git in the current folder.

src/cli.rs Outdated
Comment on lines 67 to 68
/// ignored by '.gitignore' files or the rule to exclude .git/.
/// The flag can be overridden with --ignore-vcs.
Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestion:

    /// Show search results from '.git/' folders and results that would
    /// otherwise be ignored by '.gitignore' rules.

src/cli.rs Outdated
Comment on lines 51 to 52
/// ignored by '.gitignore', '.ignore', '.fdignore', the global ignore file,
/// or the rule to exclude .git/.
Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestion:

Suggested change
/// ignored by '.gitignore', '.ignore', '.fdignore', the global ignore file,
/// or the rule to exclude .git/.
/// ignored by '.gitignore', '.ignore', '.fdignore', the global ignore file,
/// or the default rule that excludes '.git/'.

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you very much

@sharkdp sharkdp merged commit 306dacd into sharkdp:master Oct 23, 2023
@tmccombs tmccombs deleted the git-docs branch October 23, 2023 15:00
tmccombs added a commit to tmccombs/fd that referenced this pull request Apr 28, 2024
tmccombs added a commit to tmccombs/fd that referenced this pull request Apr 29, 2024
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.

3 participants