mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <>
To: Junio C Hamano <>
Cc: Eugeniu Rosca <>,, Jeff King <>,
	Julia Lawall <>,
	Eugeniu Rosca <>
Subject: Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G
Date: Thu, 25 Apr 2019 14:25:13 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu, Apr 25 2019, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <> writes:
>>> I agree. I am a bit bothered by the fact that
>>> `git log --oneline -Ux -G<regex> --pickaxe-raw-diff` outputs the
>>> contents/patch of a commit. My expectation is that we have the
>>> `log -p` knob for that?
>> This is unrelated to --pickaxe-raw-diff, -U<n> just implies -p in
>> general. See e.g. "git log -U1".
> The reason why I found this exchange interesting is because I think
> it shows a noteworthy gap between end-user expectations and what the
> implementors know.
> Stepping back (or sideways) a bit, pretend for a while that there
> were no "pickaxe" feature in Git.  Instead there is the "patch-grep"
> tool whose design is roughly:
>    1. It reads "git log -p" output from its standard input, and
>       splits the lines into records, each of which consists of the
>       header part (i.e. starting at the "commit <object name>" line,
>       to the first blank line before the title), the log message
>       part, and the patch part.
>    2. It takes command line arguments, which are, like "git grep",
>       patterns to match and instructions on how to combine the match
>       result.
>    3. It applies the match criteria only to the patch part of each
>       record.  A record without any match in the patch part is
>       discarded.
>    4. It uses the surviving record's "commit <object name>" lines
>       to decide what commits to show.  It does the moral equivalent
>       of invoking "git show" on each of them, and perhaps lets you
>       affect how the commits are shown.
>       Or perhaps it just lists the commit object names chosen for
>       further processing by downstream tools that read from it.
> So the user would be able to say something like
> 	git log -Ux --since=6.months |
> 	git patch-grep \
> 		--commit-names-only \
> 		--all-match \
> 		-e '+.*devm_request_threaded_irq(IRQF_SHARED)' \
>                 -e '-.*devm_request_threaded_irq(IRQF_ONESHOT)' |
> 	xargs git show --oneline -s
> As an implementor, you know that is not how your -G<pattern> thing
> works, but coming from the end-user side, I think it is a reasonable
> mental model to expect a tool to work more like so.  And I think the
> expectation from combining --oneline with -Ux was that the -U option
> would apply to step 1, not step 4 (as --oneline is a clear
> indication that the user wants a very concise final result).
> Personally, I think the _best_ match for the original wish would be
> to have that hypothetical "git patch-grep" read from "git log -L"
> that is limited to the C function in the source the user is
> interested in.
> And until "git patch-grep" becomes reality, I would probably have
> done
> 	git log -L<function of interest> -U<x> | less
> and asked "less" to skip to a match with
> and then kept hitting 'n' until I find what replaces them, as a
> stop-gap measure.
> By the way, I think your thing is interesting regardless, even if it
> does not match the use case in the original thread (it actually may
> match---I didn't think it through).

Yeah it's definitely a bit orthagonal, should have sent it in reply to
something else and actually read the E-Mail, but I think it's useful.

> Because in the context of diff/log family, however, the word "raw"
> has a specific connotation about the "--raw" format (as opposed to
> "--patch"), I would not call this "grep the patch output itself,
> instead of grepping the source (guided by the patch output to tell
> what lines are near the lines that got replaced)" feature anything
> "raw", by the way.

I agree, brainfarted on not thinking about "raw". Do you or anyone have
a suggestion for a better CLI option name?

Maybe --pickaxe-patch or --pickaxe-patch-format (to go with git-diff's
-u aka --patch (i.e. not --raw) default format)? Or
--pickaxe-G-with-context or --pickaxe-with-context or
--with-pickaxe-context or --pickaxe-context ? All of these suck, but I'm
coming up blank on a better one :)

Probably the least shitty of those shitty options is --pickaxe-patch,
since we have --patch which triggers the same format, and we can
document that the default is a -G search through --no-pickaxe-patch, and
you can just tweak the format.

It also leaves the door open (unlike having *-G-* in the option) to
support this for -S if anyone cared...

  reply	other threads:[~2019-04-25 12:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-24 10:26 Multi-line 'git log -G<regex>'? Eugeniu Rosca
2019-04-24 15:22 ` [PATCH 0/2] diffcore-pickaxe: implement --pickaxe-raw-diff Ævar Arnfjörð Bjarmason
2019-04-24 15:22 ` [PATCH 1/2] diffcore-pickaxe: refactor !one or !two case in diff_grep Ævar Arnfjörð Bjarmason
2019-04-24 15:22 ` [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G Ævar Arnfjörð Bjarmason
2019-04-24 15:37   ` Ævar Arnfjörð Bjarmason
2019-04-24 22:46     ` Eugeniu Rosca
2019-04-24 23:24       ` Ævar Arnfjörð Bjarmason
2019-04-25  0:44         ` Junio C Hamano
2019-04-25 12:25           ` Ævar Arnfjörð Bjarmason [this message]
2019-05-03  9:10             ` Eugeniu Rosca
2019-05-03  8:37           ` Eugeniu Rosca
2019-04-25  0:54         ` Eugeniu Rosca
2019-04-25 12:14           ` Ævar Arnfjörð Bjarmason
2019-05-03  3:15           ` Jeff King

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:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \

* 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.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).