public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v11 0/5][BZ 10871] Month names in alternative grammatical case
@ 2018-01-08 23:54 Rafal Luzynski
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-08 23:54 UTC (permalink / raw)
  To: libc-alpha

As requested by Carlos, [1] this is the 11th version of my patches.
This version is actually the same as the previous one, [2] it is only
rebased to the current master.

Again, note that this set contains only patches 1/5, 3/5, and 5/5.
The patches 2/5 and 4/5 contain updates to the generated files and
ChangeLog, they should be eventually squashed but not posted to this list.

As we are short of time, these patches are not attached to bugzilla. [3]

Additionally, I'm adding the locale data patch for pl_PL.  It may be
considered as a separate patch but I am going to push it as soon as all
other patches are accepted.  This patch is already accepted by the Polish
translation team.  Note that the change introduced by this set of patches
will be visible only in locales which actually contain different "mon"
and "alt_mon" arrays.

Regards,

Rafal


[1] https://sourceware.org/ml/libc-alpha/2018-01/msg00257.html
[2] https://sourceware.org/ml/libc-alpha/2017-11/msg00569.html
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=10871

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-08 23:54 [PATCH v11 0/5][BZ 10871] Month names in alternative grammatical case Rafal Luzynski
@ 2018-01-08 23:59 ` Rafal Luzynski
  2018-01-09  0:14   ` Rafal Luzynski
                     ` (4 more replies)
  2018-01-09  0:01 ` [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added " Rafal Luzynski
                   ` (2 subsequent siblings)
  3 siblings, 5 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-08 23:59 UTC (permalink / raw)
  To: libc-alpha

Some languages (Slavic, Baltic, etc.) require a genitive case of the
month name when formatting a full date (with the day number) while
they require a nominative case when referring to the month standalone.
This requirement cannot be fulfilled without providing two forms for
each month name.  From now it is precised that nl_langinfo(MON_1)
series (up to MON_12) and strftime("%B") generate the month names in
the grammatical form used when the month forms part of a complete date.
If the grammatical form used when the month is named by itself is needed,
the new values nl_langinfo(ALTMON_1) (up to ALTMON_12) and
strftime("%OB") are supported.  This new feature is optional so the
languages which do not need it or do not yet provide the updated
locales simply do not use it and their behaviour is unchanged.

	[BZ #10871]
	* locale/C-time.c: Add alternative month names, define them as the
	same as mon explicitly.
	* locale/categories.def: alt_mon and wide-alt_mon added.
	* locale/langinfo.h [__USE_GNU]: New public symbols ALTMON_1,
	ALTMON_2, ALTMON_3, ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7,
	ALTMON_8, ALTMON_9, ALTMON_10, ALTMON_11, ALTMON_12,
	_NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3, _NL_WALTMON_4,
	_NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7, _NL_WALTMON_8,
	_NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11, _NL_WALTMON_12.
	* locale/programs/ld-time.c: Alternative month names support
	added, they are a copy of mon if not specified explicitly.
	* locale/programs/locfile-kw.gperf: alt_mon defined.
	* locale/programs/locfile-token.h: tok_alt_mon defined.
	* localedata/tst-langinfo.c: Add tests for the new constants
	ALTMON_1 .. ALTMON_12.
	* time/Makefile (LOCALES): Add pl_PL.UTF-8 for tests.
	* time/strftime_l.c: %OB format for alternative month names
	added.
	* time/strptime_l.c: Alternative month names also recognized.
	* time/tst-strptime.c: Add tests to parse different forms of
	month names including the new %OB format specifier.
---
 locale/C-time.c                  | 28 ++++++++++++++++++++--
 locale/categories.def            |  2 ++
 locale/langinfo.h                | 50 ++++++++++++++++++++++++++++++++++++++--
 locale/programs/ld-time.c        | 21 +++++++++++++++++
 locale/programs/locfile-kw.gperf |  1 +
 locale/programs/locfile-token.h  |  1 +
 localedata/tst-langinfo.c        | 12 ++++++++++
 time/Makefile                    |  2 +-
 time/strftime_l.c                | 11 +++++++--
 time/strptime_l.c                | 32 +++++++++++++++++++++----
 time/tst-strptime.c              |  6 +++++
 11 files changed, 155 insertions(+), 11 deletions(-)

diff --git a/locale/C-time.c b/locale/C-time.c
index 1f1ee01..73bc700 100644
--- a/locale/C-time.c
+++ b/locale/C-time.c
@@ -30,7 +30,7 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
   { NULL, },			/* no cached data */
   UNDELETABLE,
   0,
-  111,
+  135,
   {
     { .string = "Sun" },
     { .string = "Mon" },
@@ -142,6 +142,30 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
     { .string = "" },
     { .string = "%a %b %e %H:%M:%S %Z %Y" },
     { .wstr = (const uint32_t *) L"%a %b %e %H:%M:%S %Z %Y" },
-    { .string = _nl_C_codeset }
+    { .string = _nl_C_codeset },
+    { .string = "January" },
+    { .string = "February" },
+    { .string = "March" },
+    { .string = "April" },
+    { .string = "May" },
+    { .string = "June" },
+    { .string = "July" },
+    { .string = "August" },
+    { .string = "September" },
+    { .string = "October" },
+    { .string = "November" },
+    { .string = "December" },
+    { .wstr = (const uint32_t *) L"January" },
+    { .wstr = (const uint32_t *) L"February" },
+    { .wstr = (const uint32_t *) L"March" },
+    { .wstr = (const uint32_t *) L"April" },
+    { .wstr = (const uint32_t *) L"May" },
+    { .wstr = (const uint32_t *) L"June" },
+    { .wstr = (const uint32_t *) L"July" },
+    { .wstr = (const uint32_t *) L"August" },
+    { .wstr = (const uint32_t *) L"September" },
+    { .wstr = (const uint32_t *) L"October" },
+    { .wstr = (const uint32_t *) L"November" },
+    { .wstr = (const uint32_t *) L"December" }
   }
 };
diff --git a/locale/categories.def b/locale/categories.def
index 47947f7..3cbb4e6 100644
--- a/locale/categories.def
+++ b/locale/categories.def
@@ -249,6 +249,8 @@ DEFINE_CATEGORY
   DEFINE_ELEMENT (_DATE_FMT,                "date_fmt",            opt, string)
   DEFINE_ELEMENT (_NL_W_DATE_FMT,           "wide-date_fmt",       opt,
wstring)
   DEFINE_ELEMENT (_NL_TIME_CODESET,	    "time-codeset",	   std, string)
+  DEFINE_ELEMENT (ALTMON_1,       "alt_mon",       opt, stringarray, 12, 12)
+  DEFINE_ELEMENT (_NL_WALTMON_1,  "wide-alt_mon",  opt, wstringarray, 12, 12)
   ), NO_POSTLOAD)
 
 
diff --git a/locale/langinfo.h b/locale/langinfo.h
index 28a0a73..0fbd838 100644
--- a/locale/langinfo.h
+++ b/locale/langinfo.h
@@ -100,7 +100,8 @@ enum
   ABMON_12,
 #define ABMON_12		ABMON_12
 
-  /* Long month names.  */
+  /* Long month names, in the grammatical form used when the month
+     forms part of a complete date.  */
   MON_1,			/* January */
 #define MON_1			MON_1
   MON_2,
@@ -189,7 +190,8 @@ enum
   _NL_WABMON_11,
   _NL_WABMON_12,
 
-  /* Long month names.  */
+  /* Long month names, in the grammatical form used when the month
+     forms part of a complete date.  */
   _NL_WMON_1,		/* January */
   _NL_WMON_2,
   _NL_WMON_3,
@@ -231,6 +233,50 @@ enum
 
   _NL_TIME_CODESET,
 
+  /* Long month names, in the grammatical form used when the month
+     is named by itself.  */
+  __ALTMON_1,			/* January */
+  __ALTMON_2,
+  __ALTMON_3,
+  __ALTMON_4,
+  __ALTMON_5,
+  __ALTMON_6,
+  __ALTMON_7,
+  __ALTMON_8,
+  __ALTMON_9,
+  __ALTMON_10,
+  __ALTMON_11,
+  __ALTMON_12,
+#ifdef __USE_GNU
+# define ALTMON_1		__ALTMON_1
+# define ALTMON_2		__ALTMON_2
+# define ALTMON_3		__ALTMON_3
+# define ALTMON_4		__ALTMON_4
+# define ALTMON_5		__ALTMON_5
+# define ALTMON_6		__ALTMON_6
+# define ALTMON_7		__ALTMON_7
+# define ALTMON_8		__ALTMON_8
+# define ALTMON_9		__ALTMON_9
+# define ALTMON_10		__ALTMON_10
+# define ALTMON_11		__ALTMON_11
+# define ALTMON_12		__ALTMON_12
+#endif
+
+  /* Long month names, in the grammatical form used when the month
+     is named by itself.  */
+  _NL_WALTMON_1,			/* January */
+  _NL_WALTMON_2,
+  _NL_WALTMON_3,
+  _NL_WALTMON_4,
+  _NL_WALTMON_5,
+  _NL_WALTMON_6,
+  _NL_WALTMON_7,
+  _NL_WALTMON_8,
+  _NL_WALTMON_9,
+  _NL_WALTMON_10,
+  _NL_WALTMON_11,
+  _NL_WALTMON_12,
+
   _NL_NUM_LC_TIME,	/* Number of indices in LC_TIME category.  */
 
   /* LC_COLLATE category: text sorting.
diff --git a/locale/programs/ld-time.c b/locale/programs/ld-time.c
index 67d055a..4186448 100644
--- a/locale/programs/ld-time.c
+++ b/locale/programs/ld-time.c
@@ -91,6 +91,9 @@ struct locale_time_t
   const char *date_fmt;
   const uint32_t *wdate_fmt;
   int alt_digits_defined;
+  const char *alt_mon[12];
+  const uint32_t *walt_mon[12];
+  int alt_mon_defined;
   unsigned char week_ndays;
   uint32_t week_1stday;
   unsigned char week_1stweek;
@@ -639,6 +642,15 @@ time_output (struct localedef_t *locale, const struct
charmap_t *charmap,
   add_locale_string (&file, time->date_fmt);
   add_locale_wstring (&file, time->wdate_fmt);
   add_locale_string (&file, charmap->code_set_name);
+
+  /* The alt'mons.  */
+  for (n = 0; n < 12; ++n)
+    add_locale_string (&file, time->alt_mon[n] ?: "");
+
+  /* The wide character alt'mons.  */
+  for (n = 0; n < 12; ++n)
+    add_locale_wstring (&file, time->walt_mon[n] ?: empty_wstr);
+
   write_locale_data (output_path, LC_TIME, "LC_TIME", &file);
 }
 
@@ -782,6 +794,7 @@ time_read (struct linereader *ldfile, struct localedef_t
*result,
 	  STRARR_ELEM (mon, 12, 12);
 	  STRARR_ELEM (am_pm, 2, 2);
 	  STRARR_ELEM (alt_digits, 0, 100);
+	  STRARR_ELEM (alt_mon, 12, 12);
 
 	case tok_era:
 	  /* Ignore the rest of the line if we don't need the input of
@@ -934,6 +947,14 @@ time_read (struct linereader *ldfile, struct localedef_t
*result,
 	    lr_error (ldfile, _("\
 %1$s: definition does not end with `END %1$s'"), "LC_TIME");
 	  lr_ignore_rest (ldfile, now->tok == tok_lc_time);
+
+	  /* If alt_mon was not specified, make it a copy of mon.  */
+	  if (!ignore_content && !time->alt_mon_defined)
+	    {
+	      memcpy (time->alt_mon, time->mon, sizeof (time->mon));
+	      memcpy (time->walt_mon, time->wmon, sizeof (time->wmon));
+	      time->alt_mon_defined = 1;
+	    }
 	  return;
 
 	default:
diff --git a/locale/programs/locfile-kw.gperf b/locale/programs/locfile-kw.gperf
index c74c1f2..dad7f21 100644
--- a/locale/programs/locfile-kw.gperf
+++ b/locale/programs/locfile-kw.gperf
@@ -148,6 +148,7 @@ first_workday,          tok_first_workday,          0
 cal_direction,          tok_cal_direction,          0
 timezone,               tok_timezone,               0
 date_fmt,               tok_date_fmt,               0
+alt_mon,                tok_alt_mon,                0
 LC_MESSAGES,            tok_lc_messages,            0
 yesexpr,                tok_yesexpr,                0
 noexpr,                 tok_noexpr,                 0
diff --git a/locale/programs/locfile-token.h b/locale/programs/locfile-token.h
index f02325d..d49da5e 100644
--- a/locale/programs/locfile-token.h
+++ b/locale/programs/locfile-token.h
@@ -186,6 +186,7 @@ enum token_t
   tok_cal_direction,
   tok_timezone,
   tok_date_fmt,
+  tok_alt_mon,
   tok_lc_messages,
   tok_yesexpr,
   tok_noexpr,
diff --git a/localedata/tst-langinfo.c b/localedata/tst-langinfo.c
index 8c3667c..0d33e75 100644
--- a/localedata/tst-langinfo.c
+++ b/localedata/tst-langinfo.c
@@ -50,6 +50,18 @@ struct map
   VAL (ABMON_8),
   VAL (ABMON_9),
   VAL (ALT_DIGITS),
+  VAL (ALTMON_1),
+  VAL (ALTMON_10),
+  VAL (ALTMON_11),
+  VAL (ALTMON_12),
+  VAL (ALTMON_2),
+  VAL (ALTMON_3),
+  VAL (ALTMON_4),
+  VAL (ALTMON_5),
+  VAL (ALTMON_6),
+  VAL (ALTMON_7),
+  VAL (ALTMON_8),
+  VAL (ALTMON_9),
   VAL (AM_STR),
   VAL (CRNCYSTR),
   VAL (CURRENCY_SYMBOL),
diff --git a/time/Makefile b/time/Makefile
index 264eed9..91adcd0 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -48,7 +48,7 @@ tests	:= test_time clocktest tst-posixtz tst-strptime
tst_wcsftime \
 include ../Rules
 
 ifeq ($(run-built-tests),yes)
-LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP
+LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP pl_PL.UTF-8
 include ../gen-locales.mk
 
 $(objpfx)tst-ftime_l.out: $(gen-locales)
diff --git a/time/strftime_l.c b/time/strftime_l.c
index 18651ff..ac5d28f 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -492,6 +492,9 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
*format,
 # define f_month \
   ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
 		     ? "?" : _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon)))
+# define f_altmonth \
+  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
+		     ? "?" : _NL_CURRENT (LC_TIME, NLW(ALTMON_1) + tp->tm_mon)))
 # define ampm \
   ((const CHAR_T *) _NL_CURRENT (LC_TIME, tp->tm_hour > 11		      \
 				 ? NLW(PM_STR) : NLW(AM_STR)))
@@ -507,6 +510,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
*format,
 		   ? "?" : month_name[tp->tm_mon])
 #  define a_wkday f_wkday
 #  define a_month f_month
+#  define f_altmonth f_month
 #  define ampm (L_("AMPM") + 2 * (tp->tm_hour > 11))
 
   size_t aw_len = 3;
@@ -785,7 +789,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
*format,
 #endif
 
 	case L_('B'):
-	  if (modifier != 0)
+	  if (modifier == L_('E'))
 	    goto bad_format;
 	  if (change_case)
 	    {
@@ -793,7 +797,10 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const
CHAR_T *format,
 	      to_lowcase = 0;
 	    }
 #if defined _NL_CURRENT || !HAVE_STRFTIME
