Re: [PATCH 2/2] help: ensure &keys_uniq follows sort -u

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

Amisha Chhajed <amishhhaaaa@gmail.com> writes:

> No, there is a case where it would not be sorted(keys_uniq won't be sorted
> even though keys is), more details on the case[0] and steps to reproduce[1].
> [0] https://lore.kernel.org/git/CAPvEtrfEZXHxcDf=z60ODfUA8cS81rhF1y7KEZApEBby7aCa1A@mail.gmail.com/
> [1] https://lore.kernel.org/git/20260212041017.91370-1-amishhhaaaa@gmail.com/T/#m64880c5cd0d36e35bc78692757cf206b13496aea
> only reason it is not causing a problem now is because we do not have
> this edge case appearing git documentation(from where the keys are built)
> but if someday a case like this appears there then it would cause problems.

Ah, if you already have a reproduction case , it would have been
very good to add it as a new test.  That way, we can (1) apply the
patch, (2) tentatively revert only the code change, (3) build and
run test to see that the test breaks, demonstrating an existing
breakage, (4) restore the code change we tentatively reverted, (5)
build and run test again to see that the existing breakage is now
gone.

>> This is not a performance critical part of the system, so it is OK
>> as a future-proof measure to sort keys_uniq immediately before we
>> start doing something that we _care_ about its sortedness (e.g.,
>> presenting the final output to the user), even if keys_uniq is known
>> to be already sorted with the current code.  Using sort_u here would
>> allow us not to worry about how keys_uniq is constructed in that
>> ugly loop.
>
> Agreed, we do not need to sort it twice if we decouple CONFIG_HUMAN
> from the rest of the switch case, that is a great way to go about it,
> thank you!.
> I will work on it.

Thanks.