Hi,
Stefan Beller wrote:
> Junio hinted at a different approach of solving this problem, which this
> patch implements. Teach the diff machinery another flag for restricting
> the information to what is shown. For example:
>
> $ ./git log --oneline --find-object=v2.0.0:Makefile
> b2feb64309 Revert the whole "ask curl-config" topic for now
> 47fbfded53 i18n: only extract comments marked with "TRANSLATORS:"
>
> we observe that the Makefile as shipped with 2.0 was appeared in
> v1.9.2-471-g47fbfded53 and in v2.0.0-rc1-5-gb2feb6430b.
Neat!
Nit: s/was appeared/appeared/ (not a big deal though, since this is
already in 'next').
>From the above it's not clear to me whether this is about commits
where the object was added or where the object was removed.
Fortunately, the docs are a bit clearer:
... one side of the diff
matches the given object id. The object can be a blob,
gitlink entry or tree (when `-t` is given).
So this appears to be about both additions and removals. Followup
questions:
- are copies and renames shown (if I am passing -M -C)?
- what about mode changes? If the file became executable but the
blob content didn't change, does that commit match?
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.
Another nit: s/gitlink entry/submodule commit/, perhaps. The commit
object is not a gitlink entry: it is pointed to by a gitlink entry.
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.
> 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?
[...]
> --- 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.
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
