Doesn't respect "match-beginning" gitignore patterns in subdirs #285

purcell · 2013-10-28T15:06:26Z

In .gitignore files, ignore patterns can be prefixed with a "/". man gitignore says:

A leading slash matches the beginning of the pathname. For example, "/*.c" matches "cat-file.c"
but not "mozilla-sha1/sha1.c".

ag currently doesn't apply such patterns when they match files in subdirectories. Steps to reproduce:

% mkdir ag-test                                                                                            % cd ag-test                                                                                               % mkdir ignored
% echo 'hello' > ignored/a
% echo 'hello' > b
% ag hello
b
1:hello

ignored/a
1:hello
% echo '/ignored/*' > .gitignore  ## should ignore "a", but doesn't
% ag hello
b
1:hello

ignored/a
1:hello
% echo 'ignored/*' > .gitignore
% ag hello
b
1:hello

However, the non-subdir case currently works correctly:

% echo '/b' > .gitignore
% ag hello
ignored/a
1:hello

Other than this minor hiccup, it's awesomeness all the way. :-)

The text was updated successfully, but these errors were encountered:

mcphail · 2013-10-28T16:06:50Z

What filesystem are you using to test this? Is this affected by a similar problem to issue #275?

purcell · 2013-10-28T16:18:00Z

This is on OS X, so HFS+.

While initially attempting to create "steps to reproduce" as above, I certainly noticed something strange, whereby the presence of one file seemed to affect the "searchability" of another, but I unfortunately neglected to note my actions or the exact details. That behaviour does sound related to #275.

mcphail · 2013-10-28T16:29:36Z

Can you try pull request #284 to see if it helps at all?

purcell · 2013-10-28T16:31:47Z

Yep, I can hopefully give that a try later today.

Fix ggreer#285 but needs testing for more complex directories

purcell · 2013-10-28T20:48:36Z

Bah, having a non-trivial time getting this to compile here on my Mac. Do you happen to know the magic configure incantation to include the homebrew libs & headers?

mcphail · 2013-10-28T21:52:34Z

I don't have OSX so I'm not sure I can help. I'd have thought pkg-config would have taken the strain. Have you installed the dependencies from the build section of the README.md and run ./build.sh? Beyond that, ./configure can accept variables like ./configure LDFLAGS='-Lwhatever_lib_dir' LIBS='-lwhatever_lib' but really pkg-config should be doing that for you automatically.

purcell · 2013-10-28T22:06:14Z

Thanks. No, pkg-config isn't making everything work magically, even though it's the one from homebrew, and all the deps are installed there. I'll have a further tinker with it in the morning: I'd like to be able to give some helpful feedback.

purcell · 2013-10-29T09:42:48Z

Okay, got it built -- sorry for the noise. So PR #284 doesn't fix this specific problem: the ignored/a file doesn't get ignored. :-(

Fix ggreer#285 but needs testing for more complex directories

mcphail · 2013-10-29T20:31:01Z

Hmm. That's odd. #284 works on Linux on both hfsplus and ntfs filesystems here.

I'm sure this patch is getting close to the nub of the problem. I can't quite get my head around the recursion and appropriate manipulation of scandir_baton->level and ig->parent to do the right thing. I'll keep playing with files until it breaks again so I can debug it further.

Fix ggreer#285 but needs testing for more complex directories

mcphail · 2013-10-30T18:44:59Z

Can you try the current version of #284 and see if it fixes your problem?

purcell · 2013-10-30T19:08:37Z

I updated to the latest version of the PR (7f4f4d1) and nope, it still doesn't ignore ignored/a in the scenario above.

mcphail · 2013-10-30T19:17:53Z

Back to the drawing board! To be honest, I'm struggling here. It is working fine on Linux with various filesystems and I don't think I'm going to be able to take it further without running the debugger on a Mac. I wonder if the problem is even more esoteric. Is there a difference in pcre? Perhaps some difference in the Linux and Mac implementations in HFS+? (Or, more likely, my code just stinks...!)

purcell · 2013-10-30T19:24:10Z

I'll dig into the code myself when I get a chance and see if I can figure things out. Would be nice to have a simple shell-scripted test suite for the various exclusion patterns etc.

mcphail · 2013-10-30T19:25:32Z

Does the ignore still fail if you remove the asterisks from the .gitignore patterns?

purcell · 2013-10-30T19:27:26Z

You know, scratch that -- your fix works. I was picking up the wrong ag in my environment.

purcell · 2013-10-30T19:27:38Z

Sorry.

