On Thu, Dec 14, 2017 at 1:22 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > - what about mode changes? If the file became executable but the > blob content didn't change, does that commit match? ./git log --find-object=$(git rev-parse ba67504f:t/perf/p3400-rebase.sh) claims it does find the mode change (commit ba67504f is just a mode change) > - are copies and renames shown (if I am passing -M -C)? It restricts the commits shown, not the renamed files. But I assume you mean it the same way as with mode changes. I did not find a good commit in gits history to demonstrate, but as it is orthogonal to the object id restrictions, I would think it works > Nit, not related to this change: it would be nice to have a long > option to go along with the short name '-t' --- e.g. --include-trees. follow up patches welcome. :) > > Another nit: s/gitlink entry/submodule commit/, perhaps. The commit > object is not a gitlink entry: it is pointed to by a gitlink entry. Well, what if the user doesn't have a submodule, but uses gitlinks for other purposes? We do inspect the gitlink, so it is correct IMHO. > Another documentation idea: it may be nice to point out that this > is only about the preimage and postimage submodule commit and that > it doesn't look at the history in between. That is sensible. One might be tempted to ask: "Which superproject commit contains a submodule pointer, that has commit $X in the submodule history?", but this new option is totally not answering this. >> The >> reason why these commits both occur prior to v2.0.0 are evil >> merges that are not found using this new mechanism. > > Would it be worth the doc mentioning that this doesn't look at merges > unless you use one of the -m/-c/--cc options, or does that go without > saying? I assumed it goes without saying, just like the lacking -t could mean to ignore trees. ;) > > [...] >> --- a/Documentation/diff-options.txt >> +++ b/Documentation/diff-options.txt >> @@ -500,6 +500,12 @@ information. >> --pickaxe-regex:: >> Treat the <string> given to `-S` as an extended POSIX regular >> expression to match. >> + >> +--find-object=<object-id>:: >> + Restrict the output such that one side of the diff >> + matches the given object id. The object can be a blob, >> + gitlink entry or tree (when `-t` is given). > > I like this name --find-object more than --blobfind! I am not sure it > quite matches what the user is looking for, though. We are not > looking for all occurences of the object; we only care about when the > object appears (was added or removed) in the diff. Thanks! Yes, but the 'edges' are so few commits that a further walk will reveal all you need to know? > > Putting it in context, we have: > > pickaxe options: > -S: detect changes in occurence count of a string > -G: grep lines in diff for a string > > --pickaxe-all: > do not filter the diff when the patch matches pickaxe > conditions. > > kind of like log --full-diff, but restricted to pickaxe > options. > --pickaxe-regex: treat -S argument as a regex, not a string > > Is this another kind of pickaxe option? It feels similar to -S, but > at an object level instead of a substring level, so in a way it would > be appealing to call it --pickaxe-object. Does --pickaxe-all affect > it like it affects -S and -G? > > Another context to put it in is: > > --diff-filter: > limit paths (but not commits?) to those with a change > matching optarg > > If I understand correctly, then --diff-filter does not interact with > --pickaxe-all, or in other words it is a different filtering > condition. Is this another kind of diff filter? In that context, it > may be appealing to call it something like --object-filter. > > --diff-filter is an example where it seems appealing to have a > --full-diff option to diff-tree that could apply to all filters and > not just pickaxe. > > [... implementation snipped ...] > > The implementation looks lovely and I'm especially happy about the > tests. Thanks for writing it. > > Thoughts? > Jonathan Regarding finding a better name, I would want to hear from others, I am happy with --find-object, though I can see --pickaxe-object or --object--filter to be a good narrative as well. Stefan
