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

feat: match ignores against directly invoked paths #13

Closed
wants to merge 22 commits into from

Conversation

seren5240
Copy link
Contributor

for https://github.com/getgrit/rewriter/issues/9042

Unfortunately, there isn't a way to do this out of the box with ignore's interface. There was a crate exposing closer functionality but it's been archived.

@seren5240 seren5240 requested a review from a team March 13, 2024 11:48
crates/language/src/target_language.rs Outdated Show resolved Hide resolved
crates/language/src/target_language.rs Outdated Show resolved Hide resolved
@seren5240 seren5240 requested a review from morgante March 13, 2024 18:39
@morgante
Copy link
Contributor

morgante commented Mar 14, 2024

Thanks for adding the memo-ization, and sorry for the false start, but I'm still a bit hesitant about the additional complexity in this PR. We are re-implementing key parts of ignore (directory traversal upwards / searching for gitignore files) to work around what feels like a bug in ignore.

Doing a bit more research:

Overall I think we are best taking some other approaches:

  1. Simply ignore this issue. The original bug I reported (https://github.com/getgrit/rewriter/issues/9042) was not this, and it doesn't seem unreasonable to always analyze directly invoked paths.
  2. Try fixing the problem upstream, it looks like there might be some fruitful code on the branch of the linked issue. Looks like the problems might be around https://github.com/BurntSushi/ripgrep/blob/master/crates/ignore/src/walk.rs#L1027-L1031 and the short-circuiting for depth 0.
  3. Reuse the Walker's gitignore that it built up instead of making a parallel implementation. https://github.com/JohnnyMorganz/StyLua/pull/765/files looks closer to this.

@morgante morgante closed this Mar 14, 2024
@seren5240 seren5240 deleted the ignore-direct branch March 18, 2024 10:29
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.

2 participants