purcell · 2013-10-30T19:30:01Z

Now all of the following patterns correctly ignore ignored/a:

/ignored/*
/ignored/
ignored/
ignored/*
/ignored
ignored
a
/a

mcphail · 2013-10-30T19:31:27Z

Ha! Great.

nebirhos · 2013-10-31T10:44:05Z

Hi folks! This fix has been published somewhere?

mcphail · 2013-10-31T10:50:10Z

You can use the code from my pull request #284 but it hasn't been merged into the master branch by @ggreer yet.

nebirhos · 2013-10-31T10:52:14Z

great, thanks!

Fix ggreer#285 but needs testing for more complex directories

Attempt to fix issue 275 Only increment the scandir baton when recursing Fix ggreer#285 but needs testing for more complex directories Check ignore paths based on correct baton level Fix ggreer#287 but doesn't fix ggreer#144 Stop comparing terminating NULL in path string Might save a few cycles... Handle absolute .gitignore patterns properly Pattern /b/c/d should ignore /b/c/d but not a/b/c/d

raine · 2013-12-14T13:00:23Z

I assume this hasn't been fixed yet in any released version?

Got slightly confused when ag started acting weird after making a change to gitignore.

More specifically, replaced dir/ with /dir which makes git ignore the path from root only, but ag doesn't see it that way.

deepfire · 2013-12-21T15:17:35Z

Ok, I just submitted a duplicate bug for this..

purcell · 2013-12-21T15:21:14Z

Hey @ggreer -- would you mind reviewing #284 at some point please, which is a fix for this issue and the duplicate issue @deepfire just filed & closed? If @mcphail would need to make any further changes in order for the PR to be acceptable, I'm sure he'd be happy to do so. :-)

Attempt to fix issue 275 Only increment the scandir baton when recursing Fix ggreer#285 but needs testing for more complex directories Check ignore paths based on correct baton level Fix ggreer#287 but doesn't fix ggreer#144 Stop comparing terminating NULL in path string Might save a few cycles... Handle absolute .gitignore patterns properly Pattern /b/c/d should ignore /b/c/d but not a/b/c/d

ctrueden · 2015-02-24T15:36:48Z

I also filed a duplicate of this issue a while back (#595). FWIW, I wrote a patch to the cram tests that illustrates the problem, and could be handy in verifying a future fix: https://github.com/ctrueden/the_silver_searcher/commit/4f1399723ecb6db069af455b2f37ee38b7e3ddab

cartolari · 2015-05-15T20:48:26Z

I can also confirm the issue. And another problem that I think it's still related to the .gitignore file is a case where you ignore a entire folder but some files as exemplified by the following hypothetical .gitigore:

ignored/*
!ignored/not_ignored

If I search for some content the file ignored/not_ignored will not be included in the files for the search.

bdesham · 2015-08-13T16:54:14Z

This fix has still not been merged in, correct?

ctrueden · 2015-08-13T16:58:27Z

@bdesham I don't think anyone has actually submitted a patch that fixes this... have they?

bdesham · 2015-08-14T18:42:35Z

@ctrueden After reading through this thread again, I guess you’re right. PR #284 fixed this but @mcphail closed it because it went so long without being merged 😐

mcphail · 2015-08-14T19:18:53Z

I'm afraid there have been layers of breakage added to the ignore code since I made that patch, so there is no way I could keep it rebased against upstream. You can use my fork if you want to try the code. It isn't perfect, but I'm sure some enterprising soul could fix the edge cases where it breaks.

mbaer3000 · 2016-11-02T07:46:41Z

Can confirm the issue. It's a really painful having ag go through dist assets -- which are commonly exempted from version control via "/dist" in the .gitignore file.

because ag picks up too many dist + node_module files: ggreer/the_silver_searcher#285 (and possibly no longer maintained?)

mcphail mentioned this issue Oct 28, 2013

Keep recursion through ignore list within appropriate scope #284

Closed

mcphail pushed a commit to mcphail/the_silver_searcher that referenced this issue Oct 28, 2013

Only increment the scandir baton when recursing

a0e5ac1

Fix ggreer#285 but needs testing for more complex directories

mcphail pushed a commit to mcphail/the_silver_searcher that referenced this issue Oct 29, 2013

Only increment the scandir baton when recursing

58cfd55

Fix ggreer#285 but needs testing for more complex directories

mcphail pushed a commit to mcphail/the_silver_searcher that referenced this issue Oct 30, 2013

Only increment the scandir baton when recursing

7e9f5b1

Fix ggreer#285 but needs testing for more complex directories

mcphail pushed a commit to mcphail/the_silver_searcher that referenced this issue Nov 4, 2013

Only increment the scandir baton when recursing

6568ce1

Fix ggreer#285 but needs testing for more complex directories

mcphail pushed a commit to mcphail/the_silver_searcher that referenced this issue Nov 6, 2013

Only increment the scandir baton when recursing

7a0a05c

Fix ggreer#285 but needs testing for more complex directories

mcphail pushed a commit to mcphail/the_silver_searcher that referenced this issue Nov 29, 2013

Only increment the scandir baton when recursing

908aab7

Fix ggreer#285 but needs testing for more complex directories

deepfire mentioned this issue Dec 21, 2013

repository-absolute paths in .gitignore are not recognized #309

Closed

Wilfred mentioned this issue Dec 21, 2013

repository-absolute paths in .gitignore are not recognized Wilfred/ag.el#30

Closed

ggreer added the ignore label Sep 26, 2014

theospears mentioned this issue Oct 8, 2014

Incorrect handling for leading / in .gitignore in subdirectories #409

Closed

mcphail mentioned this issue Feb 24, 2015

gitignore absolute paths do not work when gitignore is in a subdirectory #595

Closed

aeonaut mentioned this issue Jun 17, 2015

Glob patterns don't work inside directories #100

Open

mz026 mentioned this issue Jun 27, 2017

Support "git grep" as backend dyng/ctrlsf.vim#174

Closed

alexkuang added a commit to alexkuang/dotfiles that referenced this issue Nov 10, 2019

install fd, use for fzf

3b181f4

because ag picks up too many dist + node_module files: ggreer/the_silver_searcher#285 (and possibly no longer maintained?)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doesn't respect "match-beginning" gitignore patterns in subdirs #285

Doesn't respect "match-beginning" gitignore patterns in subdirs #285

purcell commented Oct 28, 2013

mcphail commented Oct 28, 2013

purcell commented Oct 28, 2013

mcphail commented Oct 28, 2013

purcell commented Oct 28, 2013

purcell commented Oct 28, 2013

mcphail commented Oct 28, 2013

purcell commented Oct 28, 2013

purcell commented Oct 29, 2013

mcphail commented Oct 29, 2013

mcphail commented Oct 30, 2013

purcell commented Oct 30, 2013

mcphail commented Oct 30, 2013

purcell commented Oct 30, 2013

mcphail commented Oct 30, 2013

purcell commented Oct 30, 2013

purcell commented Oct 30, 2013

purcell commented Oct 30, 2013

mcphail commented Oct 30, 2013

nebirhos commented Oct 31, 2013

mcphail commented Oct 31, 2013

nebirhos commented Oct 31, 2013

raine commented Dec 14, 2013

deepfire commented Dec 21, 2013

purcell commented Dec 21, 2013

ctrueden commented Feb 24, 2015

cartolari commented May 15, 2015

bdesham commented Aug 13, 2015

ctrueden commented Aug 13, 2015

bdesham commented Aug 14, 2015

mcphail commented Aug 14, 2015

mbaer3000 commented Nov 2, 2016

Doesn't respect "match-beginning" gitignore patterns in subdirs #285

Doesn't respect "match-beginning" gitignore patterns in subdirs #285

Comments

purcell commented Oct 28, 2013

mcphail commented Oct 28, 2013

purcell commented Oct 28, 2013

mcphail commented Oct 28, 2013

purcell commented Oct 28, 2013

purcell commented Oct 28, 2013

mcphail commented Oct 28, 2013

purcell commented Oct 28, 2013

purcell commented Oct 29, 2013

mcphail commented Oct 29, 2013

mcphail commented Oct 30, 2013

purcell commented Oct 30, 2013

mcphail commented Oct 30, 2013

purcell commented Oct 30, 2013

mcphail commented Oct 30, 2013

purcell commented Oct 30, 2013

purcell commented Oct 30, 2013

purcell commented Oct 30, 2013

mcphail commented Oct 30, 2013

nebirhos commented Oct 31, 2013

mcphail commented Oct 31, 2013

nebirhos commented Oct 31, 2013

raine commented Dec 14, 2013

deepfire commented Dec 21, 2013

purcell commented Dec 21, 2013

ctrueden commented Feb 24, 2015

cartolari commented May 15, 2015

bdesham commented Aug 13, 2015

ctrueden commented Aug 13, 2015

bdesham commented Aug 14, 2015

mcphail commented Aug 14, 2015

mbaer3000 commented Nov 2, 2016