Re: [PATCH 1/1] diffcore: add a filter to find a specific blob

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

Stefan Beller <sbeller@google.com> writes:

> the information to what is shown. For example:
>
>   $ ./git log --oneline --blobfind=v2.0.0:Makefile
>   b2feb64309 Revert the whole "ask curl-config" topic for now
>   47fbfded53 i18n: only extract comments marked with "TRANSLATORS:"

This part is a bit stale???

> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index dd0dba5b1d..67a99e522b 100644
> --- 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,
> +	or gitlink entry.

OK.  In principle you should also be able to find a tree, but I do
not now how useful it would be.  Extending it to gitlink, which is
another kind of leaf node in the reachability DAG, does make tons of
sense---it's a no brainer that I feel ashamed not to have thought of
myself ;-)

> +LIB_OBJS += diffcore-oidfind.o

Just to nitpick, but "blobfind" was to find "blob", and if you are
extending it to find any "object", then that should be "objfind".
"oid" is _A_ way to refer to an object (i.e. the _name_ of it), and
name is *not* the same as the thing the name refers to, so...

> +static int parse_oidfind_opt(struct diff_options *opt, const char *arg)
> +{
> +	struct object_id oid;
> +
> +	/* We don't even need to have the object, any oid works. */
> +	if (get_oid_blob(arg, &oid))
> +		return error("unable to resolve '%s'", arg);

Should this still be get_oid_blob(), or should it be less specific
to blobs?

> +test_expect_success 'find the greeting blob' '
> +	cat >expect <<-EOF &&
> +	Revert "add the greeting blob"
> +	add the greeting blob
> +	EOF
> +
> +	git log --format=%s --find-object=greeting^{blob} >actual &&
> +
> +	test_cmp expect actual
> +'

Makes sense.

> +
> +test_expect_success 'setup a submodule' '
> +	test_create_repo sub &&
> +	test_commit -C sub sub &&
> +	git submodule add ./sub sub &&
> +	git commit -a -m "add sub"
> +'
> +
> +test_expect_success 'find a submodule' '
> +	cat >expect <<-EOF &&
> +	add sub
> +	EOF
> +
> +	git log --format=%s --find-object=HEAD:sub >actual &&
> +
> +	test_cmp expect actual
> +'

Nice (and cute).

> +test_done

Looking good.  Thanks.