Skip to content
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

file-search: fix not ignoring .git #8721

Merged
merged 1 commit into from
Nov 11, 2020
Merged

Conversation

paul-marechal
Copy link
Member

@paul-marechal paul-marechal commented Nov 5, 2020

What it does

  • Fix issue where ripgrep would look into .git when using
    .gitignore.
  • Use files.exclude.

Signed-off-by: Paul Maréchal [email protected]

How to test

Tests should fail if you comment the line args.push('--glob', '!.git'); from the file file-search-service-impl.ts.

Review checklist

Reminder for reviewers

@paul-marechal paul-marechal force-pushed the mp/file-search-ignore branch 2 times, most recently from 2244ce8 to cc1284c Compare November 5, 2020 22:26
@vince-fugnitto vince-fugnitto added bug bugs found in the application file search issues related to the file search file-watchers issues related to filesystem watchers - nsfw quality issues related to code and application quality and removed file-watchers issues related to filesystem watchers - nsfw labels Nov 5, 2020
@paul-marechal paul-marechal force-pushed the mp/file-search-ignore branch 9 times, most recently from c8df4d5 to a5deff4 Compare November 6, 2020 22:28
- Fix issue where ripgrep would look into `.git` when using
  `.gitignore`.
- Use `files.exclude`.

Signed-off-by: Paul Maréchal <[email protected]>
@paul-marechal paul-marechal changed the title file-search: remove dependency and fix gitignore file-search: fix not ignoring .git Nov 7, 2020
@paul-marechal
Copy link
Member Author

This issue apparently only occurred during tests, but it was failing on my side. Hence why I fixed the logic rather than just changing my environment.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good and work well 👍
I confirmed the following:

  • the unit-test fails if the line is commented out/removed.
  • the functionality still works correctly, searching for files work, and searching ignored files also work when requested.
  • the only difference in behavior I could identify is that vscode respects the files.exclude preference while we always exclude .git.

@paul-marechal paul-marechal merged commit 4f763a9 into master Nov 11, 2020
@paul-marechal paul-marechal deleted the mp/file-search-ignore branch November 11, 2020 19:26
@github-actions github-actions bot added this to the 1.8.0 milestone Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application file search issues related to the file search quality issues related to code and application quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants