All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael J Gruber <git@drmicha.warpmail.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters
Date: Tue, 05 Feb 2013 09:48:05 +0100	[thread overview]
Message-ID: <5110C745.9050306@drmicha.warpmail.net> (raw)
In-Reply-To: <7vtxps2eb2.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 04.02.2013 18:12:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Recently and not so recently, we made sure that log/grep type operations
>> use textconv filters when a userfacing diff would do the same:
>>
>> ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
>> b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
>> 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
>>
>> "git grep" currently does not use textconv filters at all, that is
>> neither for displaying the match and context nor for the actual grepping.
>>
>> Introduce a binary mode "--textconv" (in addition to "--text" and "-I")
>> which makes git grep use any configured textconv filters for grepping
>> and output purposes.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>>
>> Notes:
>>     I'm somehow stuck in textconv/filespec/... hell, so I'm sending this out
>>     in request for help. I'm sure there are people for whom it's a breeze to
>>     get this right.
> 
> Looks like the patch touches the right places in the codepaths that
> need to be updated from a quick read.

I should have added that this coredumps when used on blobs and that
textconv isn't invoked in any case (because it crashes on blobs, and I
haven't implemented tetxconv on worktree files yet).

I'm somehow struggling to use just the right bits from the more diff
centric helpers like fill_textconv or fill_one (which is static so far).

>>     The difficulty is in getting the different cases (blob/sha1 vs.
>>     worktree) right, and in making the changes minimally invasive.
> 
> I think grep_source abstraction was intended to be a way for that,
> and if what it gives you is not sufficient for your needs, extending
> it should not be seen as "invasive" at all.

It seems to me that we textconvified the diff versions of these
abstractions, but not the grep_source abstractions. Doing it at the
source appears to be the right thing.

Michael

  reply	other threads:[~2013-02-05  8:48 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04 15:27 [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters Michael J Gruber
2013-02-04 17:12 ` Junio C Hamano
2013-02-05  8:48   ` Michael J Gruber [this message]
2013-02-05 11:13 ` Jeff King
2013-02-05 16:21   ` Michael J Gruber
2013-02-05 20:11     ` Jeff King
2013-02-06 15:08       ` [RFC/PATCH 0/4] textconv for show and grep Michael J Gruber
2013-02-06 15:08         ` [RFC/PATCH 1/4] show: obey --textconv for blobs Michael J Gruber
2013-02-06 16:53           ` Junio C Hamano
2013-02-06 22:12             ` Jeff King
2013-02-06 23:49               ` Junio C Hamano
2013-02-07  0:10                 ` Jeff King
2013-02-07  0:26                   ` Junio C Hamano
2013-02-07  8:48             ` Michael J Gruber
2013-02-06 22:06           ` Jeff King
2013-02-07  9:05             ` Michael J Gruber
2013-02-07  9:11               ` Jeff King
2013-02-07  9:34                 ` Michael J Gruber
2013-02-07  9:43                   ` Jeff King
2013-02-06 15:08         ` [RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters Michael J Gruber
2013-02-06 16:47           ` Junio C Hamano
2013-02-06 22:19           ` Jeff King
2013-02-06 22:23             ` Junio C Hamano
2013-02-06 22:43               ` Jeff King
2013-02-06 15:08         ` [RFC/PATCH 3/4] grep: allow to use " Michael J Gruber
2013-02-06 15:12           ` Matthieu Moy
2013-02-06 22:23           ` Jeff King
2013-02-06 15:08         ` [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path Michael J Gruber
2013-02-06 22:36           ` Jeff King
2013-02-07  9:05             ` Michael J Gruber
2013-02-07  9:26               ` Jeff King
2013-02-07  9:47                 ` Michael J Gruber
2013-02-07  9:55                   ` Jeff King
2013-02-07 10:31                     ` Michael J Gruber
2013-02-07 18:03                       ` Junio C Hamano
2013-02-08 11:27                         ` Michael J Gruber
2013-02-06 16:55         ` [RFC/PATCH 0/4] textconv for show and grep 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=5110C745.9050306@drmicha.warpmail.net \
    --to=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.