From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Conrad Irwin <conrad.irwin@gmail•com>,
git@vger•kernel.org, Nguyen Thai Ngoc Duy <pclouds@gmail•com>,
Dov Grobgeld <dov.grobgeld@gmail•com>
Subject: Re: [PATCH] Don't search files with an unset "grep" attribute
Date: Wed, 01 Feb 2012 00:01:41 -0800 [thread overview]
Message-ID: <7vhazb3rtm.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120125214625.GA4666@sigill.intra.peff.net> (Jeff King's message of "Wed, 25 Jan 2012 16:46:26 -0500")
Jeff King <peff@peff•net> writes:
> On Mon, Jan 23, 2012 at 10:59:45PM -0800, Junio C Hamano wrote:
>
>> Conrad Irwin <conrad.irwin@gmail•com> writes:
>> > I used to use this approach, hooking into the "diff" attribute directly to mark
>> > a file as binary, however that was clearly a hack.
>>
>> After thinking about this a bit more, I have to say I disagree that it is
>> a hack.
>
> I kind of agree.
>
> The biggest problem is that the name is wrong. The "diff.*.command"
> option really is about generating a diff between two blobs of a certain
> type. But "diff.*.textconv" and "diff.*.binary" are really just
> attributes of the file, and may or may not have to do with generating a
> diff. Ditto for diff.*.funcname, I think.
>
> You argue, and I agree, that if we are talking about attributes of the
> files and not diff-specific things, then other parts of git can and
> should make use of that information.
>
> So if this was all spelled:
>
> $ cat .gitattributes
> *.pdf filetype=pdf
> $ cat .git/config
> [filetype "pdf"]
> binary = true
> textconv = pdf2txt
>
> I think it would be a no-brainer that those type attributes should apply
> to "git grep".
I think this discussion has, instead of forking into two equally
interesting subthreads, veered to a more intellectually stimulating
tangent and we ended up losing focus.
Regardless of what to do with "I do not want to grep in these types of
files" and "I want textconv applied when grepping in these types", which
would be new attributes to implement two new features, I would like to see
us first concentrate on fixing the "binary" issue. When somebody tells us
"Your autodetection may screw it up, but this file is binary; just show
'Binary files differ.' when comparing." with "-diff" (or "binary"), we
should honor that when "git grep" decides if it should take the 'Binary
file matches' codepath. We currently do not, and it clearly is a bug.
This is especially made somewhat urgent because I do not want a half-baked
"two pathspecs" approach that only "git grep" knows about when we add the
support for "git grep --exclude-path=...".
We should have to teach the underlying machinery that matches pathspec
about negative pathspec entries only once. After we have done so, all the
callers, not just "git grep", should be able to take advantage of the
change by just learning to place negative pathspec entries in the "struct
pathspec" they pass to the machinery. Doing anything else will lead to
madness of adding ad-hoc "here we should further filter with the other
negative 'struct pathspec'" in each and every application.
But I suspect that it would not materialize anytime soon. And I also
suspect that the correct handling of 'Binary file matches', which is a
pure bugfix, should solve the original issue started these threads 90% in
practice.
next prev parent reply other threads:[~2012-02-01 8:01 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-17 9:14 git-grep while excluding files in a blacklist Dov Grobgeld
2012-01-17 9:19 ` Nguyen Thai Ngoc Duy
2012-01-17 20:09 ` Junio C Hamano
2012-01-18 1:24 ` Nguyen Thai Ngoc Duy
2012-01-23 9:37 ` [PATCH] Don't search files with an unset "grep" attribute conrad.irwin
2012-01-23 18:33 ` Junio C Hamano
2012-01-23 22:59 ` Conrad Irwin
2012-01-24 6:59 ` Junio C Hamano
2012-01-25 21:46 ` Jeff King
2012-01-26 13:51 ` Stephen Bash
2012-01-26 17:29 ` Jeff King
2012-01-26 16:45 ` Michael Haggerty
2012-01-27 6:35 ` Jeff King
2012-02-01 8:01 ` Junio C Hamano [this message]
2012-02-01 8:20 ` Jeff King
2012-02-01 9:10 ` Jeff King
2012-02-01 9:28 ` Conrad Irwin
2012-02-01 22:14 ` Jeff King
2012-02-01 23:20 ` Jeff King
2012-02-02 2:03 ` Junio C Hamano
2012-02-01 23:21 ` [PATCH 1/2] grep: let grep_buffer callers specify a binary flag Jeff King
2012-02-02 0:47 ` Junio C Hamano
2012-02-02 0:52 ` Jeff King
2012-02-02 8:17 ` [PATCH 0/9] respect binary attribute in grep Jeff King
2012-02-02 8:18 ` [PATCH 1/9] grep: make locking flag global Jeff King
2012-02-02 8:18 ` [PATCH 2/9] grep: move sha1-reading mutex into low-level code Jeff King
2012-02-02 8:19 ` [PATCH 3/9] grep: refactor the concept of "grep source" into an object Jeff King
2012-02-02 8:19 ` [PATCH 4/9] convert git-grep to use grep_source interface Jeff King
2012-02-02 8:20 ` [PATCH 5/9] grep: drop grep_buffer's "name" parameter Jeff King
2012-02-02 8:20 ` [PATCH 6/9] grep: cache userdiff_driver in grep_source Jeff King
2012-02-02 18:34 ` Junio C Hamano
2012-02-02 19:37 ` Jeff King
2012-02-02 8:21 ` [PATCH 7/9] grep: respect diff attributes for binary-ness Jeff King
2012-02-02 8:21 ` [PATCH 8/9] grep: load file data after checking binary-ness Jeff King
2012-02-02 8:24 ` [PATCH 9/9] grep: pre-load userdiff drivers when threaded Jeff King
2012-02-02 8:30 ` [PATCH 0/9] respect binary attribute in grep Jeff King
2012-02-02 11:00 ` Thomas Rast
2012-02-02 11:07 ` Jeff King
2012-02-02 18:39 ` Junio C Hamano
2012-02-04 19:22 ` Pete Wyckoff
2012-02-04 23:18 ` Jeff King
2012-02-01 23:21 ` [PATCH 2/2] grep: respect diff attributes for binary-ness Jeff King
2012-02-01 16:28 ` [PATCH] Don't search files with an unset "grep" attribute Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vhazb3rtm.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=conrad.irwin@gmail$(echo .)com \
--cc=dov.grobgeld@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox