On Wed, Feb 26, 2014 at 4:05 PM, Jeff King <p...@peff.net> wrote:
> On Tue, Feb 25, 2014 at 07:22:15PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> versioncmp() is copied from string/strverscmp.c in glibc commit
>> ee9247c38a8def24a59eb5cfb7196a98bef8cfdc, reformatted to Git coding
>> style. The implementation is under LGPL-2.1 and according to [1] I can
>> relicense it to GPLv2.
>
> Cool. I think doing this makes the most sense, as we do not have to
> worry about build-time config (and I do not see any particular reason
> why we would want to use the system strverscmp on glibc systems).
Another reason I want to stay away from glibc is I want to fix the
algorithm to sort YY-preXX and YY-rcXX before YY. There could be
reasons that glibc might reject such a change even if it makes sense
in our context. Even if we make it to newer glibc and fix compat
version, people on older glibc will not receive the fix while people
on compat do. Not so good.
>> +static int parse_opt_sort(const struct option *opt, const char *arg, int
>> unset)
>> +{
>> + int *sort = opt->value;
>> + if (*arg == '-') {
>> + *sort = REVERSE_SORT;
>> + arg++;
>> + } else
>> + *sort = STRCMP_SORT;
>> + if (starts_with(arg, "version:")) {
>> + *sort |= VERCMP_SORT;
>> + arg += 8;
>> + } else if (starts_with(arg, "v:")) {
>> + *sort |= VERCMP_SORT;
>> + arg += 2;
>> + }
>> + if (strcmp(arg, "refname"))
>> + die(_("unsupported sort specification %s"), arg);
>
> I found this logic a bit weird, as STRCMP_SORT and VERCMP_SORT are not
> mutually exclusive flags, but REVERSE and STRCMP are. I would have
> thought REVERSE is the flag, and the other two are selections. Like:
>
> int flags = 0;
>
> if (*arg == '-') {
> flags |= REVERSE_SORT;
> arg++;
> }
> if (starts_with(arg, "version:")) {
> *sort = VERCMP_SORT;
> arg += 8;
> } else
> *sort = STRCMP_SORT;
>
> *sort |= flags;
>
> I think they end up doing the same thing, but maybe I am missing
> something.
No you're not. I did not make it absolute beautiful because I expected
it to be deleted soon after you polish your f-e-r series. Will resend
with this change shortly.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