-	  cpy (STRLEN (f_month), f_month);
+	  if (modifier == L_('O'))
+	    cpy (STRLEN (f_altmonth), f_altmonth);
+	  else
+	    cpy (STRLEN (f_month), f_month);
 	  break;
 #else
 	  goto underlying_strftime;
diff --git a/time/strptime_l.c b/time/strptime_l.c
index 7d4758e..39cf38d 100644
--- a/time/strptime_l.c
+++ b/time/strptime_l.c
@@ -124,6 +124,8 @@ extern const struct __locale_data _nl_C_LC_TIME
attribute_hidden;
   (&_nl_C_LC_TIME.values[_NL_ITEM_INDEX (ABDAY_1)].string)
 # define month_name (&_nl_C_LC_TIME.values[_NL_ITEM_INDEX (MON_1)].string)
 # 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 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)
@@ -319,10 +321,9 @@ __strptime_internal (const char *rp, const char *fmt,
struct tm *tmp,
       while (*fmt >= '0' && *fmt <= '9')
 	++fmt;
 
-#ifndef _NL_CURRENT
-      /* We need this for handling the `E' modifier.  */
+      /* In some cases, modifiers are handled by adjusting state and
+         then restarting the switch statement below.  */
     start_over:
-#endif
 
       /* Make back up of current processing pointer.  */
       rp_backup = rp;
@@ -423,13 +424,32 @@ __strptime_internal (const char *rp, const char *fmt,
struct tm *tmp,
 				     ab_month_name[cnt]))
 			decided_longest = loc;
 		    }
+#ifdef _LIBC
+		  /* Now check the alt month.  */
+		  trp = rp;
+		  if (match_string (_NL_CURRENT (LC_TIME, ALTMON_1 + cnt), trp)
+		      && trp > rp_longest)
+		    {
+		      rp_longest = trp;
+		      cnt_longest = cnt;
+		      if (s.decided == not
+			  && strcmp (_NL_CURRENT (LC_TIME, ALTMON_1 + cnt),
+				     alt_month_name[cnt]))
+			decided_longest = loc;
+		    }
+#endif
 		}
 #endif
 	      if (s.decided != loc
 		  && (((trp = rp, match_string (month_name[cnt], trp))
 		       && trp > rp_longest)
 		      || ((trp = rp, match_string (ab_month_name[cnt], trp))
-			  && trp > rp_longest)))
+			  && trp > rp_longest)
+#ifdef _LIBC
+		      || ((trp = rp, match_string (alt_month_name[cnt], trp))
+			  && trp > rp_longest)
+#endif
+	      ))
 		{
 		  rp_longest = trp;
 		  cnt_longest = cnt;
@@ -1015,6 +1035,10 @@ __strptime_internal (const char *rp, const char *fmt,
struct tm *tmp,
 	case 'O':
 	  switch (*fmt++)
 	    {
+	    case 'B':
+	      /* Match month name.  Reprocess as plain 'B'.  */
+	      fmt--;
+	      goto start_over;
 	    case 'd':
 	    case 'e':
 	      /* Match day of month using alternate numeric symbols.  */
diff --git a/time/tst-strptime.c b/time/tst-strptime.c
index 34ad797..bbc1390 100644
--- a/time/tst-strptime.c
+++ b/time/tst-strptime.c
@@ -51,6 +51,12 @@ static const struct
     6, 0, 0, 1 },
   { "ja_JP.EUC-JP", "2001 20 \xb7\xee", "%Y %U %a", 1, 140, 4, 21 },
   { "ja_JP.EUC-JP", "2001 21 \xb7\xee", "%Y %W %a", 1, 140, 4, 21 },
+  { "pl_PL.UTF-8", "21 lis 2017", "%d %b %Y", 2, 324, 10, 21 },
+  { "pl_PL.UTF-8", "22 LIS 2017", "%d %B %Y", 3, 325, 10, 22 },
+  /* TODO: Use the genitive case here as soon as it is added to localedata.  */
+  { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
+  /* The nominative case is incorrect here but it is parseable.  */
+  { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
 };
 
 
-- 
2.7.5

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added (bug 10871).
  2018-01-08 23:54 [PATCH v11 0/5][BZ 10871] Month names in alternative grammatical case Rafal Luzynski
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
@ 2018-01-09  0:01 ` Rafal Luzynski
  2018-01-11  5:05   ` Carlos O'Donell
  2018-01-09  0:03 ` [PATCH v11 5/5] Documentation to the above changes " Rafal Luzynski
  2018-01-09  0:05 ` [PATCH] pl_PL: Add alternative month names " Rafal Luzynski
  3 siblings, 1 reply; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-09  0:01 UTC (permalink / raw)
  To: libc-alpha

All the previous changes 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
features the declension suffixes.

	[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: New public symbols _NL_ABALTMON_1,
	_NL_ABALTMON_2, _NL_ABALTMON_3, _NL_ABALTMON_4, _NL_ABALTMON_5,
	_NL_ABALTMON_6, _NL_ABALTMON_7, _NL_ABALTMON_8, _NL_ABALTMON_9,
	_NL_ABALTMON_10, _NL_ABALTMON_11, _NL_ABALTMON_12,
	_NL_WABALTMON_1, _NL_WABALTMON_2, _NL_WABALTMON_3, _NL_WABALTMON_4,
	_NL_WABALTMON_5, _NL_WABALTMON_6, _NL_WABALTMON_7, _NL_WABALTMON_8,
	_NL_WABALTMON_9, _NL_WABALTMON_10, _NL_WABALTMON_11, _NL_WABALTMON_12.
	* 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/Makefile (LOCALES): Add ru_RU.UTF-8 for tests.
	* time/strftime_l.c: %Ob (%Oh) format for abbreviated
	alternative month names added.
	* time/strptime_l.c: Abbreviated alternative month names also
	recognized.
	* time/tst-strptime.c: Add more tests to parse different forms of
	month names including the new %Ob format specifier.
---
 locale/C-time.c                  | 28 ++++++++++++++++++++++++++--
 locale/categories.def            |  6 ++++--
 locale/langinfo.h                | 36 ++++++++++++++++++++++++++++++++++--
 locale/programs/ld-time.c        | 19 +++++++++++++++++++
 locale/programs/locfile-kw.gperf |  1 +
 locale/programs/locfile-token.h  |  1 +
 time/Makefile                    |  3 ++-
 time/strftime_l.c                | 14 ++++++++++++--
 time/strptime_l.c                | 18 ++++++++++++++++++
 time/tst-strptime.c              | 13 +++++++++++++
 10 files changed, 130 insertions(+), 9 deletions(-)

diff --git a/locale/C-time.c b/locale/C-time.c
index 73bc700..e2b3b17 100644
--- a/locale/C-time.c
+++ b/locale/C-time.c
@@ -30,7 +30,7 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
   { NULL, },			/* no cached data */
   UNDELETABLE,
   0,
-  135,
+  159,
   {
     { .string = "Sun" },
     { .string = "Mon" },
@@ -166,6 +166,30 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
     { .wstr = (const uint32_t *) L"September" },
     { .wstr = (const uint32_t *) L"October" },
     { .wstr = (const uint32_t *) L"November" },
-    { .wstr = (const uint32_t *) L"December" }
+    { .wstr = (const uint32_t *) L"December" },
+    { .string = "Jan" },
+    { .string = "Feb" },
+    { .string = "Mar" },
+    { .string = "Apr" },
+    { .string = "May" },
+    { .string = "Jun" },
+    { .string = "Jul" },
+    { .string = "Aug" },
+    { .string = "Sep" },
+    { .string = "Oct" },
+    { .string = "Nov" },
+    { .string = "Dec" },
+    { .wstr = (const uint32_t *) L"Jan" },
+    { .wstr = (const uint32_t *) L"Feb" },
+    { .wstr = (const uint32_t *) L"Mar" },
+    { .wstr = (const uint32_t *) L"Apr" },
+    { .wstr = (const uint32_t *) L"May" },
+    { .wstr = (const uint32_t *) L"Jun" },
+    { .wstr = (const uint32_t *) L"Jul" },
+    { .wstr = (const uint32_t *) L"Aug" },
+    { .wstr = (const uint32_t *) L"Sep" },
+    { .wstr = (const uint32_t *) L"Oct" },
+    { .wstr = (const uint32_t *) L"Nov" },
+    { .wstr = (const uint32_t *) L"Dec" }
   }
 };
diff --git a/locale/categories.def b/locale/categories.def
index 3cbb4e6..56c5f88 100644
--- a/locale/categories.def
+++ b/locale/categories.def
@@ -249,8 +249,10 @@ DEFINE_CATEGORY
   DEFINE_ELEMENT (_DATE_FMT,                "date_fmt",            opt, string)
   DEFINE_ELEMENT (_NL_W_DATE_FMT,           "wide-date_fmt",       opt,
wstring)
   DEFINE_ELEMENT (_NL_TIME_CODESET,	    "time-codeset",	   std, string)
-  DEFINE_ELEMENT (ALTMON_1,       "alt_mon",       opt, stringarray, 12, 12)
-  DEFINE_ELEMENT (_NL_WALTMON_1,  "wide-alt_mon",  opt, wstringarray, 12, 12)
+  DEFINE_ELEMENT (ALTMON_1,        "alt_mon",         opt, stringarray,  12,
12)
+  DEFINE_ELEMENT (_NL_WALTMON_1,   "wide-alt_mon",    opt, wstringarray, 12,
12)
+  DEFINE_ELEMENT (_NL_ABALTMON_1,  "ab_alt_mon",      opt, stringarray,  12,
12)
+  DEFINE_ELEMENT (_NL_WABALTMON_1, "wide-ab_alt_mon", opt, wstringarray, 12,
12)
   ), NO_POSTLOAD)
 
 
diff --git a/locale/langinfo.h b/locale/langinfo.h
index 0fbd838..4749e9d 100644
--- a/locale/langinfo.h
+++ b/locale/langinfo.h
@@ -74,7 +74,8 @@ enum
   DAY_7,			/* Saturday */
 #define DAY_7			DAY_7
 
-  /* Abbreviated month names.  */
+  /* Abbreviated month names, in the grammatical form used when the month
+     forms part of a complete date.  */
   ABMON_1,			/* Jan */
 #define ABMON_1			ABMON_1
   ABMON_2,
@@ -176,7 +177,8 @@ enum
   _NL_WDAY_6,		/* Friday */
   _NL_WDAY_7,		/* Saturday */
 
-  /* Abbreviated month names.  */
+  /* Abbreviated month names, in the grammatical form used when the month
+     forms part of a complete date.  */
   _NL_WABMON_1,		/* Jan */
   _NL_WABMON_2,
   _NL_WABMON_3,
@@ -277,6 +279,36 @@ enum
   _NL_WALTMON_11,
   _NL_WALTMON_12,
 
+  /* Abbreviated month names, in the grammatical form used when the month
+     is named by itself.  */
+  _NL_ABALTMON_1,			/* Jan */
+  _NL_ABALTMON_2,
+  _NL_ABALTMON_3,
+  _NL_ABALTMON_4,
+  _NL_ABALTMON_5,
+  _NL_ABALTMON_6,
+  _NL_ABALTMON_7,
+  _NL_ABALTMON_8,
+  _NL_ABALTMON_9,
+  _NL_ABALTMON_10,
+  _NL_ABALTMON_11,
+  _NL_ABALTMON_12,
+
+  /* Abbreviated month names, in the grammatical form used when the month
+     is named by itself.  */
+  _NL_WABALTMON_1,			/* Jan */
+  _NL_WABALTMON_2,
+  _NL_WABALTMON_3,
+  _NL_WABALTMON_4,
+  _NL_WABALTMON_5,
+  _NL_WABALTMON_6,
+  _NL_WABALTMON_7,
+  _NL_WABALTMON_8,
+  _NL_WABALTMON_9,
+  _NL_WABALTMON_10,
+  _NL_WABALTMON_11,
+  _NL_WABALTMON_12,
+
   _NL_NUM_LC_TIME,	/* Number of indices in LC_TIME category.  */
 
   /* LC_COLLATE category: text sorting.
diff --git a/locale/programs/ld-time.c b/locale/programs/ld-time.c
index 4186448..a755792 100644
--- a/locale/programs/ld-time.c
+++ b/locale/programs/ld-time.c
@@ -94,6 +94,9 @@ struct locale_time_t
   const char *alt_mon[12];
   const uint32_t *walt_mon[12];
   int alt_mon_defined;
+  const char *ab_alt_mon[12];
+  const uint32_t *wab_alt_mon[12];
+  int ab_alt_mon_defined;
   unsigned char week_ndays;
   uint32_t week_1stday;
   unsigned char week_1stweek;
@@ -651,6 +654,14 @@ time_output (struct localedef_t *locale, const struct
charmap_t *charmap,
   for (n = 0; n < 12; ++n)
     add_locale_wstring (&file, time->walt_mon[n] ?: empty_wstr);
 
+  /* The ab'alt'mons.  */
+  for (n = 0; n < 12; ++n)
+    add_locale_string (&file, time->ab_alt_mon[n] ?: "");
+
+  /* The wide character ab'alt'mons.  */
+  for (n = 0; n < 12; ++n)
+    add_locale_wstring (&file, time->wab_alt_mon[n] ?: empty_wstr);
+
   write_locale_data (output_path, LC_TIME, "LC_TIME", &file);
 }
 
@@ -795,6 +806,7 @@ time_read (struct linereader *ldfile, struct localedef_t
*result,
 	  STRARR_ELEM (am_pm, 2, 2);
 	  STRARR_ELEM (alt_digits, 0, 100);
 	  STRARR_ELEM (alt_mon, 12, 12);
+	  STRARR_ELEM (ab_alt_mon, 12, 12);
 
 	case tok_era:
 	  /* Ignore the rest of the line if we don't need the input of
@@ -955,6 +967,13 @@ time_read (struct linereader *ldfile, struct localedef_t
*result,
 	      memcpy (time->walt_mon, time->wmon, sizeof (time->wmon));
 	      time->alt_mon_defined = 1;
 	    }
+	  /* The same for abbreviated versions.  */
+	  if (!ignore_content && !time->ab_alt_mon_defined)
+	    {
+	      memcpy (time->ab_alt_mon, time->abmon, sizeof (time->abmon));
+	      memcpy (time->wab_alt_mon, time->wabmon, sizeof (time->wabmon));
+	      time->ab_alt_mon_defined = 1;
+	    }
 	  return;
 
 	default:
diff --git a/locale/programs/locfile-kw.gperf b/locale/programs/locfile-kw.gperf
index dad7f21..6bf2f60 100644
--- a/locale/programs/locfile-kw.gperf
+++ b/locale/programs/locfile-kw.gperf
@@ -149,6 +149,7 @@ cal_direction,          tok_cal_direction,          0
 timezone,               tok_timezone,               0
 date_fmt,               tok_date_fmt,               0
 alt_mon,                tok_alt_mon,                0
+ab_alt_mon,             tok_ab_alt_mon,             0
 LC_MESSAGES,            tok_lc_messages,            0
 yesexpr,                tok_yesexpr,                0
 noexpr,                 tok_noexpr,                 0
diff --git a/locale/programs/locfile-token.h b/locale/programs/locfile-token.h
index d49da5e..e3cd18e 100644
--- a/locale/programs/locfile-token.h
+++ b/locale/programs/locfile-token.h
@@ -187,6 +187,7 @@ enum token_t
   tok_timezone,
   tok_date_fmt,
   tok_alt_mon,
+  tok_ab_alt_mon,
   tok_lc_messages,
   tok_yesexpr,
   tok_noexpr,
diff --git a/time/Makefile b/time/Makefile
index 91adcd0..4e631a1 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -48,7 +48,8 @@ tests	:= test_time clocktest tst-posixtz tst-strptime
tst_wcsftime \
 include ../Rules
 
 ifeq ($(run-built-tests),yes)
-LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP pl_PL.UTF-8
+LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP pl_PL.UTF-8 \
+	   ru_RU.UTF-8
 include ../gen-locales.mk
 
 $(objpfx)tst-ftime_l.out: $(gen-locales)
diff --git a/time/strftime_l.c b/time/strftime_l.c
index ac5d28f..c71f9f4 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -106,6 +106,7 @@ extern char *tzname[];
 # define UCHAR_T unsigned char
 # define L_(Str) Str
 # define NLW(Sym) Sym
+# define ABALTMON_1 _NL_ABALTMON_1
 
 # if !defined STDC_HEADERS && !defined HAVE_MEMCPY
 #  define MEMCPY(d, s, n) bcopy ((s), (d), (n))
@@ -492,6 +493,9 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
*format,
 # define f_month \
   ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
 		     ? "?" : _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon)))
