Re: [PATCH v4 5/6] get_short_oid: sort ambiguous objects by type, then SHA-1

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> diff --git a/sha1-name.c b/sha1-name.c
> index 9d7bbd3e96..46d8b1afa6 100644
> --- a/sha1-name.c
> +++ b/sha1-name.c
> @@ -409,6 +437,8 @@ static int get_short_oid(const char *name, int len, struct object_id *oid,
>  	status = finish_object_disambiguation(&ds, oid);
>  
>  	if (!quietly && (status == SHORT_NAME_AMBIGUOUS)) {
> +		struct oid_array collect = OID_ARRAY_INIT;
> +
>  		error(_("short SHA1 %s is ambiguous"), ds.hex_pfx);
>  
>  		/*
> @@ -421,7 +451,12 @@ static int get_short_oid(const char *name, int len, struct object_id *oid,
>  			ds.fn = NULL;
>  
>  		advise(_("The candidates are:"));
> -		for_each_abbrev(ds.hex_pfx, show_ambiguous_object, &ds);

So we used to let for_each_abbrev() to enumerate these object names
that share the prefix in the object name order and fed
show_ambiguous_object() to show them, which was the cause of the
output that is not grouped by type.  Now you collect them into
another oid_array and sort by type, relying on the fact to that the
for_each_abbrev() in the "collect" phase already does the de-duping.

Sounds good.

> +		for_each_abbrev(ds.hex_pfx, collect_ambiguous, &collect);
> +		QSORT(collect.oid, collect.nr, sort_ambiguous);
> +
> +		if (oid_array_for_each(&collect, show_ambiguous_object, &ds))
> +			BUG("show_ambiguous_object shouldn't return non-zero");
> +		oid_array_clear(&collect);
>  	}


> diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
> index 711704ba5a..2701462041 100755
> --- a/t/t1512-rev-parse-disambiguation.sh
> +++ b/t/t1512-rev-parse-disambiguation.sh
> @@ -361,4 +361,25 @@ test_expect_success 'core.disambiguate does not override context' '
>  		git -c core.disambiguate=committish rev-parse $sha1^{tree}
>  '
>  
> +test_expect_success C_LOCALE_OUTPUT 'ambiguous commits are printed by type first, then hash order' '
> +	test_must_fail git rev-parse 0000 2>stderr &&
> +	grep ^hint: stderr >hints &&
> +	grep 0000 hints >objects &&
> +	cat >expected <<-\EOF &&
> +	tag
> +	commit
> +	tree
> +	blob
> +	EOF
> +	awk "{print \$3}" <objects >objects.types &&
> +	uniq <objects.types >objects.types.uniq &&

Eww, that is somewhat tricky (but correct) use of "uniq", which
POSIX not just mandates adjacent duplicates to be removed, but also
forbids from removing duplicates that are not adjacent from each
other.  So the objects in the "hints" file are not grouped by type,
we will fail to see these four lines.

> +	test_cmp expected objects.types.uniq &&
> +	for type in tag commit tree blob
> +	do
> +		grep $type objects >$type.objects &&
> +		sort $type.objects >$type.objects.sorted &&
> +		test_cmp $type.objects.sorted $type.objects

We not only want to see objects grouped by type (and types shown in
a desired order), but within the same type we want them ordered by
object name.

OK.

> +	done
> +'
> +
>  test_done