This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
11.01.2018 13:25 "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>
> On Thu, Jan 11, 2018 at 12:46:06PM +0100, Rafal Luzynski wrote:
> > [...]
> > Initially I had an idea to add the alternative month names but
> > initialize them all to empty strings ("") and rely on the mechanism
> > which falls back to the regular month names if the alternative ones
> > are empty. Then this has been changed to initialize them just "January",
> > "February", etc. How to name those month names? Regular? Nominative?
> > Basic? Non-alternative?
>
> Whatever you like, I'd just use "month names":
>
> * locale/C-time.c (_nl_C_LC_TIME): Update nstrings. Add alternative month
> names defined to the same string values as month names.
OK, I'll think about the proper sentence here.
>
> > > [...]
> > > > * locale/programs/ld-time.c: Alternative month names support
> > > > added, they are a copy of mon if not specified explicitly.
> > >
> > > * locale/programs/ld-time.c (struct locale_time_t): Add alt_mon,
> > > walt_mon, and alt_mon_defined members.
> > > (time_output): Output alt_mon and walt_mon members.
> > > (time_read): Read them, initialize alt_mon_defined.
> >
> > This is OK but shouldn't we mention that alt_mon content is copied
> > from mon (and walt_mon from wmon) if not specified explicitly in the
> > locale data file?
>
> Feel free to mention whatever you consider appropriate,
> my comments were mostly about the ChangeLog style.
OK, also see below.
> > > [...]
> > > > * time/strptime_l.c: Alternative month names also recognized.
> > >
> > > * time/strptime_l.c (alt_month_name): New macro.
> > > (__strptime_internal) [_LIBC]: Add ... (whatever is being added).
> > > Handle %OB format.
> >
> > I'm confused. Actually alt_month_name has been added globally
> > in the #ifdef _LIBC section so I think this makes sense:
> >
> > * time/strptime_l.c [_LIBC] (alt_month_name): New macro.
>
> Sure, you must know better, I cannot tell by looking at the patch.
While iterating over your remarks I have found more symbols which
have not been mentioned so I will review and add them. For example,
the same changes should be applied to the next patch which adds
abbreviated alternative month names. But now as I got the pattern
of the ChangeLog style then I'll just follow it rather than copy
and paste your comments blindly.
> > > > [...]
> > > > - /* Long month names. */
> > > > + /* Long month names, in the grammatical form used when the month
> > > > + forms part of a complete date. */
> > >
> > > ... when month name is a part of ...?
> >
> > Does the question mark mean that you are not sure? Well, this comment
> > has been provided by Zack Weinberg who is a native English speaker.
>
> I read this as "month is a part of a part of a complete date" which
> is too complex unless your intention is to highlight that month name
> is a subpart.
Zack has already explained this and AFAIUC he kinda agrees with you.
Best regards,
Rafal