+# define a_altmonth \
+  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
+		     ? "?" : _NL_CURRENT (LC_TIME, NLW(ABALTMON_1) + tp->tm_mon)))
 # define f_altmonth \
   ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
 		     ? "?" : _NL_CURRENT (LC_TIME, NLW(ALTMON_1) + tp->tm_mon)))
@@ -501,6 +505,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
*format,
 
 # define aw_len STRLEN (a_wkday)
 # define am_len STRLEN (a_month)
+# define aam_len STRLEN (a_altmonth)
 # define ap_len STRLEN (ampm)
 #else
 # if !HAVE_STRFTIME
@@ -510,11 +515,13 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const
CHAR_T *format,
 		   ? "?" : month_name[tp->tm_mon])
 #  define a_wkday f_wkday
 #  define a_month f_month
+#  define a_altmonth a_month
 #  define f_altmonth f_month
 #  define ampm (L_("AMPM") + 2 * (tp->tm_hour > 11))
 
   size_t aw_len = 3;
   size_t am_len = 3;
+  size_t aam_len = 3;
   size_t ap_len = 2;
 # endif
 #endif
@@ -779,10 +786,13 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const
CHAR_T *format,
 	      to_uppcase = 1;
 	      to_lowcase = 0;
 	    }
-	  if (modifier != 0)
+	  if (modifier == L_('E'))
 	    goto bad_format;
 #if defined _NL_CURRENT || !HAVE_STRFTIME
-	  cpy (am_len, a_month);
+	  if (modifier == L_('O'))
+	    cpy (aam_len, a_altmonth);
+	  else
+	    cpy (am_len, a_month);
 	  break;
 #else
 	  goto underlying_strftime;
diff --git a/time/strptime_l.c b/time/strptime_l.c
index 39cf38d..cd901c2 100644
--- 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)
@@ -437,6 +439,18 @@ __strptime_internal (const char *rp, const char *fmt,
struct tm *tmp,
 				     alt_month_name[cnt]))
 			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;
+		    }
 #endif
 		}
 #endif
