-
-
Notifications
You must be signed in to change notification settings - Fork 139
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(fuzzy finder): use !
to search ignored files
#856
base: master
Are you sure you want to change the base?
Conversation
Hmm, CI failed for 2 packages, one of which isn't related to these changes? Tests are passing locally for me on a mac, and the feature seems to be working as intended when I play with it. I'll try to spin up a linux VM soon to see if it's a platform difference. (I think the CI tests are only run on linux.) |
I am getting similar failures on macOS as I'm seeing in CI. Just in case this explains it: if you open a project window on the
and then invoke Run Package Specs from there. (Of course, you've added specs, so that's not likely to be the problem.) |
I didn't know (or had forgotten about) Run Package Specs. Thanks! So ... Weird. Or interesting? If I run specs from the cli, I see:
If I open the package dir and use Run Package Specs, I see:
So, none of these are failing in the same way as in CI. Fun! I still haven't tried this out in a VM though. |
In a few days, once there's less on my plate, we can attempt to figure out the discrepancy. |
8554ff6
to
a68d3ce
Compare
I've only peeked at the failures in CI, but there are two things to note:
I'll try to scratch at this more soon, to see if I can uncover anything. |
Usage of CompositeDisposable was removed when this package was imported into Pulsar. Ref: 90566ed
This appears to have been unused for quite some time, since before this was imported into Pulsar.
Most of the metrics were already gone, so this just cleans up the remnants. Ref: pulsar-edit/fuzzy-finder#4 Co-Authored-By: @sertonix
There are a lot of parameters being passed around, to lets just bundle them into an options param and use destructuring to reduce some of the code noise.
3ed3574
to
4575137
Compare
Requirements
Description of the Change
This PR includes a few changes to the bundled fuzzy finder package:
This PR adds the ability to search for git/vcs ignored files by prefixing the file finder query with an exclamation point (
!
). This feature has been a long requested feature in Atom (see #148, dating back to 2015) that has been described by maintainers as something that they would not implement but would welcome a PR for. Well, here it is. 😄 I have been using a version of this feature since as far back as 2017, but is has seen several updates:Alternate Designs
I chose
!
because of it's connotation with negation and this feature feels like you're inverting what you're searching for. Other characters would work just as easily. One possibly drawback of using!
is that it could conflict with a possible future feature that might try to implement a "show files that don't match this query" feature, but I haven't seen that requested, nor have I personally ever felt the need for such a thing.As I recall, I did not add support for Teletype in this PR, and I don't know that that matters anymore now anyway (see #536). I didn't add this because it seemed like an edge case that might not be worth it; it should still search your local ignored items in Teletype sessions, though. (Again, I have not tested this, but that part of the code was pretty small and straightforward.)
The main thing I don't like about this implementation is how the ripgrep loader depends on the order of indexing to determine if a file is tracked vs ignored. This is because the
ripgrep
crawler doesn't instantiate a git repo (soisIgnored(file)
won't work), andripgrep
doesn't provide a way to search only in ignored files**. I felt that the only way to continue using a similar-ish technique to what already exists in this package was to first index all tracked files, then (if the user has setindexIgnoredPaths
totrue
) go through and immediately reindex all files, tracked and otherwise. We use the existing state from the first crawl (of tracked files) to basically say that any files not in that set must be ignored files. This is not the case for the non-ripgrep loader; that one should only crawl files 1x.If this is not acceptable, another possibility might be to have
rg
dump the list of files it would search into a temporary file, then feed that back intorg
as a "pattern file" while asking it print non-matching lines. This is trivial from command line, but would involve more Node/fs/process plumbing and it's not clear to me if writing to temp files for things like this is frowned up or not. Open to suggestions on this.Here's an example of doing this on the command line:
**
ripgrep
searches in tracked files by default, and the--no-ignores
flag lets you search all files. There is no way to list/search only ignored files with ripgrep. (See BurntSushi/ripgrep#2029)Benefits
My main use case is exactly that mentioned in atom/fuzzy-finder#148 (comment): source diving. I use Laravel and I have the
vendor/
directory ignored so that find-and-replace and fuzzy finding are limited just to my app files and do not include package dependencies. I often find myself source diving into the framework files, though, and this feature has been incredibly helpful for that.Another example (also Laravel): we have a
.env.example
file that is a generic environment config file and is tracked, but we keep our real/dev values in a.env
file that is not tracked. When ignoring VCS files,.env
never comes up (even though we need to edit pretty regularly) and I have to go hunting for it with my mouse. With this feature,!env
pulls it right up.Possible Drawbacks
Speed is an obvious one, both in indexing and searching through the index, because enabling this feature will then index all of your
vendor/
,node_modules/
, etc directories, and these can been quite large. In practice, though, I've been indexing projects with 30k-60k total files in them and everything has been pretty sprightly, even the older 2011 MBP that I originally used while implementing this.I did not set the order of the config settings, so the "index ignored files" ends up before the "useRipGrep" option in the settings.
The interaction with the
core.excludeVcsIgnoredPaths
setting is a little tricky. For example, the behavior of this feature whenexcludeVcsIgnoredPaths
is true makes sense, but whenexcludeVcsIgnoredPaths
is false (ie ignored paths should be included in normal results), then what should this feature do for!
queries? As coded, it shows an empty list, as if nothing matched. Open to suggestions on this.This also gets to another trick w/ this: there are a lot of odd combos of settings and states that affect how the fuzzy matcher works: use
ripgrep
or not, is project a git repo or no, isexcludeVcsIgnoredPaths
true of false, isindexIgnoredFiles
true or false, etc. I tried to consider them all, but if you see where I got one of these combinations wrong, please let me know!The only breaking change that I can think of is for users that are already in the habit of starting their queries with exclamation points as a way to search for tracked files with exclamation marks in their name. If these users set
indexIgnoredPaths
totrue
, then these queries will stop working. A workaround is to begin their search with a space, then an exclamation point. This feature is only triggered if the first character of the non-trimmed query is an exclamation point. If the query starts w/ a space, then this feature is bypassed entirely.Applicable Issues
atom/atom#124
atom/atom#148
atom/atom#214