Re: [PATCH v12 4/4] ls-remote: create '--sort' option

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 9, 2018, 02:31 UTC
Message-ID: <CAPig+cR4kA+SHmOOOq20BJGzqQkqKvYrFEqPdDF6g6c=apjHtA@mail.gmail.com>
In-Reply-To: <xmqq7eph9wds.fsf@gitster-ct.c.googlers.com>

On Sun, Apr 8, 2018 at 6:16 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> Harald Nordgren <haraldnordgren@gmail.com> writes:
>> +--sort=<key>::
>> +     Sort based on the key given. [...]
>
> This is a tangent but I suspect that the description of --sort in
> git-for-each-ref documentation is wrong on the use of multiple
> options, or I am misreading parse_opt_ref_sorting(), which I think
> appends later keys to the end of the list, and compare_refs() which
> I think yields results when an earlier key in the last decides two
> are not equal and ignores the later keys.  The commit that added the
> description was c0f6dc9b ("git-for-each-ref.txt: minor
> improvements", 2008-06-05), and I think even back then the code in
> builtin-for-each-ref.c appended later keys at the end, and it seems
> nobody complained, so it is possible I am not reading the code right
> before fully adjusting to timezone change ;-)

I just re-read parse_opt_ref_sorting(), and I'm pretty sure that it is _prepending_ keys to the list, despite the confusingly named variable "sorting_tail", thus it agrees with the documentation that --sort=<key> specified last becomes primary sort key.

For me, at least, that behavior (last --sort= becomes primary key) feels counterintuitive, but it's too late to change it now.