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

.gitignore patterns beginning with '/' not treated correctly. #127

Closed
codyps opened this issue Dec 7, 2012 · 13 comments · Fixed by #132
Closed

.gitignore patterns beginning with '/' not treated correctly. #127

codyps opened this issue Dec 7, 2012 · 13 comments · Fixed by #132

Comments

@codyps
Copy link
Contributor

codyps commented Dec 7, 2012

For example, in the linux kernel tree's /.gitignore:

/linux

Causes ag to ignore the path /include/linux, which it should not (and recently caused me some pain).

Mercurial allows this via '^' in regexes, git doesn't use regexes. On another note: as .hgignore uses actual regexes instead of glob patterns (which fnmatch handles), so I don't think they'll work at all.

Possible solutions I see from glancing at the code:

  • create a separate listing in struct ignores for these types of "full path relative to ignore dir" matches. Of course, there would need to be duplicate names and regexes.
  • Build knowledge of what the '/' means into the filename matching. Potentially replace it with './' and have the "current directory" = "." be the directory the ignore file was from.
  • Construct, from all patterns (globs, regexes, & static), a FSM for performing the matching. Essentially, unify regexes and names. This could potentially decrease the cost of ignore look ups (or potentially increase it if branch-misses increase). It would also fix the lack of real hgignore support.
@codyps
Copy link
Contributor Author

codyps commented Dec 7, 2012

I should also note that as a temporary work around I've just adjusted the code to skip patterns starting with '/' so I don't miss important things.

@mutewinter
Copy link

Would love to see this get added. Right now patterns like /log/* are not being used for Ag's ignore. I'm running HEAD.

@losingkeys
Copy link

Same here (though I'm running version 0.14). The pattern that isn't getting ignored for me is /log/*.log (for reference).

@raimo
Copy link

raimo commented Apr 2, 2013

Just ran into this problem also. Using patterns that begin with '/' is the best practice in large projects if the exact directory structure is known, so this really is a crucial feature.

My specific problem is having this in .gitignore:

/tmp/performance

Running ag blabla still returns matches from file tmp/performance/PERF-REPORT-234234234.html

@MSch
Copy link

MSch commented Apr 20, 2013

Just ran into this problem too. At first I thought that ag doesn't deal with gitignores in subfolders. Having support for this is IMO crucial, because look at for example the default gitignore of Rails projects:

# Ignore bundler config.
/.bundle

# Ignore the default SQLite database.
/db/*.sqlite3
/db/*.sqlite3-journal

# Ignore all logfiles and tempfiles.
/log/*.log
/tmp

@smathy
Copy link

smathy commented Apr 22, 2013

Umm, 4148d6a seems to have reversed the proper handling of the leading / in .gitignore. Reverting that commit makes ag do the right thing for me.

@ggreer
Copy link
Owner

ggreer commented Apr 22, 2013

Actually, reverting that commit will make ag treat slashes incorrectly. Instead of just ignoring /tmp, it will ignore all files and directories named tmp.

It's safer to show extra matches than to silently ignore a file that the user expects to be searched.

@smathy
Copy link

smathy commented Apr 22, 2013

Ahh, I see. So we really need something that prepends a . for leading /s

novalis added a commit to novalis/the_silver_searcher that referenced this issue Jun 14, 2013
novalis added a commit to novalis/the_silver_searcher that referenced this issue Jun 14, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Jul 23, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Jul 23, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Aug 7, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Aug 7, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Aug 13, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Aug 13, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Aug 26, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Aug 26, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Sep 3, 2013
thomasf pushed a commit to thomasf/the_silver_searcher that referenced this issue Sep 3, 2013
@bitboxer
Copy link

I would love to see this fixed 😸

@rik
Copy link

rik commented Mar 24, 2014

This seems to be closed with #355

@ggreer
Copy link
Owner

ggreer commented Mar 24, 2014

That is correct.

@ggreer ggreer closed this as completed Mar 24, 2014
@bitboxer
Copy link

👍 Thanks for fixing this! ❤️

@smathy
Copy link

smathy commented Mar 24, 2014

Thanks 👌

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 a pull request may close this issue.

9 participants