Ignore with wildignore

[losingkeys]

Closes #67

[losingkeys]

Looking for feedback/testing/review:

• Is g:ag_use_wildignore a good option name?
• Should it be on by default? (I think similar functionality is enabled by default in plugins like vim-vinegar and ctrlp.vim), so I'd say yes.
• Code review?
  • I'm not sure if I did this function right: https://github.com/rking/ag.vim/pull/77/files#diff-05f5ccde4a41d818acd6e65aeba5a26eR135. Is there a function type that's local to the script? I don't think that would matter, but it's probably better style.
  • Other style issues?
  • Will this fail with some patterns that need to be escaped? Mine work, but I don't have that many and only tested it lightly

Let me know if you want help pulling it down and testing it locally.

[rgrinberg]

I haven't reviewed the code but would welcome the functionality. +1 from me

[ches]

FWIW, ack.vim had this feature and removed it because of problems. Granted, one of the problems there was that it needed more special-case handling workarounds for people that wanted to use Ag with ack.vim, which maybe isn't a problem here (until a next-generation tool comes along that people want to set as agprg? Or maybe this PR will break things for people who use ag.vim but have agprg fall back to Ack on a system where they don't have Ag installed? My vimrc does things like this.).

For my two cents I think I agree with the author's reasoning on that issue:

• Ag has it's own ignore system, and one with very good defaults already that are a major reason it's so fast (.gitignore support, etc.). My first instinct is to use .agignore—I'd be surprised and confused if something unexpectedly wasn't turning up in results, and it ended up being because of wildignore.
• There could possibly be headaches like shell quoting or cases where patterns valid in Vim's syntax don't work the same way with Ag's PCRE regexes.
• It might be more configuration for users or more special handling in the plugin when agprg is customized.
• wildignore is not a setting I personally think about much, and I think the issue linked above shows that many Vim users aren't aware of it, so it may cause surprises.

[losingkeys]

Thanks for your thought on this. I've added some comments inline below.

FWIW, ack.vim had this feature and removed it because of problems. Granted, one of the problems there was that it needed more special-case handling workarounds for people that wanted to use Ag with ack.vim, which maybe isn't a problem here (until a next-generation tool comes along that people want to set as agprg? Or maybe this PR will break things for people who use ag.vim but have agprg fall back to Ack on a system where they don't have Ag installed? My vimrc does things like this.).

I don't think this needs to be a concern for this plugin. If we can help those users deal with using other programs w/this wrapper, then good. However I think it's ok for this plugin just to focus on ag support. We can fork it when $BETTER_SEARCH_TOOL becomes available 😄.

For my two cents I think I agree with the author's reasoning on that issue:

Ag has it's own ignore system, and one with very good defaults already that are a major reason it's so fast (.gitignore support, etc.). My first instinct is to use .agignore—I'd be surprised and confused if something unexpectedly wasn't turning up in results, and it ended up being because of wildignore.

I sort of agree with you here, but I also like that (by setting 'widlignore' I can get some consistent behavior from vim plugins.

There could possibly be headaches like shell quoting or cases where patterns valid in Vim's syntax don't work the same way with Ag's PCRE regexes.

I agree here. This is the main reason I've been leaving this one open for so long (other than I've been busy and it needs to be rebased). Maybe we could add testing to this plugin to prove this isn't possible/likely.

It might be more configuration for users or more special handling in the plugin when agprg is customized.

You could just turn off g:ag_use_wildignore if it's intrusive.

wildignore is not a setting I personally think about much, and I think the issue linked above shows that many Vim users aren't aware of it, so it may cause surprises.

I guess it depends on if you set 'wildignore' and what you expect it to do, but I agree that it could cause surprises.

These aren't all amazing arguments, and more discussion/testing/rebasing is needed if/before we want to merge this, but I still think it could be useful.

[ches]

Fair points. I'm certainly not opposed to adding the feature and think some people will like it if it's solid, just wanted to air those counterpoints for discussion.

My vote would be for this to be off by default, though. Personally, everything in my 'wildignore' setting is almost always covered by VCS ignores anyway, so it's superfluous to tell Ag specifically and leaves the extra args as nothing but a potential cause of error.

[rgrinberg]

I also think it would be better to leave it off by default. You can imagine the flurry of users asking why things are slightly different with the update 😄

[abarax]

Any status on this? The feature is much needed.