@@ -448,6 +462,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
 	      ))
 		{
@@ -1035,7 +1051,9 @@ __strptime_internal (const char *rp, const char *fmt,
struct tm *tmp,
 	case 'O':
 	  switch (*fmt++)
 	    {
+	    case 'b':
 	    case 'B':
+	    case 'h':
 	      /* Match month name.  Reprocess as plain 'B'.  */
 	      fmt--;
 	      goto start_over;
diff --git a/time/tst-strptime.c b/time/tst-strptime.c
index bbc1390..ab09f0f 100644
--- a/time/tst-strptime.c
+++ b/time/tst-strptime.c
@@ -24,6 +24,11 @@
 #include <time.h>
 
 
+/* Some Cyrillic letters in UTF-8.  */
+#define CYR_n  "\xd0\xbd"
+#define CYR_o  "\xd0\xbe"
+#define CYR_ya "\xd1\x8f"
+
 static const struct
 {
   const char *locale;
@@ -57,6 +62,14 @@ static const struct
   { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
   /* The nominative case is incorrect here but it is parseable.  */
   { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
+  { "pl_PL.UTF-8", "25 lis 2017", "%d %Ob %Y", 6, 328, 10, 25 },
+  { "ru_RU.UTF-8", "26 " CYR_n CYR_o CYR_ya " 2017", "%d %b %Y",
+     0, 329, 10, 26 },
+  /* TODO: Add an example of "may"/"maya" (5th month, May) using %Ob in
+     Russian when the localedata is updated.  Without the genitive forms
+     in localedata the word "maya" is ambiguous and may be mistaken for
+     "mart" (March).
+   */
 };
 
 
-- 
2.7.5

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v11 5/5] Documentation to the above changes (bug 10871).
  2018-01-08 23:54 [PATCH v11 0/5][BZ 10871] Month names in alternative grammatical case Rafal Luzynski
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
  2018-01-09  0:01 ` [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added " Rafal Luzynski
@ 2018-01-09  0:03 ` Rafal Luzynski
  2018-01-11  4:50   ` Carlos O'Donell
  2018-01-09  0:05 ` [PATCH] pl_PL: Add alternative month names " Rafal Luzynski
  3 siblings, 1 reply; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-09  0:03 UTC (permalink / raw)
  To: libc-alpha

	[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.
+  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
+  used by "%OB" and "%Ob", respectively.
+
+  In a locale definition file, use "alt_mon" and "ab_alt_mon" to
+  define the strings for %OB and %Ob, respectively; these have the
+  same syntax as "mon" and "ab_mon".
+
+  This feature is currently a GNU extension, but it is expected to
+  be added to the next revision of POSIX, and it is also already
+  available on some BSD-derived operating systems.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * Support for statically linked applications which call dlopen is deprecated
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
 unabbreviated weekday name.
 @item ABMON_1
 @itemx ABMON_2
@@ -937,7 +937,8 @@ unabbreviated weekday name.
 @itemx ABMON_10
 @itemx ABMON_11
 @itemx ABMON_12
-The return value is abbreviated name of the month.  @code{ABMON_1}
+The return value is the abbreviated name of the month, in the grammatical
+form used when the month forms part of a complete date.  @code{ABMON_1}
 corresponds to January.
 @item MON_1
 @itemx MON_2
@@ -951,8 +952,27 @@ corresponds to January.
 @itemx MON_10
 @itemx MON_11
 @itemx MON_12
-Similar to @code{ABMON_1} etc., but here the month names are not abbreviated.
+Similar to @code{ABMON_1} etc.,@: but here the month names are not abbreviated.
 Here the first value @code{MON_1} also corresponds to January.
+@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}.
+
+Note that not all languages need two different forms of the month names,
+so the strings returned for @code{MON_@dots{}} and @code{ALTMON_@dots{}}
+may or may not be the same, depending on the locale.
 @item AM_STR
 @itemx PM_STR
 The return values are strings which can be used in the representation of time
diff --git a/manual/time.texi b/manual/time.texi
index 33aa221..81c7674 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
+that is appropriate when the month is named by itself, rather than
+the form that is appropriate when the month is used as part of a
+complete date.  This is a GNU extension.
 @end table
 
 If the format supports the modifier but no alternate representation
@@ -1365,13 +1370,21 @@ The abbreviated weekday name according to the current
locale.
 The full weekday name according to the current locale.
 
 @item %b
-The abbreviated month name according to the current locale.
+The abbreviated month name according to the current locale, in the
+grammatical form used when the month is part of a complete date.
+As a GNU extension, the @code{O} modifier can be used (@code{%Ob})
+to get the grammatical form used when the month is named by itself.
 
 @item %B
-The full month name according to the current locale.
+The full month name according to the current locale, in the
+grammatical form used when the month is part of a complete date.
+As a GNU extension, the @code{O} modifier can be used (@code{%OB})
+to get the grammatical form used when the month is named by itself.
 
-Using @code{%B} together with @code{%d} produces grammatically
-incorrect results for some locales.
+Note that not all languages need two different forms of the month
+names, so the text produced by @code{%B} and @code{%OB}, and by
+@code{%b} and @code{%Ob}, may or may not be the same, depending on
+the locale.
 
 @item %c
 The preferred calendar time representation for the current locale.
@@ -1778,8 +1791,14 @@ the full name.
 @item %b
 @itemx %B
 @itemx %h
-The month name according to the current locale, in abbreviated form or
-the full name.
+A month name according to the current locale.  All three specifiers
+will recognize both abbreviated and full month names.  If the
+locale provides two different grammatical forms of month names,
+all three specifiers will recognize both forms.
+
+As a GNU extension, the @code{O} modifier can be used with these
+specifiers; it has no effect, as both grammatical forms of month
+names are recognized anyway.
 
 @item %c
 The date and time representation for the current locale.
-- 
2.7.5

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] pl_PL: Add alternative month names (bug 10871).
  2018-01-08 23:54 [PATCH v11 0/5][BZ 10871] Month names in alternative grammatical case Rafal Luzynski
                   ` (2 preceding siblings ...)
  2018-01-09  0:03 ` [PATCH v11 5/5] Documentation to the above changes " Rafal Luzynski
@ 2018-01-09  0:05 ` Rafal Luzynski
  2018-01-09  0:20   ` Rafal Luzynski
  2018-01-11  4:40   ` Carlos O'Donell
  3 siblings, 2 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-09  0:05 UTC (permalink / raw)
  To: libc-alpha

	[BZ #10871]
	* localedata/locales/pl_PL: alternative month names added,
	basic month names are genitive now.
---
 localedata/locales/pl_PL | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/localedata/locales/pl_PL b/localedata/locales/pl_PL
index 2a8d09d..632a1b3 100644
--- a/localedata/locales/pl_PL
+++ b/localedata/locales/pl_PL
@@ -175,7 +175,7 @@ abmon   "sty";"lut";/
         "lip";"sie";/
         "wrz";"pa<U017A>";/
         "lis";"gru"
-mon     "stycze<U0144>";/
+alt_mon "stycze<U0144>";/
         "luty";/
         "marzec";/
         "kwiecie<U0144>";/
@@ -187,6 +187,18 @@ mon     "stycze<U0144>";/
         "pa<U017A>dziernik";/
         "listopad";/
         "grudzie<U0144>"
+mon     "stycznia";/
+        "lutego";/
+        "marca";/
+        "kwietnia";/
+        "maja";/
+        "czerwca";/
+        "lipca";/
+        "sierpnia";/
+        "wrze<U015B>nia";/
+        "pa<U017A>dziernika";/
+        "listopada";/
+        "grudnia"
 d_t_fmt "%a, %-d %b %Y, %T"
 d_fmt   "%d.%m.%Y"
 t_fmt   "%T"
-- 
2.7.5

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
@ 2018-01-09  0:14   ` Rafal Luzynski
  2018-01-11  4:20     ` Carlos O'Donell
  2018-01-11  2:16   ` Dmitry V. Levin
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-09  0:14 UTC (permalink / raw)
  To: libc-alpha; +Cc: Carlos O'Donell

9.01.2018 00:59 Rafal Luzynski <digitalfreak@lingonborough.com> wrote:
> [...]
> [BZ #10871]
> * locale/C-time.c: Add alternative month names, define them as the
> same as mon explicitly.
> * locale/categories.def: alt_mon and wide-alt_mon added.
> * locale/langinfo.h [__USE_GNU]: New public symbols ALTMON_1,
> ALTMON_2, ALTMON_3, ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7,
> ALTMON_8, ALTMON_9, ALTMON_10, ALTMON_11, ALTMON_12,
> _NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3, _NL_WALTMON_4,
> _NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7, _NL_WALTMON_8,
> _NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11, _NL_WALTMON_12.

Carlos, what do you think about splitting this into:

	* locale/langinfo.h: New public symbols _NL_WALTMON_1,
	_NL_WALTMON_2, _NL_WALTMON_3, _NL_WALTMON_4, _NL_WALTMON_5,
	_NL_WALTMON_6, _NL_WALTMON_7, _NL_WALTMON_8, _NL_WALTMON_9,
	_NL_WALTMON_10, _NL_WALTMON_11, _NL_WALTMON_12.
	[__USE_GNU]: New public symbols ALTMON_1, ALTMON_2, ALTMON_3,
	ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7, ALTMON_8, ALTMON_9,
	ALTMON_10, ALTMON_11, ALTMON_12.

or even:

	* locale/langinfo.h: New public symbols __ALTMON_1,
	__ALTMON_2, __ALTMON_3, __ALTMON_4, __ALTMON_5, __ALTMON_6,
	__ALTMON_7, __ALTMON_8, __ALTMON_9, __ALTMON_10, __ALTMON_11,
	__ALTMON_12, _NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3,
	_NL_WALTMON_4, _NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7,
	_NL_WALTMON_8, _NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11,
	_NL_WALTMON_12.
	[__USE_GNU]: New public symbols ALTMON_1, ALTMON_2, ALTMON_3,
	ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7, ALTMON_8, ALTMON_9,
	ALTMON_10, ALTMON_11, ALTMON_12.

?

Regards,

Rafal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] pl_PL: Add alternative month names (bug 10871).
  2018-01-09  0:05 ` [PATCH] pl_PL: Add alternative month names " Rafal Luzynski
@ 2018-01-09  0:20   ` Rafal Luzynski
  2018-01-11  4:40   ` Carlos O'Donell
  1 sibling, 0 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-09  0:20 UTC (permalink / raw)
  To: libc-alpha; +Cc: Carlos O'Donell

Now I think that maybe this patch should also change these lines
from the first patch:

+  /* TODO: Use the genitive case here as soon as it is added to localedata.  */
+  { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },

into something like:

-  /* TODO: Use the genitive case here as soon as it is added to localedata.  */
-  { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
+  { "pl_PL.UTF-8", "23 listopada 2017", "%d %B %Y", 4, 326, 10, 23 },

I don't mean that the first patch is wrong but it contains a placeholder
for a test which should be added as soon as pl_PL localedata contains
both mon and alt_mon arrays.

Regards,

Rafal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
  2018-01-09  0:14   ` Rafal Luzynski
@ 2018-01-11  2:16   ` Dmitry V. Levin
  2018-01-11  5:05   ` Carlos O'Donell
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 26+ messages in thread
From: Dmitry V. Levin @ 2018-01-11  2:16 UTC (permalink / raw)
  To: libc-alpha

[-- Attachment #1: Type: text/plain, Size: 3198 bytes --]

On Tue, Jan 09, 2018 at 12:59:21AM +0100, Rafal Luzynski wrote:
> 	[BZ #10871]
> 	* locale/C-time.c: Add alternative month names, define them as the
> 	same as mon explicitly.

* locale/C-time.c (_nl_C_LC_TIME): Add alternative month names.

I don't understand what do you mean by "define them as the same as mon explicitly", though.

> 	* locale/categories.def: alt_mon and wide-alt_mon added.

* locale/categories.def (LC_TIME): Add alt_mon and wide-alt_mon.

> 	* locale/langinfo.h [__USE_GNU]: New public symbols ALTMON_1,
> 	ALTMON_2, ALTMON_3, ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7,
> 	ALTMON_8, ALTMON_9, ALTMON_10, ALTMON_11, ALTMON_12,
> 	_NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3, _NL_WALTMON_4,
> 	_NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7, _NL_WALTMON_8,
> 	_NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11, _NL_WALTMON_12.

* locale/langinfo.h (__ALTMON_1, __ALTMON_2, __ALTMON_3, __ALTMON_4,
__ALTMON_5, __ALTMON_6, __ALTMON_7, __ALTMON_8, __ALTMON_9, __ALTMON_10,
__ALTMON_11, __ALTMON_12, _NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3,
_NL_WALTMON_4, _NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7,
_NL_WALTMON_8, _NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11,
_NL_WALTMON_12): New enum constants.
[__USE_GNU] (ALTMON_1, ALTMON_2, ALTMON_3, ALTMON_4, ALTMON_5, ALTMON_6,
ALTMON_7, ALTMON_8, ALTMON_9, ALTMON_10, ALTMON_11, ALTMON_12): New
macros.

> 	* 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.

> 	* locale/programs/locfile-kw.gperf: alt_mon defined.

* locale/programs/locfile-kw.gperf (alt_mon): Define.

> 	* locale/programs/locfile-token.h: tok_alt_mon defined.

* locale/programs/locfile-token.h (tok_alt_mon): New enum constant.

> 	* localedata/tst-langinfo.c: Add tests for the new constants
> 	ALTMON_1 .. ALTMON_12.

* localedata/tst-langinfo.c (map): Add ... (whatever is being added).

> 	* time/Makefile (LOCALES): Add pl_PL.UTF-8 for tests.

* time/Makefile [$(run-built-tests) = yes] (LOCALES): Add pl_PL.UTF-8.

> 	* time/strftime_l.c: %OB format for alternative month names
> 	added.

* time/strftime_l.c (f_altmonth): New macro.
(__strftime_internal): Handle %OB format.

> 	* 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.

> 	* time/tst-strptime.c: Add tests to parse different forms of
> 	month names including the new %OB format specifier.

* time/tst-strptime.c (day_tests): Add ... (whatever is being added).

[...]
> --- a/locale/langinfo.h
> +++ b/locale/langinfo.h
> @@ -100,7 +100,8 @@ enum
>    ABMON_12,
>  #define ABMON_12		ABMON_12
>  
> -  /* 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 ...?


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-09  0:14   ` Rafal Luzynski
@ 2018-01-11  4:20     ` Carlos O'Donell
  0 siblings, 0 replies; 26+ messages in thread
From: Carlos O'Donell @ 2018-01-11  4:20 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha

On 01/08/2018 04:14 PM, Rafal Luzynski wrote:
> 9.01.2018 00:59 Rafal Luzynski <digitalfreak@lingonborough.com> wrote:
>> [...]
>> [BZ #10871]
>> * locale/C-time.c: Add alternative month names, define them as the
>> same as mon explicitly.
>> * locale/categories.def: alt_mon and wide-alt_mon added.
>> * locale/langinfo.h [__USE_GNU]: New public symbols ALTMON_1,
>> ALTMON_2, ALTMON_3, ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7,
>> ALTMON_8, ALTMON_9, ALTMON_10, ALTMON_11, ALTMON_12,
>> _NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3, _NL_WALTMON_4,
>> _NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7, _NL_WALTMON_8,
>> _NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11, _NL_WALTMON_12.
> 
> Carlos, what do you think about splitting this into:
> 
> 	* locale/langinfo.h: New public symbols _NL_WALTMON_1,
> 	_NL_WALTMON_2, _NL_WALTMON_3, _NL_WALTMON_4, _NL_WALTMON_5,
> 	_NL_WALTMON_6, _NL_WALTMON_7, _NL_WALTMON_8, _NL_WALTMON_9,
> 	_NL_WALTMON_10, _NL_WALTMON_11, _NL_WALTMON_12.
> 	[__USE_GNU]: New public symbols ALTMON_1, ALTMON_2, ALTMON_3,
> 	ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7, ALTMON_8, ALTMON_9,
> 	ALTMON_10, ALTMON_11, ALTMON_12.
> 
> or even:
> 
> 	* locale/langinfo.h: New public symbols __ALTMON_1,
> 	__ALTMON_2, __ALTMON_3, __ALTMON_4, __ALTMON_5, __ALTMON_6,
> 	__ALTMON_7, __ALTMON_8, __ALTMON_9, __ALTMON_10, __ALTMON_11,
> 	__ALTMON_12, _NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3,
> 	_NL_WALTMON_4, _NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7,
> 	_NL_WALTMON_8, _NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11,
> 	_NL_WALTMON_12.
> 	[__USE_GNU]: New public symbols ALTMON_1, ALTMON_2, ALTMON_3,
> 	ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7, ALTMON_8, ALTMON_9,
> 	ALTMON_10, ALTMON_11, ALTMON_12.
> 

I think Dmitry has given a good suggestion here, and I would follow
what he recommends. The ChangeLog is a bit messy with complex macros
like this, and you have done a great job with it.

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] pl_PL: Add alternative month names (bug 10871).
  2018-01-09  0:05 ` [PATCH] pl_PL: Add alternative month names " Rafal Luzynski
  2018-01-09  0:20   ` Rafal Luzynski
@ 2018-01-11  4:40   ` Carlos O'Donell
  1 sibling, 0 replies; 26+ messages in thread
From: Carlos O'Donell @ 2018-01-11  4:40 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha

On 01/08/2018 04:05 PM, Rafal Luzynski wrote:
> 	[BZ #10871]
> 	* localedata/locales/pl_PL: alternative month names added,
> 	basic month names are genitive now.

LGTM (pending other patches).

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  localedata/locales/pl_PL | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/localedata/locales/pl_PL b/localedata/locales/pl_PL
> index 2a8d09d..632a1b3 100644
> --- a/localedata/locales/pl_PL
> +++ b/localedata/locales/pl_PL
> @@ -175,7 +175,7 @@ abmon   "sty";"lut";/
>          "lip";"sie";/
>          "wrz";"pa<U017A>";/
>          "lis";"gru"
> -mon     "stycze<U0144>";/
> +alt_mon "stycze<U0144>";/
>          "luty";/
>          "marzec";/
>          "kwiecie<U0144>";/
> @@ -187,6 +187,18 @@ mon     "stycze<U0144>";/
>          "pa<U017A>dziernik";/
>          "listopad";/
>          "grudzie<U0144>"
> +mon     "stycznia";/
> +        "lutego";/
> +        "marca";/
> +        "kwietnia";/
> +        "maja";/
> +        "czerwca";/
> +        "lipca";/
> +        "sierpnia";/
> +        "wrze<U015B>nia";/
> +        "pa<U017A>dziernika";/
> +        "listopada";/
> +        "grudnia"
>  d_t_fmt "%a, %-d %b %Y, %T"
>  d_fmt   "%d.%m.%Y"
>  t_fmt   "%T"
> 


-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 5/5] Documentation to the above changes (bug 10871).
  2018-01-09  0:03 ` [PATCH v11 5/5] Documentation to the above changes " Rafal Luzynski
@ 2018-01-11  4:50   ` Carlos O'Donell
  2018-01-12  4:21     ` Rafal Luzynski
  0 siblings, 1 reply; 26+ messages in thread
From: Carlos O'Donell @ 2018-01-11  4:50 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha

On 01/08/2018 04:03 PM, 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.

OK, with the one suggestion below.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  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.
> +  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.

OK.

> +
> +  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
> +  used by "%OB" and "%Ob", respectively.

OK.

> +
> +  In a locale definition file, use "alt_mon" and "ab_alt_mon" to
> +  define the strings for %OB and %Ob, respectively; these have the
> +  same syntax as "mon" and "ab_mon".

OK.

> +
> +  This feature is currently a GNU extension, but it is expected to
> +  be added to the next revision of POSIX, and it is also already
> +  available on some BSD-derived operating systems.

OK.

> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>  * Support for statically linked applications which call dlopen is deprecated
> 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

OK.

>  unabbreviated weekday name.
>  @item ABMON_1
>  @itemx ABMON_2
> @@ -937,7 +937,8 @@ unabbreviated weekday name.
>  @itemx ABMON_10
>  @itemx ABMON_11
>  @itemx ABMON_12
> -The return value is abbreviated name of the month.  @code{ABMON_1}
> +The return value is the abbreviated name of the month, in the grammatical
> +form used when the month forms part of a complete date.  @code{ABMON_1}

OK.

>  corresponds to January.
>  @item MON_1
>  @itemx MON_2
> @@ -951,8 +952,27 @@ corresponds to January.
>  @itemx MON_10
>  @itemx MON_11
>  @itemx MON_12
> -Similar to @code{ABMON_1} etc., but here the month names are not abbreviated.
> +Similar to @code{ABMON_1} etc.,@: but here the month names are not abbreviated.
>  Here the first value @code{MON_1} also corresponds to January.
> +@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}.

OK.

> +
> +Note that not all languages need two different forms of the month names,
> +so the strings returned for @code{MON_@dots{}} and @code{ALTMON_@dots{}}
> +may or may not be the same, depending on the locale.

OK.

>  @item AM_STR
>  @itemx PM_STR
>  The return values are strings which can be used in the representation of time
> diff --git a/manual/time.texi b/manual/time.texi
> index 33aa221..81c7674 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
> +that is appropriate when the month is named by itself, rather than
> +the form that is appropriate when the month is used as part of a
> +complete date.  This is a GNU extension.

OK.

>  @end table
>  
>  If the format supports the modifier but no alternate representation
> @@ -1365,13 +1370,21 @@ The abbreviated weekday name according to the current
> locale.
>  The full weekday name according to the current locale.
>  
>  @item %b
> -The abbreviated month name according to the current locale.
> +The abbreviated month name according to the current locale, in the
> +grammatical form used when the month is part of a complete date.
> +As a GNU extension, the @code{O} modifier can be used (@code{%Ob})
> +to get the grammatical form used when the month is named by itself.

OK.

>  
>  @item %B
> -The full month name according to the current locale.
> +The full month name according to the current locale, in the
> +grammatical form used when the month is part of a complete date.
> +As a GNU extension, the @code{O} modifier can be used (@code{%OB})
> +to get the grammatical form used when the month is named by itself.

OK.

>  
> -Using @code{%B} together with @code{%d} produces grammatically
> -incorrect results for some locales.
> +Note that not all languages need two different forms of the month
> +names, so the text produced by @code{%B} and @code{%OB}, and by
> +@code{%b} and @code{%Ob}, may or may not be the same, depending on
> +the locale.

OK.

>  
>  @item %c
>  The preferred calendar time representation for the current locale.
> @@ -1778,8 +1791,14 @@ the full name.
>  @item %b
>  @itemx %B
>  @itemx %h
> -The month name according to the current locale, in abbreviated form or
> -the full name.
> +A month name according to the current locale.  All three specifiers
> +will recognize both abbreviated and full month names.  If the
> +locale provides two different grammatical forms of month names,
> +all three specifiers will recognize both forms.
> +
> +As a GNU extension, the @code{O} modifier can be used with these
> +specifiers; it has no effect, as both grammatical forms of month
> +names are recognized anyway.

s/anyway//g.

>  
>  @item %c
>  The date and time representation for the current locale.
> 


-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added (bug 10871).
  2018-01-09  0:01 ` [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added " Rafal Luzynski
@ 2018-01-11  5:05   ` Carlos O'Donell
  2018-01-11 19:04     ` Rafal Luzynski
  0 siblings, 1 reply; 26+ messages in thread
From: Carlos O'Donell @ 2018-01-11  5:05 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha

On 01/08/2018 04:01 PM, Rafal Luzynski wrote:
> All the previous changes 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
> features the declension suffixes.
> 
> 	[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: New public symbols _NL_ABALTMON_1,
> 	_NL_ABALTMON_2, _NL_ABALTMON_3, _NL_ABALTMON_4, _NL_ABALTMON_5,
> 	_NL_ABALTMON_6, _NL_ABALTMON_7, _NL_ABALTMON_8, _NL_ABALTMON_9,
> 	_NL_ABALTMON_10, _NL_ABALTMON_11, _NL_ABALTMON_12,
> 	_NL_WABALTMON_1, _NL_WABALTMON_2, _NL_WABALTMON_3, _NL_WABALTMON_4,
> 	_NL_WABALTMON_5, _NL_WABALTMON_6, _NL_WABALTMON_7, _NL_WABALTMON_8,
> 	_NL_WABALTMON_9, _NL_WABALTMON_10, _NL_WABALTMON_11, _NL_WABALTMON_12.
> 	* 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/Makefile (LOCALES): Add ru_RU.UTF-8 for tests.
> 	* time/strftime_l.c: %Ob (%Oh) format for abbreviated
> 	alternative month names added.
> 	* time/strptime_l.c: Abbreviated alternative month names also
> 	recognized.
> 	* time/tst-strptime.c: Add more tests to parse different forms of
> 	month names including the new %Ob format specifier.

Why is there no ABALTMON_* via #ifdef __USE_GNU like there is for ALTMON_*?
It is OK without them, but seems like a missing useful feature.

OK with the test case changes to write UTF-8 directly in the test case string.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  locale/C-time.c                  | 28 ++++++++++++++++++++++++++--
>  locale/categories.def            |  6 ++++--
>  locale/langinfo.h                | 36 ++++++++++++++++++++++++++++++++++--
>  locale/programs/ld-time.c        | 19 +++++++++++++++++++
>  locale/programs/locfile-kw.gperf |  1 +
>  locale/programs/locfile-token.h  |  1 +
>  time/Makefile                    |  3 ++-
>  time/strftime_l.c                | 14 ++++++++++++--
>  time/strptime_l.c                | 18 ++++++++++++++++++
>  time/tst-strptime.c              | 13 +++++++++++++
>  10 files changed, 130 insertions(+), 9 deletions(-)
> 
> diff --git a/locale/C-time.c b/locale/C-time.c
> index 73bc700..e2b3b17 100644
> --- a/locale/C-time.c
> +++ b/locale/C-time.c
> @@ -30,7 +30,7 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
>    { NULL, },			/* no cached data */
>    UNDELETABLE,
>    0,
> -  135,
> +  159,

OK.

>    {
>      { .string = "Sun" },
>      { .string = "Mon" },
> @@ -166,6 +166,30 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
>      { .wstr = (const uint32_t *) L"September" },
>      { .wstr = (const uint32_t *) L"October" },
>      { .wstr = (const uint32_t *) L"November" },
> -    { .wstr = (const uint32_t *) L"December" }
> +    { .wstr = (const uint32_t *) L"December" },
> +    { .string = "Jan" },
> +    { .string = "Feb" },
> +    { .string = "Mar" },
> +    { .string = "Apr" },
> +    { .string = "May" },
> +    { .string = "Jun" },
> +    { .string = "Jul" },
> +    { .string = "Aug" },
> +    { .string = "Sep" },
> +    { .string = "Oct" },
> +    { .string = "Nov" },
> +    { .string = "Dec" },
> +    { .wstr = (const uint32_t *) L"Jan" },
> +    { .wstr = (const uint32_t *) L"Feb" },
> +    { .wstr = (const uint32_t *) L"Mar" },
> +    { .wstr = (const uint32_t *) L"Apr" },
> +    { .wstr = (const uint32_t *) L"May" },
> +    { .wstr = (const uint32_t *) L"Jun" },
> +    { .wstr = (const uint32_t *) L"Jul" },
> +    { .wstr = (const uint32_t *) L"Aug" },
> +    { .wstr = (const uint32_t *) L"Sep" },
> +    { .wstr = (const uint32_t *) L"Oct" },
> +    { .wstr = (const uint32_t *) L"Nov" },
> +    { .wstr = (const uint32_t *) L"Dec" }

OK.

>    }
>  };
> diff --git a/locale/categories.def b/locale/categories.def
> index 3cbb4e6..56c5f88 100644
> --- a/locale/categories.def
> +++ b/locale/categories.def
> @@ -249,8 +249,10 @@ DEFINE_CATEGORY
>    DEFINE_ELEMENT (_DATE_FMT,                "date_fmt",            opt, string)
>    DEFINE_ELEMENT (_NL_W_DATE_FMT,           "wide-date_fmt",       opt,
> wstring)
>    DEFINE_ELEMENT (_NL_TIME_CODESET,	    "time-codeset",	   std, string)
> -  DEFINE_ELEMENT (ALTMON_1,       "alt_mon",       opt, stringarray, 12, 12)
> -  DEFINE_ELEMENT (_NL_WALTMON_1,  "wide-alt_mon",  opt, wstringarray, 12, 12)
> +  DEFINE_ELEMENT (ALTMON_1,        "alt_mon",         opt, stringarray,  12,
> 12)
> +  DEFINE_ELEMENT (_NL_WALTMON_1,   "wide-alt_mon",    opt, wstringarray, 12,
> 12)
> +  DEFINE_ELEMENT (_NL_ABALTMON_1,  "ab_alt_mon",      opt, stringarray,  12,
> 12)
> +  DEFINE_ELEMENT (_NL_WABALTMON_1, "wide-ab_alt_mon", opt, wstringarray, 12,

OK.

> 12)
>    ), NO_POSTLOAD)
>  
>  
> diff --git a/locale/langinfo.h b/locale/langinfo.h
> index 0fbd838..4749e9d 100644
> --- a/locale/langinfo.h
> +++ b/locale/langinfo.h
> @@ -74,7 +74,8 @@ enum
>    DAY_7,			/* Saturday */
>  #define DAY_7			DAY_7
>  
> -  /* Abbreviated month names.  */
> +  /* Abbreviated month names, in the grammatical form used when the month
> +     forms part of a complete date.  */

OK.

>    ABMON_1,			/* Jan */
>  #define ABMON_1			ABMON_1
>    ABMON_2,
> @@ -176,7 +177,8 @@ enum
>    _NL_WDAY_6,		/* Friday */
>    _NL_WDAY_7,		/* Saturday */
>  
> -  /* Abbreviated month names.  */
> +  /* Abbreviated month names, in the grammatical form used when the month
> +     forms part of a complete date.  */

OK.

>    _NL_WABMON_1,		/* Jan */
>    _NL_WABMON_2,
>    _NL_WABMON_3,
> @@ -277,6 +279,36 @@ enum
>    _NL_WALTMON_11,
>    _NL_WALTMON_12,
>  
> +  /* Abbreviated month names, in the grammatical form used when the month
> +     is named by itself.  */
> +  _NL_ABALTMON_1,			/* Jan */
> +  _NL_ABALTMON_2,
> +  _NL_ABALTMON_3,
> +  _NL_ABALTMON_4,
> +  _NL_ABALTMON_5,
> +  _NL_ABALTMON_6,
> +  _NL_ABALTMON_7,
> +  _NL_ABALTMON_8,
> +  _NL_ABALTMON_9,
> +  _NL_ABALTMON_10,
> +  _NL_ABALTMON_11,
> +  _NL_ABALTMON_12,
> +
> +  /* Abbreviated month names, in the grammatical form used when the month
> +     is named by itself.  */
> +  _NL_WABALTMON_1,			/* Jan */
> +  _NL_WABALTMON_2,
> +  _NL_WABALTMON_3,
> +  _NL_WABALTMON_4,
> +  _NL_WABALTMON_5,
> +  _NL_WABALTMON_6,
> +  _NL_WABALTMON_7,
> +  _NL_WABALTMON_8,
> +  _NL_WABALTMON_9,
> +  _NL_WABALTMON_10,
> +  _NL_WABALTMON_11,
> +  _NL_WABALTMON_12,

OK.

> +
>    _NL_NUM_LC_TIME,	/* Number of indices in LC_TIME category.  */
>  
>    /* LC_COLLATE category: text sorting.
> diff --git a/locale/programs/ld-time.c b/locale/programs/ld-time.c
> index 4186448..a755792 100644
> --- a/locale/programs/ld-time.c
> +++ b/locale/programs/ld-time.c
> @@ -94,6 +94,9 @@ struct locale_time_t
>    const char *alt_mon[12];
>    const uint32_t *walt_mon[12];
>    int alt_mon_defined;
> +  const char *ab_alt_mon[12];
> +  const uint32_t *wab_alt_mon[12];
> +  int ab_alt_mon_defined;

OK.

>    unsigned char week_ndays;
>    uint32_t week_1stday;
>    unsigned char week_1stweek;
> @@ -651,6 +654,14 @@ time_output (struct localedef_t *locale, const struct
> charmap_t *charmap,
>    for (n = 0; n < 12; ++n)
>      add_locale_wstring (&file, time->walt_mon[n] ?: empty_wstr);
>  
> +  /* The ab'alt'mons.  */
> +  for (n = 0; n < 12; ++n)
> +    add_locale_string (&file, time->ab_alt_mon[n] ?: "");
> +
> +  /* The wide character ab'alt'mons.  */
> +  for (n = 0; n < 12; ++n)
> +    add_locale_wstring (&file, time->wab_alt_mon[n] ?: empty_wstr);

OK.

> +
>    write_locale_data (output_path, LC_TIME, "LC_TIME", &file);
>  }
>  
> @@ -795,6 +806,7 @@ time_read (struct linereader *ldfile, struct localedef_t
> *result,
>  	  STRARR_ELEM (am_pm, 2, 2);
>  	  STRARR_ELEM (alt_digits, 0, 100);
>  	  STRARR_ELEM (alt_mon, 12, 12);
> +	  STRARR_ELEM (ab_alt_mon, 12, 12);

OK.

>  
>  	case tok_era:
>  	  /* Ignore the rest of the line if we don't need the input of
> @@ -955,6 +967,13 @@ time_read (struct linereader *ldfile, struct localedef_t
> *result,
>  	      memcpy (time->walt_mon, time->wmon, sizeof (time->wmon));
>  	      time->alt_mon_defined = 1;
>  	    }
> +	  /* The same for abbreviated versions.  */
> +	  if (!ignore_content && !time->ab_alt_mon_defined)
> +	    {
> +	      memcpy (time->ab_alt_mon, time->abmon, sizeof (time->abmon));
> +	      memcpy (time->wab_alt_mon, time->wabmon, sizeof (time->wabmon));
> +	      time->ab_alt_mon_defined = 1;

OK. Good, copy the abmon/wabmon versions.

> +	    }
>  	  return;
>  
>  	default:
> diff --git a/locale/programs/locfile-kw.gperf b/locale/programs/locfile-kw.gperf
> index dad7f21..6bf2f60 100644
> --- a/locale/programs/locfile-kw.gperf
> +++ b/locale/programs/locfile-kw.gperf
> @@ -149,6 +149,7 @@ cal_direction,          tok_cal_direction,          0
>  timezone,               tok_timezone,               0
>  date_fmt,               tok_date_fmt,               0
>  alt_mon,                tok_alt_mon,                0
> +ab_alt_mon,             tok_ab_alt_mon,             0
>  LC_MESSAGES,            tok_lc_messages,            0
>  yesexpr,                tok_yesexpr,                0
>  noexpr,                 tok_noexpr,                 0
> diff --git a/locale/programs/locfile-token.h b/locale/programs/locfile-token.h
> index d49da5e..e3cd18e 100644
> --- a/locale/programs/locfile-token.h
> +++ b/locale/programs/locfile-token.h
> @@ -187,6 +187,7 @@ enum token_t
>    tok_timezone,
>    tok_date_fmt,
>    tok_alt_mon,
> +  tok_ab_alt_mon,

OK. New token.

>    tok_lc_messages,
>    tok_yesexpr,
>    tok_noexpr,
> diff --git a/time/Makefile b/time/Makefile
> index 91adcd0..4e631a1 100644
> --- a/time/Makefile
> +++ b/time/Makefile
> @@ -48,7 +48,8 @@ tests	:= test_time clocktest tst-posixtz tst-strptime
> tst_wcsftime \
>  include ../Rules
>  
>  ifeq ($(run-built-tests),yes)
> -LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP pl_PL.UTF-8
> +LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP pl_PL.UTF-8 \
> +	   ru_RU.UTF-8

OK.

>  include ../gen-locales.mk
>  
>  $(objpfx)tst-ftime_l.out: $(gen-locales)
> diff --git a/time/strftime_l.c b/time/strftime_l.c
> index ac5d28f..c71f9f4 100644
> --- a/time/strftime_l.c
> +++ b/time/strftime_l.c
> @@ -106,6 +106,7 @@ extern char *tzname[];
>  # define UCHAR_T unsigned char
>  # define L_(Str) Str
>  # define NLW(Sym) Sym
> +# define ABALTMON_1 _NL_ABALTMON_1

OK.

>  
>  # if !defined STDC_HEADERS && !defined HAVE_MEMCPY
>  #  define MEMCPY(d, s, n) bcopy ((s), (d), (n))
> @@ -492,6 +493,9 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
> *format,
>  # define f_month \
>    ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
>  		     ? "?" : _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon)))
> +# define a_altmonth \
> +  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
> +		     ? "?" : _NL_CURRENT (LC_TIME, NLW(ABALTMON_1) + tp->tm_mon)))

OK.

>  # define f_altmonth \
>    ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
>  		     ? "?" : _NL_CURRENT (LC_TIME, NLW(ALTMON_1) + tp->tm_mon)))
> @@ -501,6 +505,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
> *format,
>  
>  # define aw_len STRLEN (a_wkday)
>  # define am_len STRLEN (a_month)
> +# define aam_len STRLEN (a_altmonth)
>  # define ap_len STRLEN (ampm)
>  #else
>  # if !HAVE_STRFTIME
> @@ -510,11 +515,13 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const
> CHAR_T *format,
>  		   ? "?" : month_name[tp->tm_mon])
>  #  define a_wkday f_wkday
>  #  define a_month f_month
> +#  define a_altmonth a_month
>  #  define f_altmonth f_month
>  #  define ampm (L_("AMPM") + 2 * (tp->tm_hour > 11))
>  
>    size_t aw_len = 3;
>    size_t am_len = 3;
> +  size_t aam_len = 3;
>    size_t ap_len = 2;
>  # endif
>  #endif
> @@ -779,10 +786,13 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const
> CHAR_T *format,
>  	      to_uppcase = 1;
>  	      to_lowcase = 0;
>  	    }
> -	  if (modifier != 0)
> +	  if (modifier == L_('E'))

OK.

>  	    goto bad_format;
>  #if defined _NL_CURRENT || !HAVE_STRFTIME
> -	  cpy (am_len, a_month);
> +	  if (modifier == L_('O'))
> +	    cpy (aam_len, a_altmonth);
> +	  else
> +	    cpy (am_len, a_month);

OK.

>  	  break;
>  #else
>  	  goto underlying_strftime;
> diff --git a/time/strptime_l.c b/time/strptime_l.c
> index 39cf38d..cd901c2 100644
> --- 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)

OK.

>  # 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)
> @@ -437,6 +439,18 @@ __strptime_internal (const char *rp, const char *fmt,
> struct tm *tmp,
>  				     alt_month_name[cnt]))
>  			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;

OK.

> +		    }
>  #endif
>  		}
>  #endif
> @@ -448,6 +462,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)

OK.


