search

atom / scandal

Efficient directory scan + search utilities
http://atom.github.io/scandal
MIT License
47 stars 29 forks source link

Fix excludeVcsIgnore when working in subdirs #40

Closed artgillespie closed 7 years ago

artgillespie commented 8 years ago

Since I switched to atom a few weeks ago, I've found that our project's node_modules and other .gitignored paths are always searched, despite having excludeVcsIgnoredPaths set to true in my config. I have to remember to add !node_modules, !dist, ..., etc. to the file directory pattern.

I dug into this and found that if your atom project is rooted in a directory other than the root of your git working directory (as is the case when you're working on a single project in a monorepo like ours) PathFilters isPathIsExcludedByGit will always return false. (because, in fact, it doesn't recognize the relativized path passed in by PathScanner

See related issue in atom core: https://github.com/atom/atom/issues/9175

maxbrunsfeld commented 8 years ago

@artgillespie Thanks so much for figuring this out. I'd like to get a test on this behavior before merging, if possible. Are you interested in trying to add a test? Let me know if I can help with it.

artgillespie commented 8 years ago

@maxbrunsfeld Happy to. I punted on adding a test because I didn't understand how best to add to the git fixture—if you have any pointers, that'd be great. Either way, I'll dig in later today.

artgillespie commented 7 years ago

@maxbrunsfeld Updated the git fixture and added a test.

maxbrunsfeld commented 7 years ago

✨ Really nice work @artgillespie. This will go out in Atom 1.14.

maxbrunsfeld commented 7 years ago

Hey @artgillespie, I upgraded this in Atom, but then downgraded it because it caused the find-and-replace test to fail. Would you be interested in looking into those test failures? The easiest way is:

  1. Run npm link from the scandal directory
  2. Clone the atom repo to ~/github/atom
  3. run npm link scandal from the atom directory
  4. Clone the find-and-replace repository
  5. Open find-and-replace in Atom and use the window:run-package-specs command.
lazyatom commented 7 years ago

I've been trying to figure out how to get atom to play nicely with bundle open (see atom/find-and-replace#910) when the bundle directory is ignored and within the same directory as the repo, and it looks like my hopes are in conflict with the fix provided here.

Just to be clear, a simple example:

When I run atom /home/user/project/vendor/bundle/dependency-0.0.1, I hope to be able to search it independently of the fact that it technically lives within the /home/user/project repository.

I believe I understand the mono-repo case, and I'm sympathetic to it, but it seems diametrically opposed to what I'm hoping for. Is there any interest in exploring a way to support both use cases?