On Thu, Apr 25 2019, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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). 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...