>  #endif
>  	      ))
>  		{
> @@ -1035,7 +1051,9 @@ __strptime_internal (const char *rp, const char *fmt,
> struct tm *tmp,
>  	case 'O':
>  	  switch (*fmt++)
>  	    {
> +	    case 'b':
>  	    case 'B':
> +	    case 'h':

OK.

>  	      /* Match month name.  Reprocess as plain 'B'.  */
>  	      fmt--;
>  	      goto start_over;
> diff --git a/time/tst-strptime.c b/time/tst-strptime.c
> index bbc1390..ab09f0f 100644
> --- a/time/tst-strptime.c
> +++ b/time/tst-strptime.c
> @@ -24,6 +24,11 @@
>  #include <time.h>
>  
>  
> +/* Some Cyrillic letters in UTF-8.  */
> +#define CYR_n  "\xd0\xbd"
> +#define CYR_o  "\xd0\xbe"
> +#define CYR_ya "\xd1\x8f"

Please encode the UTF-8 directly into the test case.

Developers have to use UTF-8 capable editors, and fonts.

> +
>  static const struct
>  {
>    const char *locale;
> @@ -57,6 +62,14 @@ static const struct
>    { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
>    /* The nominative case is incorrect here but it is parseable.  */
>    { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
> +  { "pl_PL.UTF-8", "25 lis 2017", "%d %Ob %Y", 6, 328, 10, 25 },
> +  { "ru_RU.UTF-8", "26 " CYR_n CYR_o CYR_ya " 2017", "%d %b %Y",
> +     0, 329, 10, 26 },

OK.

> +  /* TODO: Add an example of "may"/"maya" (5th month, May) using %Ob in
> +     Russian when the localedata is updated.  Without the genitive forms
> +     in localedata the word "maya" is ambiguous and may be mistaken for
> +     "mart" (March).
> +   */
>  };
>  
>  
> 


-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
  2018-01-09  0:14   ` Rafal Luzynski
  2018-01-11  2:16   ` Dmitry V. Levin
@ 2018-01-11  5:05   ` Carlos O'Donell
  2018-01-12  3:44     ` Rafal Luzynski
  2018-01-11 11:46   ` Rafal Luzynski
  2018-01-11 22:26   ` Rafal Luzynski
  4 siblings, 1 reply; 26+ messages in thread
From: Carlos O'Donell @ 2018-01-11  5:05 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha

On 01/08/2018 03:59 PM, Rafal Luzynski wrote:
> Some languages (Slavic, Baltic, etc.) require a genitive case of the
> month name when formatting a full date (with the day number) while
> they require a nominative case when referring to the month standalone.
> This requirement cannot be fulfilled without providing two forms for
> each month name.  From now it is precised that nl_langinfo(MON_1)
> series (up to MON_12) and strftime("%B") generate the month names in
> the grammatical form used when the month forms part of a complete date.
> If the grammatical form used when the month is named by itself is needed,
> the new values nl_langinfo(ALTMON_1) (up to ALTMON_12) and
> strftime("%OB") are supported.  This new feature is optional so the
> languages which do not need it or do not yet provide the updated
> locales simply do not use it and their behaviour is unchanged.

High level:

Overall the idea you have proposed makes perfect sense. I'm excited to see
someone pushing a new interface design like this to solve real language
problems with the interfaces. I look forward to the POSIX standardization.
My obvious worry remains that POSIX does not standardize %OB/%Ob, but all
we can do is lead by example and solve user problems. Thank you for this
work.

Design:

By the very definition of the %O* variants, if the locale does not 
provide ALTMON_* then it must revert back to the non-%O* variant e.g. %B. 
I see this implemented in the parser which is good.

I also see that strftime is relaxed in what it accepts and that is also
good and required.

One oddity I noticed in patch 3 is the missing ABALTMON_* definitions
for the _GNU_SOURCE case, is that on purpose and why? It's not needed,
but it seems like a missing useful feature that completes the API.

Implementation:

My only request here is additional tests that verify, for locales
that don't have ALTMON or ABALTMON specified, thus verifying the fallback
works. With that added I think you can commit this change squashed together
with the pl_PL and ru_RU changes and the test cases fixed up to pass.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 	[BZ #10871]
> 	* locale/C-time.c: Add alternative month names, define them as the
> 	same as mon explicitly.
> 	* locale/categories.def: alt_mon and wide-alt_mon added.
> 	* locale/langinfo.h [__USE_GNU]: New public symbols ALTMON_1,
> 	ALTMON_2, ALTMON_3, ALTMON_4, ALTMON_5, ALTMON_6, ALTMON_7,
> 	ALTMON_8, ALTMON_9, ALTMON_10, ALTMON_11, ALTMON_12,
> 	_NL_WALTMON_1, _NL_WALTMON_2, _NL_WALTMON_3, _NL_WALTMON_4,
> 	_NL_WALTMON_5, _NL_WALTMON_6, _NL_WALTMON_7, _NL_WALTMON_8,
> 	_NL_WALTMON_9, _NL_WALTMON_10, _NL_WALTMON_11, _NL_WALTMON_12.
> 	* locale/programs/ld-time.c: Alternative month names support
> 	added, they are a copy of mon if not specified explicitly.
> 	* locale/programs/locfile-kw.gperf: alt_mon defined.
> 	* locale/programs/locfile-token.h: tok_alt_mon defined.
> 	* localedata/tst-langinfo.c: Add tests for the new constants
> 	ALTMON_1 .. ALTMON_12.
> 	* time/Makefile (LOCALES): Add pl_PL.UTF-8 for tests.
> 	* time/strftime_l.c: %OB format for alternative month names
> 	added.
> 	* time/strptime_l.c: Alternative month names also recognized.
> 	* time/tst-strptime.c: Add tests to parse different forms of
> 	month names including the new %OB format specifier.
> ---
>  locale/C-time.c                  | 28 ++++++++++++++++++++--
>  locale/categories.def            |  2 ++
>  locale/langinfo.h                | 50 ++++++++++++++++++++++++++++++++++++++--
>  locale/programs/ld-time.c        | 21 +++++++++++++++++
>  locale/programs/locfile-kw.gperf |  1 +
>  locale/programs/locfile-token.h  |  1 +
>  localedata/tst-langinfo.c        | 12 ++++++++++
>  time/Makefile                    |  2 +-
>  time/strftime_l.c                | 11 +++++++--
>  time/strptime_l.c                | 32 +++++++++++++++++++++----
>  time/tst-strptime.c              |  6 +++++
>  11 files changed, 155 insertions(+), 11 deletions(-)
> 
> diff --git a/locale/C-time.c b/locale/C-time.c
> index 1f1ee01..73bc700 100644
> --- a/locale/C-time.c
> +++ b/locale/C-time.c
> @@ -30,7 +30,7 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
>    { NULL, },			/* no cached data */
>    UNDELETABLE,
>    0,
> -  111,
> +  135,

OK.

>    {
>      { .string = "Sun" },
>      { .string = "Mon" },
> @@ -142,6 +142,30 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
>      { .string = "" },
>      { .string = "%a %b %e %H:%M:%S %Z %Y" },
>      { .wstr = (const uint32_t *) L"%a %b %e %H:%M:%S %Z %Y" },
> -    { .string = _nl_C_codeset }
> +    { .string = _nl_C_codeset },
> +    { .string = "January" },
> +    { .string = "February" },
> +    { .string = "March" },
> +    { .string = "April" },
> +    { .string = "May" },
> +    { .string = "June" },
> +    { .string = "July" },
> +    { .string = "August" },
> +    { .string = "September" },
> +    { .string = "October" },
> +    { .string = "November" },
> +    { .string = "December" },
> +    { .wstr = (const uint32_t *) L"January" },
> +    { .wstr = (const uint32_t *) L"February" },
> +    { .wstr = (const uint32_t *) L"March" },
> +    { .wstr = (const uint32_t *) L"April" },
> +    { .wstr = (const uint32_t *) L"May" },
> +    { .wstr = (const uint32_t *) L"June" },
> +    { .wstr = (const uint32_t *) L"July" },
> +    { .wstr = (const uint32_t *) L"August" },
> +    { .wstr = (const uint32_t *) L"September" },
> +    { .wstr = (const uint32_t *) L"October" },
> +    { .wstr = (const uint32_t *) L"November" },
> +    { .wstr = (const uint32_t *) L"December" }

OK.

>    }
>  };
> diff --git a/locale/categories.def b/locale/categories.def
> index 47947f7..3cbb4e6 100644
> --- a/locale/categories.def
> +++ b/locale/categories.def
> @@ -249,6 +249,8 @@ DEFINE_CATEGORY
>    DEFINE_ELEMENT (_DATE_FMT,                "date_fmt",            opt, string)
>    DEFINE_ELEMENT (_NL_W_DATE_FMT,           "wide-date_fmt",       opt,
> wstring)
>    DEFINE_ELEMENT (_NL_TIME_CODESET,	    "time-codeset",	   std, string)
> +  DEFINE_ELEMENT (ALTMON_1,       "alt_mon",       opt, stringarray, 12, 12)
> +  DEFINE_ELEMENT (_NL_WALTMON_1,  "wide-alt_mon",  opt, wstringarray, 12, 12)

OK.

>    ), NO_POSTLOAD)
>  
>  
> diff --git a/locale/langinfo.h b/locale/langinfo.h
> index 28a0a73..0fbd838 100644
> --- a/locale/langinfo.h
> +++ b/locale/langinfo.h
> @@ -100,7 +100,8 @@ enum
>    ABMON_12,
>  #define ABMON_12		ABMON_12
>  
> -  /* Long month names.  */
> +  /* Long month names, in the grammatical form used when the month
> +     forms part of a complete date.  */

OK.

>    MON_1,			/* January */
>  #define MON_1			MON_1
>    MON_2,
> @@ -189,7 +190,8 @@ enum
>    _NL_WABMON_11,
>    _NL_WABMON_12,
>  
> -  /* Long month names.  */
> +  /* Long month names, in the grammatical form used when the month
> +     forms part of a complete date.  */

OK.

>    _NL_WMON_1,		/* January */
>    _NL_WMON_2,
>    _NL_WMON_3,
> @@ -231,6 +233,50 @@ enum
>  
>    _NL_TIME_CODESET,
>  
> +  /* Long month names, in the grammatical form used when the month
> +     is named by itself.  */
> +  __ALTMON_1,			/* January */
> +  __ALTMON_2,
> +  __ALTMON_3,
> +  __ALTMON_4,
> +  __ALTMON_5,
> +  __ALTMON_6,
> +  __ALTMON_7,
> +  __ALTMON_8,
> +  __ALTMON_9,
> +  __ALTMON_10,
> +  __ALTMON_11,
> +  __ALTMON_12,

OK.

> +#ifdef __USE_GNU
> +# define ALTMON_1		__ALTMON_1
> +# define ALTMON_2		__ALTMON_2
> +# define ALTMON_3		__ALTMON_3
> +# define ALTMON_4		__ALTMON_4
> +# define ALTMON_5		__ALTMON_5
> +# define ALTMON_6		__ALTMON_6
> +# define ALTMON_7		__ALTMON_7
> +# define ALTMON_8		__ALTMON_8
> +# define ALTMON_9		__ALTMON_9
> +# define ALTMON_10		__ALTMON_10
> +# define ALTMON_11		__ALTMON_11
> +# define ALTMON_12		__ALTMON_12

OK. Requires _GNU_SOURCE, which is good because this is an extension not
yet defined by POSIX.

> +#endif
> +
> +  /* Long month names, in the grammatical form used when the month
> +     is named by itself.  */
> +  _NL_WALTMON_1,			/* January */
> +  _NL_WALTMON_2,
> +  _NL_WALTMON_3,
> +  _NL_WALTMON_4,
> +  _NL_WALTMON_5,
> +  _NL_WALTMON_6,
> +  _NL_WALTMON_7,
> +  _NL_WALTMON_8,
> +  _NL_WALTMON_9,
> +  _NL_WALTMON_10,
> +  _NL_WALTMON_11,
> +  _NL_WALTMON_12,

OK.

> +
>    _NL_NUM_LC_TIME,	/* Number of indices in LC_TIME category.  */
>  
>    /* LC_COLLATE category: text sorting.
> diff --git a/locale/programs/ld-time.c b/locale/programs/ld-time.c
> index 67d055a..4186448 100644
> --- a/locale/programs/ld-time.c
> +++ b/locale/programs/ld-time.c
> @@ -91,6 +91,9 @@ struct locale_time_t
>    const char *date_fmt;
>    const uint32_t *wdate_fmt;
>    int alt_digits_defined;
> +  const char *alt_mon[12];
> +  const uint32_t *walt_mon[12];
> +  int alt_mon_defined;

OK.

>    unsigned char week_ndays;
>    uint32_t week_1stday;
>    unsigned char week_1stweek;
> @@ -639,6 +642,15 @@ time_output (struct localedef_t *locale, const struct
> charmap_t *charmap,
>    add_locale_string (&file, time->date_fmt);
>    add_locale_wstring (&file, time->wdate_fmt);
>    add_locale_string (&file, charmap->code_set_name);
> +
> +  /* The alt'mons.  */
> +  for (n = 0; n < 12; ++n)
> +    add_locale_string (&file, time->alt_mon[n] ?: "");
> +
> +  /* The wide character alt'mons.  */
> +  for (n = 0; n < 12; ++n)
> +    add_locale_wstring (&file, time->walt_mon[n] ?: empty_wstr);

OK.

> +
>    write_locale_data (output_path, LC_TIME, "LC_TIME", &file);
>  }
>  
> @@ -782,6 +794,7 @@ time_read (struct linereader *ldfile, struct localedef_t
> *result,
>  	  STRARR_ELEM (mon, 12, 12);
>  	  STRARR_ELEM (am_pm, 2, 2);
>  	  STRARR_ELEM (alt_digits, 0, 100);
> +	  STRARR_ELEM (alt_mon, 12, 12);

OK.

>  
>  	case tok_era:
>  	  /* Ignore the rest of the line if we don't need the input of
> @@ -934,6 +947,14 @@ time_read (struct linereader *ldfile, struct localedef_t
> *result,
>  	    lr_error (ldfile, _("\
>  %1$s: definition does not end with `END %1$s'"), "LC_TIME");
>  	  lr_ignore_rest (ldfile, now->tok == tok_lc_time);
> +
> +	  /* If alt_mon was not specified, make it a copy of mon.  */
> +	  if (!ignore_content && !time->alt_mon_defined)
> +	    {
> +	      memcpy (time->alt_mon, time->mon, sizeof (time->mon));
> +	      memcpy (time->walt_mon, time->wmon, sizeof (time->wmon));
> +	      time->alt_mon_defined = 1;
> +	    }

OK. Good, fall back to %B.

>  	  return;
>  
>  	default:
> diff --git a/locale/programs/locfile-kw.gperf b/locale/programs/locfile-kw.gperf
> index c74c1f2..dad7f21 100644
> --- a/locale/programs/locfile-kw.gperf
> +++ b/locale/programs/locfile-kw.gperf
> @@ -148,6 +148,7 @@ first_workday,          tok_first_workday,          0
>  cal_direction,          tok_cal_direction,          0
>  timezone,               tok_timezone,               0
>  date_fmt,               tok_date_fmt,               0
> +alt_mon,                tok_alt_mon,                0

OK.

>  LC_MESSAGES,            tok_lc_messages,            0
>  yesexpr,                tok_yesexpr,                0
>  noexpr,                 tok_noexpr,                 0
> diff --git a/locale/programs/locfile-token.h b/locale/programs/locfile-token.h
> index f02325d..d49da5e 100644
> --- a/locale/programs/locfile-token.h
> +++ b/locale/programs/locfile-token.h
> @@ -186,6 +186,7 @@ enum token_t
>    tok_cal_direction,
>    tok_timezone,
>    tok_date_fmt,
> +  tok_alt_mon,

OK.

>    tok_lc_messages,
>    tok_yesexpr,
>    tok_noexpr,
> diff --git a/localedata/tst-langinfo.c b/localedata/tst-langinfo.c
> index 8c3667c..0d33e75 100644
> --- a/localedata/tst-langinfo.c
> +++ b/localedata/tst-langinfo.c
> @@ -50,6 +50,18 @@ struct map
>    VAL (ABMON_8),
>    VAL (ABMON_9),
>    VAL (ALT_DIGITS),
> +  VAL (ALTMON_1),
> +  VAL (ALTMON_10),
> +  VAL (ALTMON_11),
> +  VAL (ALTMON_12),
> +  VAL (ALTMON_2),
> +  VAL (ALTMON_3),
> +  VAL (ALTMON_4),
> +  VAL (ALTMON_5),
> +  VAL (ALTMON_6),
> +  VAL (ALTMON_7),
> +  VAL (ALTMON_8),
> +  VAL (ALTMON_9),

OK.

>    VAL (AM_STR),
>    VAL (CRNCYSTR),
>    VAL (CURRENCY_SYMBOL),
> diff --git a/time/Makefile b/time/Makefile
> index 264eed9..91adcd0 100644
> --- a/time/Makefile
> +++ b/time/Makefile
> @@ -48,7 +48,7 @@ tests	:= test_time clocktest tst-posixtz tst-strptime
> tst_wcsftime \
>  include ../Rules
>  
>  ifeq ($(run-built-tests),yes)
> -LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP
> +LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP pl_PL.UTF-8

OK.

>  include ../gen-locales.mk
>  
>  $(objpfx)tst-ftime_l.out: $(gen-locales)
> diff --git a/time/strftime_l.c b/time/strftime_l.c
> index 18651ff..ac5d28f 100644
> --- a/time/strftime_l.c
> +++ b/time/strftime_l.c
> @@ -492,6 +492,9 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
> *format,
>  # define f_month \
>    ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
>  		     ? "?" : _NL_CURRENT (LC_TIME, NLW(MON_1) + tp->tm_mon)))
> +# define f_altmonth \
> +  ((const CHAR_T *) (tp->tm_mon < 0 || tp->tm_mon > 11			     \
> +		     ? "?" : _NL_CURRENT (LC_TIME, NLW(ALTMON_1) + tp->tm_mon)))

OK.

>  # define ampm \
>    ((const CHAR_T *) _NL_CURRENT (LC_TIME, tp->tm_hour > 11		      \
>  				 ? NLW(PM_STR) : NLW(AM_STR)))
> @@ -507,6 +510,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
> *format,
>  		   ? "?" : month_name[tp->tm_mon])
>  #  define a_wkday f_wkday
>  #  define a_month f_month
> +#  define f_altmonth f_month

OK.

>  #  define ampm (L_("AMPM") + 2 * (tp->tm_hour > 11))
>  
>    size_t aw_len = 3;
> @@ -785,7 +789,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T
> *format,
>  #endif
>  
>  	case L_('B'):
> -	  if (modifier != 0)
> +	  if (modifier == L_('E'))
>  	    goto bad_format;

OK. Accept %O but not %E.

>  	  if (change_case)
>  	    {
> @@ -793,7 +797,10 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const
> CHAR_T *format,
>  	      to_lowcase = 0;
>  	    }
>  #if defined _NL_CURRENT || !HAVE_STRFTIME
> -	  cpy (STRLEN (f_month), f_month);
> +	  if (modifier == L_('O'))
> +	    cpy (STRLEN (f_altmonth), f_altmonth);

OK.

> +	  else
> +	    cpy (STRLEN (f_month), f_month);
>  	  break;
>  #else
>  	  goto underlying_strftime;
> diff --git a/time/strptime_l.c b/time/strptime_l.c
> index 7d4758e..39cf38d 100644
> --- a/time/strptime_l.c
> +++ b/time/strptime_l.c
> @@ -124,6 +124,8 @@ extern const struct __locale_data _nl_C_LC_TIME
> attribute_hidden;
>    (&_nl_C_LC_TIME.values[_NL_ITEM_INDEX (ABDAY_1)].string)
>  # define month_name (&_nl_C_LC_TIME.values[_NL_ITEM_INDEX (MON_1)].string)
>  # 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)

OK.

