This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v6 2/2] strftime: Pass the additional flags from "%EY" to "%Ey" [BZ #23758]


This review covers only the documentation and commit message.


> For the output string of the conversion specifier "%EY", an optional
> flag is given to the conversion specifier so that it can be also used
> the current non-padding format, and the padding format can be
> controlled.  To achieve this, when an optional flag is given to the
> conversion specifier "%EY", the "%Ey" included in the combined
> conversion specifier is interpreted as if decorated with the
> appropriate flag.

This is unclear.  Suggest instead:

| The full representation of the alternative calendar year (%EY)
| typically includes an internal use of %Ey.  As a GNU extension,
| apply any flags on %EY (e.g. '%-EY', '%_Ey') to the internal %Ey,
| allowing users of %EY to control how the year is padded.

(It seems to me that this extension ought to be generalized to all of
the "macro" formats (%c, %D, %F, %r, %R, %T, %x, %X, %Ec, %Ex, %EX),
and to all format flags and field widths, but that would be a separate
patch and not appropriate for 2.29 at this point.)

> Currently in glibc, besides ja_JP (Japan) locale, the locales using
> the conversion specifier "%Ey" are lo_LA (Laos) and th_TH (Thailand).
> In these locales, they use the Buddhist era.  The Buddhist era is a
> value obtained by adding 543 to the Christian era, so they are not
> affected by the change of the conversion specifier "%Ey".

Drop this paragraph, as it is already covered in the previous patch's
commit message.

>
> ChangeLog:
>
>       [BZ #23758]
>       * NEWS: Mention the change.

Changes to NEWS are not mentioned in the ChangeLog.

>       * manual/time.texi (strftime): Document the desctiption for "%EC" and
>       "%EY".

Typo: "desctiption" -> "description".  But it would be even better written

|    * manual/time.texi (strftime): Document "%EC" and "%EY".

>       If an optional flag ('_' or '-') is specified to "%EY", the "%Ey" in
>       subformat is interpreted as if decorated with the appropriate flag.

Write this sentence with an active main verb.  Also, "subformat" needs
a "the", and "the appropriate" would be better as "that", which makes
clear that it applies the *same* optional flag that was used on the %EY.

|       If an optional flag ('_' or '-') is specified to "%EY", interpret
|       the "%Ey" in the subformat as if decorated with that flag.

>  * Improve the width of alternative representation for year in
>    strftime.  For %Ey conversion specifier, the default action is now
>    to pad the number with zero to keep minimum 2 digits, similar to %y.
> +  Also, the optional flag (either _ or -) can be used for %EY, so that
> +  the internal %Ey is interpreted as if decorated with the appropriate
> +  flag.

Paul's suggested revision of this addition is technically incorrect;
he got confused about which way around the flag propagates.  I would
recommend using a separate bullet point for this change, and I would
also recommend not describing the behavior in terms of implementation
details:

| * As a GNU extension, the '-' and '_' flags can now be applied to '%EY'
|   to control how the year number is formatted; they have the same effect
|   that they would on %Ey.

>  The century of the year.  This is equivalent to the greatest integer not
>  greater than the year divided by 100.
>
> +If the @code{E} modifier is specified (@code{%EC}), the locale's
> +alternative representation for year (the era name) is used instead.

Recommend instead

| If the @code{E} modifier is specified (@code{%EC}), instead produces
| the name of the period for the current year (e.g.@: an era name) in
| the locale's alternative calendar.

> +If the @code{E} modifier is specified (@code{%EY}), the locale's
> +alternative representation for year (generally the combination of
> +@code{%EC} and @code{%Ey}) is used instead.  In this case, the
> +optional flag (either @code{_} or @code{-}) can be used, so that the
> +internal @code{%Ey} is interpreted as if decorated with the
> +appropriate flag.

Recommend instead

| If the @code{E} modifier is specified (@code{%EY}), instead produces
| a complete representation of the year according to the locale's
| alternative calendar.  Generally this will be some combination of
| the information produced by @code{%EC} and @code{Ey}.  As a GNU
| extension, the formatting flags @code{_} or @code{-} may be used
| with this conversion specifier; they affect how the year number is
| printed.

zw


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]