Re: [PATCH v4 5/8] list-objects: implement object type filter

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

On Tue, Apr 13, 2021 at 11:57:04AM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Apr 12 2021, Patrick Steinhardt wrote:
> 
> > +	} else if (skip_prefix(arg, "object:type=", &v0)) {
> > +		int type = type_from_string_gently(v0, -1, 1);
> > +		if (type < 0) {
> > +			strbuf_addstr(errbuf, _("expected 'object:type=<type>'"));
> 
> Maybe:
> 
> 	strbuf_addstr(errbuf, _("'%s' for 'object:type=<type>' is not a valid object type"), v0);

`strbuf_addf`, but otherwise I agree that it does make for a more
actionable error message.

> > +# Test object:type=<type> filter.
> > +
> > +test_expect_success 'setup object-type' '
> > +	git init object-type &&
> > +	echo contents >object-type/blob &&
> > +	git -C object-type add blob &&
> > +	git -C object-type commit -m commit-message &&
> > +	git -C object-type tag tag -m tag-message
> > +'
> 
> Does this really need to not be the shorter:
> 
>     test_create_repo object-type &&
>     test_commit -C object-type blob
> 
> Or if it really needs the annotated tag maybe that + --no-tag + create
> the tag after, and it can eventually use my
> https://lore.kernel.org/git/patch-06.16-8d43fdd5865-20210412T110456Z-avarab@gmail.com/

It must be an annotated tag because that's what the `--object` flag of
git-rev-list(1) is working on: objects. But anyway, as you say I can
still use these two helpers and then manually create the tag.

> > +test_expect_success 'verify object:type=blob prints blob and commit' '
> > +	(
> > +		git -C object-type rev-parse HEAD &&
> > +		printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob)
> > +	) >expected &&
> 
> Just use > and >> or a single printf with two arguments instead of a
> subshell?

I don't really mind, happy to take this proposal.

Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]