>  # 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)
> @@ -319,10 +321,9 @@ __strptime_internal (const char *rp, const char *fmt,
> struct tm *tmp,
>        while (*fmt >= '0' && *fmt <= '9')
>  	++fmt;
>  
> -#ifndef _NL_CURRENT
> -      /* We need this for handling the `E' modifier.  */
> +      /* In some cases, modifiers are handled by adjusting state and
> +         then restarting the switch statement below.  */

OK.

>      start_over:
> -#endif
>  
>        /* Make back up of current processing pointer.  */
>        rp_backup = rp;
> @@ -423,13 +424,32 @@ __strptime_internal (const char *rp, const char *fmt,
> struct tm *tmp,
>  				     ab_month_name[cnt]))
>  			decided_longest = loc;
>  		    }
> +#ifdef _LIBC
> +		  /* Now check the alt month.  */
> +		  trp = rp;
> +		  if (match_string (_NL_CURRENT (LC_TIME, ALTMON_1 + cnt), trp)
> +		      && trp > rp_longest)
> +		    {
> +		      rp_longest = trp;
> +		      cnt_longest = cnt;
> +		      if (s.decided == not
> +			  && strcmp (_NL_CURRENT (LC_TIME, ALTMON_1 + cnt),
> +				     alt_month_name[cnt]))
> +			decided_longest = loc;
> +		    }
> +#endif

OK. Look for alternative month name.

>  		}
>  #endif
>  	      if (s.decided != loc
>  		  && (((trp = rp, match_string (month_name[cnt], trp))
>  		       && trp > rp_longest)
>  		      || ((trp = rp, match_string (ab_month_name[cnt], trp))
> -			  && trp > rp_longest)))
> +			  && trp > rp_longest)
> +#ifdef _LIBC
> +		      || ((trp = rp, match_string (alt_month_name[cnt], trp))
> +			  && trp > rp_longest)

OK.

