-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
filters don't work correctly when a search path is specified #278
Comments
This is a tough one to fix. First and foremost, the proposed solution can never work. Canonicalizing candidate paths does seem like the obvious solution---and I've been tempted to do it because it simplifies so much---but it has two significant problems:
Here are the key values in this particular case, assuming we've reproduced your test case inside
That last step is where things go bad. In particular, the text that we actually match should be just More generally, relative components like Part of the problem here is that we're canonicalizing the parent directory paths, which is one way of traversing up the file hierarchy to find parent gitignore files. Doing this canonicalization inherently makes the parent directory paths get out of sync with the search paths, which is what causes the last step above to fail. If we found a way to go up the tree without canonicalization, then perhaps that would fix this. Let's try:
The interesting bits come from parent directories at I'm not sure when I'm likely to fix this, but I thought it'd be useful to try to write up the problem and a possible solution. Sorry it took me so long to get to this, but I had a feeling this was going to be a fairly intricate bug, so I stayed away. :-( |
There is one other solution. Notably, this command gives the correct output:
ripgrep could canonicalize (sans symlinks) only the paths given by the user. This still might be costly, but we could limit it only to paths that contain |
@BurntSushi a simple test case that reproduces the problem even without
|
I'd like to help fix this, but need a little help. cross posted from related issue.
|
@tmccombs I wish I could be the oracle you want me to be, but I don't have the context to answer your query. Unfortunately, the quickest way to fix this bug probably requires you to read and understand the code. You'll need to experiment and debug. |
Fixes BurntSushi#829 and BurntSushi#278. This does change the Ignore struct to depend on the working directory at the time of creation, I _think_ this is fine, since Ignore isn't publicly accessible and the Walk structs already depend on the current working directory implicitly. I tried to make a minimal change to fix the issue. An alternative implementation could be to call `current_dir` in the `matched_ignore` method instead of in `add_parents`, but I'm not sure of the performance implications of doing that. Another possible solution would be to change the places that we call `Ignore::matched` to change the path to be relative to the absolute_base of the `Ignore`.
ripgrep seems to ignore unintended files when the base directory is not the current directory.
How to reproduce:
I found the following lines in
ignore/src/dir.rs
don't make sense becausepath
on the right side is a relative path from the current directory, not fromabs_parent_path
.A possible fix to run the example above correctly is the following. However, I'm not sure this fixes other cases.
The text was updated successfully, but these errors were encountered: