Re: [RFC][PATCH v9 4/6] Abbreviated alternative month names (%Ob) also added (bug 10871).

[Zack Weinberg <zackw at panix dot com>]

On Tue, Sep 19, 2017 at 6:43 AM, Rafal Luzynski
<digitalfreak@lingonborough.com> wrote:
> All the previous changes (except the backward compatibility) also
> repeated to support abbreviated alternative month names.
> In most languages which have declension and need nominative/genitive
> month names the abbreviated forms for both cases are the same.
> An example where they do differ is May in Russian: this name is
> too short to be abbreviated so even the abbreviated form feature
> the declension suffixes.

Again, I'm reviewing this on the assumption that the feature has
consensus, and I'm asking Joseph to check for conformance problems.

>         [BZ #10871]
>         * locale/C-time.c: Add abbreviated alternative month names, define
>         them as the same as abbreviated month names explicitly.
>         * locale/categories.def: ab_alt_mon and wide-ab_alt_mon added.
>         * locale/langinfo.h: _NL_[W]ABALTMON_1 .. _NL_[W]ABALTMON_12
>         defined.
>         * locale/programs/ld-time.c: Abbreviated alternative month names
>         support added, they are a copy of abmon if not provided
>         explicitly.
>         * locale/programs/locfile-kw.gperf: ab_alt_mon defined.
>         * locale/programs/locfile-token.h: tok_ab_alt_mon defined.
>         * time/strftime_l.c: %Ob (%Oh) format for abbreviated
>         alternative month names added.
>         * time/strptime_l.c: Abbreviated alternative month names also
>         recognized.

Same concerns re the ChangeLog as I expressed for the earlier patch.

> --- a/time/strptime_l.c
> +++ b/time/strptime_l.c
> @@ -126,6 +126,8 @@ extern const struct __locale_data _nl_C_LC_TIME
> attribute_hidden;
>  # define ab_month_name (&_nl_C_LC_TIME.values[_NL_ITEM_INDEX (ABMON_1)].string)
>  # define alt_month_name \
>    (&_nl_C_LC_TIME.values[_NL_ITEM_INDEX (ALTMON_1)].string)
> +# define ab_alt_month_name \
> +  (&_nl_C_LC_TIME.values[_NL_ITEM_INDEX (_NL_ABALTMON_1)].string)
>  # define HERE_D_T_FMT (_nl_C_LC_TIME.values[_NL_ITEM_INDEX (D_T_FMT)].string)
>  # define HERE_D_FMT (_nl_C_LC_TIME.values[_NL_ITEM_INDEX (D_FMT)].string)
>  # define HERE_AM_STR (_nl_C_LC_TIME.values[_NL_ITEM_INDEX (AM_STR)].string)
> @@ -417,6 +419,18 @@ __strptime_internal (const char *rp, const char *fmt,
> struct tm *tmp,
>                         decided_longest = loc;
>                     }
>                   trp = rp;
> +                 if (match_string (_NL_CURRENT (LC_TIME, _NL_ABALTMON_1 + cnt),
> +                                   trp)
> +                     && trp > rp_longest)
> +                   {
> +                     rp_longest = trp;
> +                     cnt_longest = cnt;
> +                     if (s.decided == not
> +                         && strcmp (_NL_CURRENT (LC_TIME, _NL_ABALTMON_1 + cnt),
> +                                    alt_month_name[cnt]))
> +                       decided_longest = loc;
> +                   }
> +                 trp = rp;
>  #endif
>                   if (match_string (_NL_CURRENT (LC_TIME, MON_1 + cnt), trp)
>                       && trp > rp_longest)
> @@ -447,6 +461,8 @@ __strptime_internal (const char *rp, const char *fmt, struct
> tm *tmp,
>  #ifdef _LIBC
>                       || ((trp = rp, match_string (alt_month_name[cnt], trp))
>                           && trp > rp_longest)
> +                     || ((trp = rp, match_string (ab_alt_month_name[cnt], trp))
> +                         && trp > rp_longest)
>  #endif
>                       || ((trp = rp, match_string (ab_month_name[cnt], trp))
>                           && trp > rp_longest)))

This appears to make the order-of-checks inconsistency, that I
remarked on in the previous patch, even worse.  Please make sure the
checks are done in a consistent order in both places.

Otherwise looks good to me.

zw