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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 11, 2017, 23:17 UTC
Message-ID: <xmqqr2s04zxw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171211195835.57057-2-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 5 quoted lines

> 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???

Show 13 quoted lines

> 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...

Show 7 quoted lines

> +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?

Show 10 quoted lines

> +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.

Show 17 quoted lines

> +
> +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.