> +#endif
> +	      ))
>  		{
>  		  rp_longest = trp;
>  		  cnt_longest = cnt;
> @@ -1015,6 +1035,10 @@ __strptime_internal (const char *rp, const char *fmt,
> struct tm *tmp,
>  	case 'O':
>  	  switch (*fmt++)
>  	    {
> +	    case 'B':
> +	      /* Match month name.  Reprocess as plain 'B'.  */
> +	      fmt--;
> +	      goto start_over;

OK.

>  	    case 'd':
>  	    case 'e':
>  	      /* Match day of month using alternate numeric symbols.  */
> diff --git a/time/tst-strptime.c b/time/tst-strptime.c
> index 34ad797..bbc1390 100644
> --- a/time/tst-strptime.c
> +++ b/time/tst-strptime.c
> @@ -51,6 +51,12 @@ static const struct
>      6, 0, 0, 1 },
>    { "ja_JP.EUC-JP", "2001 20 \xb7\xee", "%Y %U %a", 1, 140, 4, 21 },
>    { "ja_JP.EUC-JP", "2001 21 \xb7\xee", "%Y %W %a", 1, 140, 4, 21 },
> +  { "pl_PL.UTF-8", "21 lis 2017", "%d %b %Y", 2, 324, 10, 21 },
> +  { "pl_PL.UTF-8", "22 LIS 2017", "%d %B %Y", 3, 325, 10, 22 },
> +  /* TODO: Use the genitive case here as soon as it is added to localedata.  */
> +  { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
> +  /* The nominative case is incorrect here but it is parseable.  */
> +  { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },

Here we need examples of %OB for langauges that do *not* have genitive names
in order to test the conversion of %OB for locales that would have this
definition missing (tests that %OB does fall back to %B).

>  };
>  
>  
> 


-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
                     ` (2 preceding siblings ...)
  2018-01-11  5:05   ` Carlos O'Donell
@ 2018-01-11 11:46   ` Rafal Luzynski
  2018-01-11 12:25     ` Dmitry V. Levin
  2018-01-11 22:26   ` Rafal Luzynski
  4 siblings, 1 reply; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-11 11:46 UTC (permalink / raw)
  To: libc-alpha; +Cc: Dmitry V. Levin

11.01.2018 03:16 "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>
>
> On Tue, Jan 09, 2018 at 12:59:21AM +0100, Rafal Luzynski wrote:
> > [BZ #10871]
> > * locale/C-time.c: Add alternative month names, define them as the
> > same as mon explicitly.
>
> * locale/C-time.c (_nl_C_LC_TIME): Add alternative month names.
>
> I don't understand what do you mean by "define them as the same as mon
> explicitly", though.

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?

> [...]
> > * 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?

> [...]
> > * 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.

But I don't understand what do you mean by "(__strptime_internal)
[_LIBC]: Add ... (whatever is being added)."  It's "Handle %OB format"
what has been added.  And it's not really conditional [_LIBC]; the
difference is that if _LIBC is not defined then %OB format is accepted
but alternative month names are not checked (because alt_month_name does
not exist).  Is this a copy-paste typo?  What else do I miss?

> [...]
> > --- a/locale/langinfo.h
> > +++ b/locale/langinfo.h
> > @@ -100,7 +100,8 @@ enum
> > ABMON_12,
> > #define ABMON_12 ABMON_12
> >
> > - /* 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.
We both are not native speakers so I'd rather trust Zack here.  So far
I don't change this comment unless other native speakers tell me to change.

>
> --
> ldv

Thank you Dmitry for your review.  I fully agree with your other comments
which I don't quote here and I'm applying them locally.

Regards,

Rafal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-11 11:46   ` Rafal Luzynski
@ 2018-01-11 12:25     ` Dmitry V. Levin
  2018-01-11 14:32       ` Zack Weinberg
  2018-01-11 15:44       ` Rafal Luzynski
  0 siblings, 2 replies; 26+ messages in thread
From: Dmitry V. Levin @ 2018-01-11 12:25 UTC (permalink / raw)
  To: libc-alpha

[-- Attachment #1: Type: text/plain, Size: 3405 bytes --]

On Thu, Jan 11, 2018 at 12:46:06PM +0100, Rafal Luzynski wrote:
> 11.01.2018 03:16 "Dmitry V. Levin" <ldv@altlinux.org> wrote:
> > On Tue, Jan 09, 2018 at 12:59:21AM +0100, Rafal Luzynski wrote:
> > > [BZ #10871]
> > > * locale/C-time.c: Add alternative month names, define them as the
> > > same as mon explicitly.
> >
> > * locale/C-time.c (_nl_C_LC_TIME): Add alternative month names.
> >
> > I don't understand what do you mean by "define them as the same as mon
> > explicitly", though.
> 
> 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.

> > [...]
> > > * 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.

> > [...]
> > > * 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.

> But I don't understand what do you mean by "(__strptime_internal)
> [_LIBC]: Add ... (whatever is being added)."  It's "Handle %OB format"
> what has been added.  And it's not really conditional [_LIBC]; the
> difference is that if _LIBC is not defined then %OB format is accepted
> but alternative month names are not checked (because alt_month_name does
> not exist).  Is this a copy-paste typo?  What else do I miss?

If it's not conditional, than [_LIBC] is not needed.

> > [...]
> > > --- a/locale/langinfo.h
> > > +++ b/locale/langinfo.h
> > > @@ -100,7 +100,8 @@ enum
> > > ABMON_12,
> > > #define ABMON_12 ABMON_12
> > >
> > > - /* 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.


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-11 12:25     ` Dmitry V. Levin
@ 2018-01-11 14:32       ` Zack Weinberg
  2018-01-11 15:07         ` Dmitry V. Levin
  2018-01-11 15:44       ` Rafal Luzynski
  1 sibling, 1 reply; 26+ messages in thread
From: Zack Weinberg @ 2018-01-11 14:32 UTC (permalink / raw)
  To: GNU C Library

On Thu, Jan 11, 2018 at 7:25 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Thu, Jan 11, 2018 at 12:46:06PM +0100, Rafal Luzynski wrote:
>> 11.01.2018 03:16 "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>> > On Tue, Jan 09, 2018 at 12:59:21AM +0100, Rafal Luzynski wrote:
>> > > + /* 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.

I did not mean this sentence to be a definitive native-speaker
pronouncement on How To Say This In English.  That _anyone_ finds it
confusing is enough reason to revise it.

But I'm having trouble revising it - I'm sure it should begin with

/* Long month names, in the grammatical form used when ...

but I don't know what to put after "when" anymore.  Perhaps the
problem is that I don't speak any language that needs this feature!
How would *you* describe it?

zw

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-11 14:32       ` Zack Weinberg
@ 2018-01-11 15:07         ` Dmitry V. Levin
  2018-01-11 15:22           ` Zack Weinberg
  0 siblings, 1 reply; 26+ messages in thread
From: Dmitry V. Levin @ 2018-01-11 15:07 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

On Thu, Jan 11, 2018 at 09:32:16AM -0500, Zack Weinberg wrote:
> On Thu, Jan 11, 2018 at 7:25 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> > On Thu, Jan 11, 2018 at 12:46:06PM +0100, Rafal Luzynski wrote:
> >> 11.01.2018 03:16 "Dmitry V. Levin" <ldv@altlinux.org> wrote:
> >> > On Tue, Jan 09, 2018 at 12:59:21AM +0100, Rafal Luzynski wrote:
> >> > > + /* 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.
> 
> I did not mean this sentence to be a definitive native-speaker
> pronouncement on How To Say This In English.  That _anyone_ finds it
> confusing is enough reason to revise it.
> 
> But I'm having trouble revising it - I'm sure it should begin with
> 
> /* Long month names, in the grammatical form used when ...
> 
> but I don't know what to put after "when" anymore.  Perhaps the
> problem is that I don't speak any language that needs this feature!
> How would *you* describe it?

/* Long month names, in the grammatical form used when the month is a part
   of a complete date. */

The closes analogy in English I could think of is "of January" in phrase
"11th of January".


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-11 15:07         ` Dmitry V. Levin
@ 2018-01-11 15:22           ` Zack Weinberg
  0 siblings, 0 replies; 26+ messages in thread
From: Zack Weinberg @ 2018-01-11 15:22 UTC (permalink / raw)
  To: Zack Weinberg, GNU C Library

On Thu, Jan 11, 2018 at 10:07 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Thu, Jan 11, 2018 at 09:32:16AM -0500, Zack Weinberg wrote:
>>
>> I did not mean this sentence to be a definitive native-speaker
>> pronouncement on How To Say This In English.  That _anyone_ finds it
>> confusing is enough reason to revise it.
>>
>> But I'm having trouble revising it - I'm sure it should begin with
>>
>> /* Long month names, in the grammatical form used when ...
>>
>> but I don't know what to put after "when" anymore.  Perhaps the
>> problem is that I don't speak any language that needs this feature!
>> How would *you* describe it?
>
> /* Long month names, in the grammatical form used when the month is a part
>    of a complete date. */

Sounds good to me!  And I think I understand where the confusion came
from now.  In "the month forms part", "forms" was meant to be read as
a _verb_, with essentially the same meaning as your "the month is a
part", but because of the earlier "grammatical form", it sounded like
it was another use of the _noun_ "form", in the plural.  I shall
remember that for future wordsmithing.

zw

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-11 12:25     ` Dmitry V. Levin
  2018-01-11 14:32       ` Zack Weinberg
@ 2018-01-11 15:44       ` Rafal Luzynski
  1 sibling, 0 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-11 15:44 UTC (permalink / raw)
  To: libc-alpha, Dmitry V. Levin

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added (bug 10871).
  2018-01-11  5:05   ` Carlos O'Donell
@ 2018-01-11 19:04     ` Rafal Luzynski
  2018-01-11 19:14       ` Carlos O'Donell
  0 siblings, 1 reply; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-11 19:04 UTC (permalink / raw)
  To: libc-alpha, Carlos O'Donell

11.01.2018 06:01 Carlos O'Donell <carlos@redhat.com> wrote:
>
>
> On 01/08/2018 04:01 PM, Rafal Luzynski wrote:
> > [...]
> > * locale/langinfo.h: New public symbols _NL_ABALTMON_1,
> > _NL_ABALTMON_2, _NL_ABALTMON_3, _NL_ABALTMON_4, _NL_ABALTMON_5,
> > _NL_ABALTMON_6, _NL_ABALTMON_7, _NL_ABALTMON_8, _NL_ABALTMON_9,
> > _NL_ABALTMON_10, _NL_ABALTMON_11, _NL_ABALTMON_12,
> > _NL_WABALTMON_1, _NL_WABALTMON_2, _NL_WABALTMON_3, _NL_WABALTMON_4,
> > _NL_WABALTMON_5, _NL_WABALTMON_6, _NL_WABALTMON_7, _NL_WABALTMON_8,
> > _NL_WABALTMON_9, _NL_WABALTMON_10, _NL_WABALTMON_11, _NL_WABALTMON_12.
> > [...]
>
> Why is there no ABALTMON_* via #ifdef __USE_GNU like there is for ALTMON_*?
> It is OK without them, but seems like a missing useful feature.

Short answer: the idea to define ABALTMON_* did not yet exist when
I was implementing this.  And even now I'm not sure this is the right
moment to add this already.

Long answer: the reason behind defining ALTMON_ via #ifdef __USE_GNU is
that this standard already exists somewhere (in BSD and as accepted but
not yet published future change in POSIX) but while it is not yet officially
specified by POSIX we treat it as a GNU extension (hence #ifdef __USE_GNU)
while __ALTMON_* symbols are defined unconditionally.  But the idea of
introducing abbreviated alternative month names to glibc is originally
mine (although it already exists in other libraries, like ICU) and I did
not think it would be standardized.  So I have defined them as _NL_ABALTMON_*
(as a forever private GNU extension).  However, the idea has been filed
as a POSIX update proposal only on October 27, 2017. [1] I don't think
it will be accepted and published soon.

So, do you guys want me to use a different naming pattern already?

> OK with the test case changes to write UTF-8 directly in the test case string.

A question about it below.

> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

I am going to publish another set of patches, mostly due to the changes
in ChangeLog and commit messages.  Does this Reviewed-by tag apply to the
new patches as well?

> > [...]
> > +/* Some Cyrillic letters in UTF-8. */
> > +#define CYR_n "\xd0\xbd"
> > +#define CYR_o "\xd0\xbe"
> > +#define CYR_ya "\xd1\x8f"
>
> Please encode the UTF-8 directly into the test case.
>
> Developers have to use UTF-8 capable editors, and fonts.

A question about it below.

> > [...]
> > static const struct
> > {
> > const char *locale;
> > @@ -57,6 +62,14 @@ static const struct
> > { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
> > /* The nominative case is incorrect here but it is parseable. */
> > { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
> > + { "pl_PL.UTF-8", "25 lis 2017", "%d %Ob %Y", 6, 328, 10, 25 },
> > + { "ru_RU.UTF-8", "26 " CYR_n CYR_o CYR_ya " 2017", "%d %b %Y",
> > + 0, 329, 10, 26 },
>

Do I understand correctly, should I use lines like these?

+ /* ноя - pronounce: 'noya' - "Nov" (abbreviated "November") in Russian.  */
+ { "ru_RU.UTF-8", "26 ноя 2017", "%d %b %Y", 0, 329, 10, 26 },

Regards,

Rafal


[1] https://sourceware.org/ml/libc-alpha/2017-10/msg01307.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added (bug 10871).
  2018-01-11 19:04     ` Rafal Luzynski
@ 2018-01-11 19:14       ` Carlos O'Donell
  2018-01-12  4:01         ` Rafal Luzynski
  0 siblings, 1 reply; 26+ messages in thread
From: Carlos O'Donell @ 2018-01-11 19:14 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha

On 01/11/2018 11:04 AM, Rafal Luzynski wrote:
> 11.01.2018 06:01 Carlos O'Donell <carlos@redhat.com> wrote:
>>
>>
>> On 01/08/2018 04:01 PM, Rafal Luzynski wrote:
>>> [...]
>>> * locale/langinfo.h: New public symbols _NL_ABALTMON_1,
>>> _NL_ABALTMON_2, _NL_ABALTMON_3, _NL_ABALTMON_4, _NL_ABALTMON_5,
>>> _NL_ABALTMON_6, _NL_ABALTMON_7, _NL_ABALTMON_8, _NL_ABALTMON_9,
>>> _NL_ABALTMON_10, _NL_ABALTMON_11, _NL_ABALTMON_12,
>>> _NL_WABALTMON_1, _NL_WABALTMON_2, _NL_WABALTMON_3, _NL_WABALTMON_4,
>>> _NL_WABALTMON_5, _NL_WABALTMON_6, _NL_WABALTMON_7, _NL_WABALTMON_8,
>>> _NL_WABALTMON_9, _NL_WABALTMON_10, _NL_WABALTMON_11, _NL_WABALTMON_12.
>>> [...]
>>
>> Why is there no ABALTMON_* via #ifdef __USE_GNU like there is for ALTMON_*?
>> It is OK without them, but seems like a missing useful feature.
> 
> Short answer: the idea to define ABALTMON_* did not yet exist when
> I was implementing this.  And even now I'm not sure this is the right
> moment to add this already.
> 
> Long answer: the reason behind defining ALTMON_ via #ifdef __USE_GNU is
> that this standard already exists somewhere (in BSD and as accepted but
> not yet published future change in POSIX) but while it is not yet officially
> specified by POSIX we treat it as a GNU extension (hence #ifdef __USE_GNU)
> while __ALTMON_* symbols are defined unconditionally.  But the idea of
> introducing abbreviated alternative month names to glibc is originally
> mine (although it already exists in other libraries, like ICU) and I did
> not think it would be standardized.  So I have defined them as _NL_ABALTMON_*
> (as a forever private GNU extension).  However, the idea has been filed
> as a POSIX update proposal only on October 27, 2017. [1] I don't think
> it will be accepted and published soon.
> 
> So, do you guys want me to use a different naming pattern already?

Please keep the code as-is. Thank you for explaining the situation, it clarifies
the point. The code is OK to go in as-is.

>> OK with the test case changes to write UTF-8 directly in the test case string.
> 
> A question about it below.
> 
>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 
> I am going to publish another set of patches, mostly due to the changes
> in ChangeLog and commit messages.  Does this Reviewed-by tag apply to the
> new patches as well?

Yes, it applies also to the new patches, as long as no substantive changes
are made to the code. I don't count the ChangeLog as being substantive.

>>> [...]
>>> +/* Some Cyrillic letters in UTF-8. */
>>> +#define CYR_n "\xd0\xbd"
>>> +#define CYR_o "\xd0\xbe"
>>> +#define CYR_ya "\xd1\x8f"
>>
>> Please encode the UTF-8 directly into the test case.
>>
>> Developers have to use UTF-8 capable editors, and fonts.
> 
> A question about it below.
> 
>>> [...]
>>> static const struct
>>> {
>>> const char *locale;
>>> @@ -57,6 +62,14 @@ static const struct
>>> { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
>>> /* The nominative case is incorrect here but it is parseable. */
>>> { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
>>> + { "pl_PL.UTF-8", "25 lis 2017", "%d %Ob %Y", 6, 328, 10, 25 },
>>> + { "ru_RU.UTF-8", "26 " CYR_n CYR_o CYR_ya " 2017", "%d %b %Y",
>>> + 0, 329, 10, 26 },
>>
> 
> Do I understand correctly, should I use lines like these?
> 
> + /* ноя - pronounce: 'noya' - "Nov" (abbreviated "November") in Russian.  */
> + { "ru_RU.UTF-8", "26 ноя 2017", "%d %b %Y", 0, 329, 10, 26 },

Yes, exactly like this.
 
> Regards,
> 
> Rafal
> 
> 
> [1] https://sourceware.org/ml/libc-alpha/2017-10/msg01307.html
> 


-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
                     ` (3 preceding siblings ...)
  2018-01-11 11:46   ` Rafal Luzynski
@ 2018-01-11 22:26   ` Rafal Luzynski
  4 siblings, 0 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-11 22:26 UTC (permalink / raw)
  To: libc-alpha

9.01.2018 00:59 Rafal Luzynski <digitalfreak@lingonborough.com> wrote:
> [...]
> each month name. From now it is precised that nl_langinfo(MON_1)
> series (up to MON_12) and strftime("%B") generate the month names in

Following some old discussion with Zack it should be s/precised/specified/g.
Applying locally.

Regards,

Rafal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-11  5:05   ` Carlos O'Donell
@ 2018-01-12  3:44     ` Rafal Luzynski
  2018-01-12  4:09       ` Carlos O'Donell
  0 siblings, 1 reply; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-12  3:44 UTC (permalink / raw)
  To: libc-alpha, Carlos O'Donell

Thank you Carlos for your reviews.  Please find my comments below:

11.01.2018 06:03 Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 01/08/2018 03:59 PM, Rafal Luzynski wrote:
> > [...]
> > +#ifdef __USE_GNU
> > +# define ALTMON_1 __ALTMON_1
> > +# define ALTMON_2 __ALTMON_2
> > +# define ALTMON_3 __ALTMON_3
> > +# define ALTMON_4 __ALTMON_4
> > +# define ALTMON_5 __ALTMON_5
> > +# define ALTMON_6 __ALTMON_6
> > +# define ALTMON_7 __ALTMON_7
> > +# define ALTMON_8 __ALTMON_8
> > +# define ALTMON_9 __ALTMON_9
> > +# define ALTMON_10 __ALTMON_10
> > +# define ALTMON_11 __ALTMON_11
> > +# define ALTMON_12 __ALTMON_12
>
> OK. Requires _GNU_SOURCE, which is good because this is an extension not
> yet defined by POSIX.

I guess you meant __USE_GNU because that's what my patch uses.  So does
/usr/include/langinfo.h everywhere.  Should I use _GNU_SOURCE instead?

> > [...]
> > diff --git a/time/tst-strptime.c b/time/tst-strptime.c
> > index 34ad797..bbc1390 100644
> > --- a/time/tst-strptime.c
> > +++ b/time/tst-strptime.c
> > @@ -51,6 +51,12 @@ static const struct
> > 6, 0, 0, 1 },
> > { "ja_JP.EUC-JP", "2001 20 \xb7\xee", "%Y %U %a", 1, 140, 4, 21 },
> > { "ja_JP.EUC-JP", "2001 21 \xb7\xee", "%Y %W %a", 1, 140, 4, 21 },
> > + { "pl_PL.UTF-8", "21 lis 2017", "%d %b %Y", 2, 324, 10, 21 },
> > + { "pl_PL.UTF-8", "22 LIS 2017", "%d %B %Y", 3, 325, 10, 22 },
> > + /* TODO: Use the genitive case here as soon as it is added to localedata.
> > */
> > + { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
> > + /* The nominative case is incorrect here but it is parseable. */
> > + { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
>
> Here we need examples of %OB for langauges that do *not* have genitive names
> in order to test the conversion of %OB for locales that would have this
> definition missing (tests that %OB does fall back to %B).

OK, I'm adding en_US.ISO-8859-1 for %B, de_DE.ISO-8859-1 for %b
(those locales look weird but they are already used in this file so
we don't need to add them), fr_FR.UTF-8 for %OB, and then I'll also
add es_ES.UTF-8 for %Ob in another patch which adds %Ob.

Regards,

Rafal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added (bug 10871).
  2018-01-11 19:14       ` Carlos O'Donell
@ 2018-01-12  4:01         ` Rafal Luzynski
  0 siblings, 0 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-12  4:01 UTC (permalink / raw)
  To: libc-alpha, Carlos O'Donell

11.01.2018 20:14 Carlos O'Donell <carlos@redhat.com> wrote:
>
>
> On 01/11/2018 11:04 AM, Rafal Luzynski wrote:
> > [...]
> > So, do you guys want me to use a different naming pattern already?
>
> Please keep the code as-is. Thank you for explaining the situation, it
> clarifies
> the point. The code is OK to go in as-is.

OK

> >> OK with the test case changes to write UTF-8 directly in the test case
> >> string.
> >
> > A question about it below.
> >
> >> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> >
> > I am going to publish another set of patches, mostly due to the changes
> > in ChangeLog and commit messages. Does this Reviewed-by tag apply to the
> > new patches as well?
>
> Yes, it applies also to the new patches, as long as no substantive changes
> are made to the code. I don't count the ChangeLog as being substantive.

OK, great.

> >>> [...]
> >>> static const struct
> >>> {
> >>> const char *locale;
> >>> @@ -57,6 +62,14 @@ static const struct
> >>> { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
> >>> /* The nominative case is incorrect here but it is parseable. */
> >>> { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
> >>> + { "pl_PL.UTF-8", "25 lis 2017", "%d %Ob %Y", 6, 328, 10, 25 },
> >>> + { "ru_RU.UTF-8", "26 " CYR_n CYR_o CYR_ya " 2017", "%d %b %Y",
> >>> + 0, 329, 10, 26 },
> >>
> >
> > Do I understand correctly, should I use lines like these?
> >
> > + /* ноя - pronounce: 'noya' - "Nov" (abbreviated "November") in Russian. */
> > + { "ru_RU.UTF-8", "26 ноя 2017", "%d %b %Y", 0, 329, 10, 26 },
>
> Yes, exactly like this.

OK, done locally.

Regards,

Rafal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 1/5] Implement alternative month names (bug 10871).
  2018-01-12  3:44     ` Rafal Luzynski
@ 2018-01-12  4:09       ` Carlos O'Donell
  0 siblings, 0 replies; 26+ messages in thread
From: Carlos O'Donell @ 2018-01-12  4:09 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha

On 01/11/2018 07:44 PM, Rafal Luzynski wrote:
> Thank you Carlos for your reviews.  Please find my comments below:
> 
> 11.01.2018 06:03 Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> On 01/08/2018 03:59 PM, Rafal Luzynski wrote:
>>> [...]
>>> +#ifdef __USE_GNU
>>> +# define ALTMON_1 __ALTMON_1
>>> +# define ALTMON_2 __ALTMON_2
>>> +# define ALTMON_3 __ALTMON_3
>>> +# define ALTMON_4 __ALTMON_4
>>> +# define ALTMON_5 __ALTMON_5
>>> +# define ALTMON_6 __ALTMON_6
>>> +# define ALTMON_7 __ALTMON_7
>>> +# define ALTMON_8 __ALTMON_8
>>> +# define ALTMON_9 __ALTMON_9
>>> +# define ALTMON_10 __ALTMON_10
>>> +# define ALTMON_11 __ALTMON_11
>>> +# define ALTMON_12 __ALTMON_12
>>
>> OK. Requires _GNU_SOURCE, which is good because this is an extension not
>> yet defined by POSIX.
> 
> I guess you meant __USE_GNU because that's what my patch uses.  So does
> /usr/include/langinfo.h everywhere.  Should I use _GNU_SOURCE instead?

Your use of __USE_GNU is *correct*, that is the internal macro that glibc
uses when enabling GNU extensions.

The user or developer will not use __USE_GNU, instead they will define
_GNU_SOURCE (feature test macro), and that will under the hood define
__USE_GNU (see include/features.h).

Again, you don't need to change anything here, my "OK." indicates that
I saw everything correct. My subsequent comment is designed to educate
those reading the review and provide insight into what I was looking
for in my review.

>>> [...]
>>> diff --git a/time/tst-strptime.c b/time/tst-strptime.c
>>> index 34ad797..bbc1390 100644
>>> --- a/time/tst-strptime.c
>>> +++ b/time/tst-strptime.c
>>> @@ -51,6 +51,12 @@ static const struct
>>> 6, 0, 0, 1 },
>>> { "ja_JP.EUC-JP", "2001 20 \xb7\xee", "%Y %U %a", 1, 140, 4, 21 },
>>> { "ja_JP.EUC-JP", "2001 21 \xb7\xee", "%Y %W %a", 1, 140, 4, 21 },
>>> + { "pl_PL.UTF-8", "21 lis 2017", "%d %b %Y", 2, 324, 10, 21 },
>>> + { "pl_PL.UTF-8", "22 LIS 2017", "%d %B %Y", 3, 325, 10, 22 },
>>> + /* TODO: Use the genitive case here as soon as it is added to localedata.
>>> */
>>> + { "pl_PL.UTF-8", "23 listopad 2017", "%d %B %Y", 4, 326, 10, 23 },
>>> + /* The nominative case is incorrect here but it is parseable. */
>>> + { "pl_PL.UTF-8", "24 listopad 2017", "%d %OB %Y", 5, 327, 10, 24 },
>>
>> Here we need examples of %OB for langauges that do *not* have genitive names
>> in order to test the conversion of %OB for locales that would have this
>> definition missing (tests that %OB does fall back to %B).
> 
> OK, I'm adding en_US.ISO-8859-1 for %B, de_DE.ISO-8859-1 for %b
> (those locales look weird but they are already used in this file so
> we don't need to add them), fr_FR.UTF-8 for %OB, and then I'll also
> add es_ES.UTF-8 for %Ob in another patch which adds %Ob.

Perfect. Thank you very much.

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v11 5/5] Documentation to the above changes (bug 10871).
  2018-01-11  4:50   ` Carlos O'Donell
@ 2018-01-12  4:21     ` Rafal Luzynski
  0 siblings, 0 replies; 26+ messages in thread
From: Rafal Luzynski @ 2018-01-12  4:21 UTC (permalink / raw)
  To: libc-alpha, Carlos O'Donell

11.01.2018 05:46 Carlos O'Donell <carlos@redhat.com> wrote:
>
>
> On 01/08/2018 04:03 PM, Rafal Luzynski wrote:
> > [...]
> > +As a GNU extension, the @code{O} modifier can be used with these
> > +specifiers; it has no effect, as both grammatical forms of month
> > +names are recognized anyway.
>
> s/anyway//g.

OK, I trust you as an English native speaker.  Done locally.

Regards,

Rafal

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2018-01-12  4:21 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 23:54 [PATCH v11 0/5][BZ 10871] Month names in alternative grammatical case Rafal Luzynski
2018-01-08 23:59 ` [PATCH v11 1/5] Implement alternative month names (bug 10871) Rafal Luzynski
2018-01-09  0:14   ` Rafal Luzynski
2018-01-11  4:20     ` Carlos O'Donell
2018-01-11  2:16   ` Dmitry V. Levin
2018-01-11  5:05   ` Carlos O'Donell
2018-01-12  3:44     ` Rafal Luzynski
2018-01-12  4:09       ` Carlos O'Donell
2018-01-11 11:46   ` Rafal Luzynski
2018-01-11 12:25     ` Dmitry V. Levin
2018-01-11 14:32       ` Zack Weinberg
2018-01-11 15:07         ` Dmitry V. Levin
2018-01-11 15:22           ` Zack Weinberg
2018-01-11 15:44       ` Rafal Luzynski
2018-01-11 22:26   ` Rafal Luzynski
2018-01-09  0:01 ` [PATCH v11 3/5] Abbreviated alternative month names (%Ob) also added " Rafal Luzynski
2018-01-11  5:05   ` Carlos O'Donell
2018-01-11 19:04     ` Rafal Luzynski
2018-01-11 19:14       ` Carlos O'Donell
2018-01-12  4:01         ` Rafal Luzynski
2018-01-09  0:03 ` [PATCH v11 5/5] Documentation to the above changes " Rafal Luzynski
2018-01-11  4:50   ` Carlos O'Donell
2018-01-12  4:21     ` Rafal Luzynski
2018-01-09  0:05 ` [PATCH] pl_PL: Add alternative month names " Rafal Luzynski
2018-01-09  0:20   ` Rafal Luzynski
2018-01-11  4:40   ` Carlos O'Donell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).