From: "Ævar Arnfjörð Bjarmason" <email@example.com>
To: Junio C Hamano <firstname.lastname@example.org>
Cc: Eugeniu Rosca <email@example.com>,
firstname.lastname@example.org, Jeff King <email@example.com>,
Julia Lawall <firstname.lastname@example.org>,
Eugeniu Rosca <email@example.com>
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: <firstname.lastname@example.org> (raw)
On Thu, Apr 25 2019, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <email@example.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
> 3. It applies the match criteria only to the patch part of each
> record. A record without any match in the patch part is
> 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
> 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...
next prev parent 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
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: http://vger.kernel.org/majordomo-info.html
* 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).