Re: [PATCH 2/2] diffcore-pickaxe: add --pickaxe-raw-diff for use with -G

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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

	/(IRQF_SHARED|IRQF_ONESHOT)

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).

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.