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 v12 5/6] Documentation to the above changes (bug 10871).


15.01.2018 04:42 Rical Jasan <ricaljasan@pacific.net> wrote:
>
>
> Sorry I haven't had time to review sooner, but I was able to make some
> time today.

Thank you for your review.

> On 01/12/2018 12:18 AM, Rafal Luzynski wrote:
> > [BZ #10871]
> > * manual/locale.texi: Document ALTMON_1..12 constants for
> > nl_langinfo. Specify when to use ALTMON instead of MON.
> > * manual/time.texi (strftime, strptime): Document GNU extension
> > permitting O modifier with %B and %b. Specify when to use
> > %OB instead of %B.
> > ---
> > ChangeLog | 9 +++++++++
> > NEWS | 24 ++++++++++++++++++++++++
> > manual/locale.texi | 26 +++++++++++++++++++++++---
> > manual/time.texi | 35 +++++++++++++++++++++++++++--------
> > 4 files changed, 83 insertions(+), 11 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 75bf467..c70d8a9 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -69,6 +69,30 @@ Major new features:
> > collation ordering. Previous glibc versions used locale-specific
> > ordering, the change might break systems that relied on that.
> >
> > +* Support for two grammatical forms of month name has been added.
>
> "month names"

OK, I'm going to apply this locally.  That's why I'm asking other people
to review or even write the documentation from scratch because the details
like "month(s) name(s)" are sometimes confusing for me as a foreigner.

>
> > + In a call to strftime, the "%B" and "%b" format specifiers will now
> > + produce the grammatical form required when the month is used as part
> > + of a complete date. New "%OB" and "%Ob" specifiers produce the form
> > + required when the month is named by itself. For instance, in Greek
> > + and in many Slavic and Baltic languages, "%B" will produce the month
> > + in genitive case, and "%OB" will produce the month in nominative case.
> > +
> > + In a call to strptime, "%B", "%b", "%h", "%OB", "%Ob", and "%Oh"
> > + are all valid and will all accept any known form of month
> > + name---standalone or complete, abbreviated or full. In a call to
> > + nl_langinfo, the query constants MON_1..12 and ABMON_1..12 return
> > + the strings used by "%B" and "%b", respectively. New query
> > + constants ALTMON_1..12 and _NL_ABALTMON_1..12 return the strings
>
> It seems odd not to have ABALTMON_*. Unfortunately I didn't get to
> reviewing this sooner, and I don't want to block this, and another
> developer has OK'd it [1], but I wanted to throw in my 2 cents.
>
> ABALTMON_* is both intuitive and consistent, and I wonder how many
> people are going to try using it, only to have to go look up
> instructions on the _NL_* variant, which isn't documented at all...

It has been answered by Carlos already.  Well, the same question could be
asked about all other _NL_* constants.  Especially since they cannot be
changed in future due to ABI compatibility.  They are not documented in
the documentation but documented in the header file.

As I wrote before, the idea to standardize ABALTMON_* is very new.
Can we assume already that it will be eventually accepted even if it
takes many years and define it as a public GNU extension already?
I'm kinda tempted to say yes.  With _NL_ABALTMON_* the situation is
more complex because... here I need to put a longer story.  First of
all, we should discourage using nl_langinfo() to display month names.
Programmers should use strftime("%OB") and strftime("%Ob") even if
nl_langinfo() produces the same results more efficiently.  nl_langinfo()
should be considered a low-level API used to implement strftime().
I think I saw this recommendation somewhere so I think the idea is not
new.  But if they really want to use nl_langinfo() they should use ALTMON_*
for full month names if it is available (MON_* if it is not) and
_NL_ABALTMON_* for short month names (ABMON_* if it is not).  Recommend
to use an undocumented feature?

But OTOH I was told not to declare even ALTMON_* series as public
because it is not yet published by POSIX (even if it is accepted to
be published in future).  ABALTMON_* was not yet even filed to POSIX.

Sorry, I'm short of time so I don't provide links here.

> ...which brings up another question: why are we announcing a reserved
> name (i.e., "_*") as available for general use (and not documenting it)?

Actually they are public symbols even if unofficial and undocumented.
Would it be better to remove them from ChangeLog?  AFAIK the purpose is
to help if someone investigates in future who and why introduced these
symbols.

> > [...]
> > diff --git a/manual/locale.texi b/manual/locale.texi
> > index 60ad2a1..059db75 100644
> > --- a/manual/locale.texi
> > +++ b/manual/locale.texi
> > @@ -923,7 +923,7 @@ corresponds to Sunday.
> > @itemx DAY_5
> > @itemx DAY_6
> > @itemx DAY_7
> > -Similar to @code{ABDAY_1} etc., but here the return value is the
> > +Similar to @code{ABDAY_1} etc.,@: but here the return value is the
>
> There shouldn't be a colon immediately following the comma, but there
> should be a comma between ABDAY_1 and etc.: "Similar to @code{ABDAY_1},
> etc., but..." [...]

AFAIK it's not a colon but "@:" is an operator to force a regular space.
Otherwise there would be a full stop space automatically inserted due to
the "dot-space" sequence.

> > [...]
> > +@item ALTMON_1
> > +@itemx ALTMON_2
> > +@itemx ALTMON_3
> > +@itemx ALTMON_4
> > +@itemx ALTMON_5
> > +@itemx ALTMON_6
> > +@itemx ALTMON_7
> > +@itemx ALTMON_8
> > +@itemx ALTMON_9
> > +@itemx ALTMON_10
> > +@itemx ALTMON_11
> > +@itemx ALTMON_12
> > +Similar to @code{MON_1} etc.,@: but here the month names are in the
> > grammatical
> > +form used when the month is named by itself. The @code{strftime} functions
> > +use these month names for the format specifier @code{OB}.
>
> An "(@pxref{Formatting Calendar Time})" would be good at the end there.

It is already in other places in this document.  If you really need it here
then I'll appreciate a full example which I can just copy & paste.

> Also, I think it should read "conversion specifier @code{%OB}." In the
> strftime description, "B" is called the "format specifier", "O" is an
> "optional modifier", and "%OB" would be an example of a conversion
> specifier.

Again, I'm not a real expert to determine which term is correct here.
I will appreciate the corrections from native English speakers but I'd
like to get a consistent version.

> > [...]
> > diff --git a/manual/time.texi b/manual/time.texi
> > index 33aa221..2a5afd9 100644
> > --- a/manual/time.texi
> > +++ b/manual/time.texi
> > @@ -1346,8 +1346,13 @@ example, @code{%Ex} might yield a date format based
> > on
> > the Japanese
> > Emperors' reigns.
> >
> > @item O
> > -Use the locale's alternate numeric symbols for numbers. This modifier
> > -applies only to numeric format specifiers.
> > +With all format specifiers that produce numbers: use the locale's
> > +alternate numeric symbols.
> > +
> > +With @code{%B} and @code{%b}: use the grammatical form for month names
>
> And "h"? What about: "With the format specifiers that produce month
> names:"?

The documentation of "%h" says it works exactly the same as "%b" and I read
this as a sufficient explanation.  I can add "%h" here if you insist.
As a reader I don't like the documents which refer to other documents
(which may refer to yet other documents etc.) so I wouldn't like to see
"With the format specifiers that produce month names" (and please, readers,
read this document again if you have already forgotten which format
specifiers produce month names) :-)  So, again, at the moment I don't
change anything and I'll add "%h" if you insist.

> [...]
> The ABALTMON issue aside, I'm glad to see this patch/bugfix finally have
> gained enough consensus to be going in. Congratulations. :)
>
> Rical

Thank you again for your review.  I believe it helps me to finalize
this task.

Can I suggest that if there are no issues beyond the documentation
(yes, I know that whether we add _NL_ABALTMON_* or __ABALTMON_* and
ABALTMON_* is a serious API issue) then please let's commit this ASAP
to make sure we have those remaining 2 weeks to announce the change
to the outer world and let's polish the documentation within this
period?

Regards,

Rafal


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