Re: [PATCH] Don't search files with an unset "grep" attribute

On Thu, Jan 26, 2012 at 05:45:16PM +0100, Michael Haggerty wrote:

You're right. I thought of it as an annotation that the project could mark via .gitattributes, or the user could mark via .git/info/attributes. But that is not following the right split of responsibility for attributes and config. The attributes should annotate "this isn't really part of the regular git code base" or "this is really part of the nedmalloc codebase". And then the _config_ should say "when I am grepping, I am not interested in nedmalloc". I.e.:

  # mark a set of paths with an attribute
  echo "compat/nedmalloc external" >>.gitattributes

  # and then ignore that attribute for this grep
  git grep --exclude-attr=external 

  # or for all greps
  git config --add grep.exclude external

and git doesn't even have to care about what the attribute is called. It's between the project and the user how they want to annotate their files, and how they want to feed them to grep.

Or any other program, for that matter. I wonder if this could also be a more powerful way of grouping files to be included or excluded from diff pathspecs. Something like (and I'm just talking off the top of my head, so there may be some syntactic conflicts here):

  # annotate some files
  cat >>.gitattributes <<-\EOF
  t/t????-*.sh test-script
  t/lib-*.sh test-script
  t/test-lib.sh test-script
  EOF

  # and then consider the tagged files to be a group, and look only at
  # that group
  git log :attr:test-script

  # ditto, but imagine we had the negative pathspecs Duy has proposed
  git log :~attr:test-script

That seems kind of cool to me. But maybe it is getting into crazy over-engineering. I like the idea that we don't need a new option to grep or diff; rather it is simply a new syntax for mentioning paths.

So obviously I took what you were saying here and ran with it above. But I do disagree with one thing here: the attributes should be giving some tag to the paths, but the actual decision about whether to grep should be part of the _config_. That's the usual split we have for all of the other attributes, and I think it makes sense and has worked well.

I dunno. I admit my attribute usage tends to just match by extension, and I generally only have one or two such lines.

I think it's not a big deal if you are making your own macros. I was more concerned that people would want to use the "binary" macro to get the "-grep" automagic, but could not do so because they don't want "-diff", but rather "diff=foo".

Anyway, after reading your response and thinking on it more, I think "-grep" is totally the wrong way to go. If the files are marked binary, then grep should be respecting "-diff" or the "diff.*.binary" config. If we want to do more advanced exclusion, then the right place for that is the config file (or the weird :attr pathspec thing I mentioned above).

I don't think there's any semantic reason why that is not workable. It's simply not syntactically allowed at this point.

-Peff