Re: for-each-ref output order change in 2.7.0

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jan 9, 2016, 21:29 UTC
Message-ID: <CAOLa=ZTUcmAjpC4nf_o7ZAHC3Asj9iwfKOOrJr90_kAyMbd22A@mail.gmail.com>
In-Reply-To: <56914AD2.9060303@kdbg.org>

On Sat, Jan 9, 2016 at 11:30 PM, Johannes Sixt <j6t@kdbg.org> wrote:

Show 30 quoted lines

> Am 09.01.2016 um 18:21 schrieb Karthik Nayak:
>>>>
>>>> (Note: The alphabetical-ness of the branch names is reversed, which
>>>> seems logical given my original sort was -committerdate. A
>>>> --sort=refname looks like this.
>>>>
>>>> refs/heads/!@#$% -> Tue Jan 3 17:00:51 2012 +1100
>>>> refs/heads/!@#% -> Tue Jan 3 17:04:06 2012 +1100
>>>> refs/heads/% -> Tue Jan 3 17:00:51 2012 +1100
>>>> refs/heads/@#$% - >Tue Jan 3 17:00:51 2012 +1100
>>>> refs/heads/@#% -> Tue Jan 3 17:00:51 2012 +1100
>>>>
>>>> That's probably more correct too.)
>>>>
>>>> Best regards,
>>>> Bryan Turner
>>
>>
>> This is correct as per the patch, But I'm wondering if this is desired.
>> I.E when sorting in reverse order should the fallback (alphabetical sort)
>> also be in reverse order?
>
>
> IMO, the fallback sorting should be in reverse order only when the user
> explicitley asked for reverse order. Just because committer date implies
> some "reverse" ordering should not imply that refs with the same committer
> date should also be listed in reverse alphabetical order.
>
> -- Hannes
>

I was thinking along the same lines. But do we want to expose the fallback to the user (i.e let them choose if its reversible or not)? If not Its only a small change required:

diff --git a/ref-filter.c b/ref-filter.c
index cc850b0..59d43d7 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1554,7 +1554,7 @@ static int cmp_ref_sorting(struct ref_sorting
*s, struct ref_array_item *a, stru
                if (va->ul < vb->ul)
                        cmp = -1;
                else if (va->ul == vb->ul)
-                       cmp = strcmp(a->refname, b->refname);
+                       return strcmp(a->refname, b->refname);
                else
                        cmp = 1;
        }

I could send a patch, as soon as we decide if we want to stick something
simple like this or expose the fallback sort to the user.

-- 
Regards,
Karthik Nayak