public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] strftime: Improve the width of alternate representation for year [BZ #23758]
@ 2019-01-06  6:30 TAMUKI Shoichi
  2019-01-06  6:36 ` [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. " TAMUKI Shoichi
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-06  6:30 UTC (permalink / raw)
  To: libc-alpha

Hello,

This is the new set of patches.  I have split the patch into 5 parts.

Changes include:

Patch 1: strftime: Add missing uses of L_ macro, etc.
Patch 2: strftime: Set the default width of "%Ey" to 2
Patch 3: strftime: Pass the additional flags from "%EY" to "%Ey"
Patch 4: strftime: Add tst-strfitme2
Patch 5: strftime: Document the description for "%EC", "%Ey", and "%EY"

Regards,
TAMUKI Shoichi

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

* [PATCH v5 4/5] strftime: Add tst-strfitme2 [BZ #23758]
  2019-01-06  6:30 [PATCH v5 0/5] strftime: Improve the width of alternate representation for year [BZ #23758] TAMUKI Shoichi
                   ` (2 preceding siblings ...)
  2019-01-06  6:36 ` [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 " TAMUKI Shoichi
@ 2019-01-06  6:36 ` TAMUKI Shoichi
  2019-01-06  6:54 ` [PATCH v5 5/5] strftime: Document the description for "%EC", "%Ey", and "%EY" " TAMUKI Shoichi
  4 siblings, 0 replies; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-06  6:36 UTC (permalink / raw)
  To: libc-alpha

Add a test to verify the behavior of strftime on alternative
representation for year.

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

ChangeLog:

	[BZ #23758]
	* time/Makefile: Add tst-strftime2 to tests.  Also add ja_JP.UTF-8,
	lo_LA.UTF-8, and th_TH.UTF-8 to LOCALES.
	* time/tst-strftime2.c: New file.
---
 time/Makefile        |   5 +-
 time/tst-strftime2.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 2 deletions(-)
 create mode 100644 time/tst-strftime2.c

diff --git a/time/Makefile b/time/Makefile
index d23ba2dee6e..5c6304ece1d 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -43,13 +43,14 @@ tests	:= test_time clocktest tst-posixtz tst-strptime tst_wcsftime \
 	   tst-getdate tst-mktime tst-mktime2 tst-ftime_l tst-strftime \
 	   tst-mktime3 tst-strptime2 bug-asctime bug-asctime_r bug-mktime1 \
 	   tst-strptime3 bug-getdate1 tst-strptime-whitespace tst-ftime \
-	   tst-tzname tst-y2039 bug-mktime4
+	   tst-tzname tst-y2039 bug-mktime4 tst-strftime2
 
 include ../Rules
 
 ifeq ($(run-built-tests),yes)
 LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP fr_FR.UTF-8 \
-	   es_ES.UTF-8 pl_PL.UTF-8 ru_RU.UTF-8
+	   es_ES.UTF-8 pl_PL.UTF-8 ru_RU.UTF-8 \
+	   ja_JP.UTF-8 lo_LA.UTF-8 th_TH.UTF-8
 include ../gen-locales.mk
 
 $(objpfx)tst-ftime_l.out: $(gen-locales)
diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
new file mode 100644
index 00000000000..756c841ba9c
--- /dev/null
+++ b/time/tst-strftime2.c
@@ -0,0 +1,133 @@
+/* Verify the behavior of strftime on alternate representation for year.
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <locale.h>
+#include <time.h>
+#include <stdio.h>
+#include <string.h>
+
+static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8" };
+#define nlocales (sizeof (locales) / sizeof (locales[0]))
+
+static const char *formats[] = { "%EY", "%_EY", "%-EY" };
+#define nformats (sizeof (formats) / sizeof (formats[0]))
+
+static const struct
+{
+  const int d, m, y;
+} dates[] =
+  {
+    { 1, 3, 88 },
+    { 7, 0, 89 },
+    { 8, 0, 89 },
+    { 1, 3, 90 },
+    { 1, 3, 97 },
+    { 1, 3, 98 }
+  };
+#define ndates (sizeof (dates) / sizeof (dates[0]))
+
+static char ref[nlocales][nformats][ndates][100];
+
+static void
+mkreftable (void)
+{
+  int i, j, k;
+  char era[10];
+  static const int yrj[] = { 63, 64, 1, 2, 9, 10 };
+  static const int yrb[] = { 2531, 2532, 2532, 2533, 2540, 2541 };
+
+  for (i = 0; i < nlocales; i++)
+    for (j = 0; j < nformats; j++)
+      for (k = 0; k < ndates; k++)
+	{
+	  if (i == 0)
+	    {
+	      sprintf (era, "%s", (k < 2) ? "\xe6\x98\xad\xe5\x92\x8c"
+					  : "\xe5\xb9\xb3\xe6\x88\x90");
+	      if (yrj[k] == 1)
+		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);
+	      else
+		{
+		  if (j == 0)
+		    sprintf (ref[i][j][k], "%s%02d\xe5\xb9\xb4", era, yrj[k]);
+		  else if (j == 1)
+		    sprintf (ref[i][j][k], "%s%2d\xe5\xb9\xb4", era, yrj[k]);
+		  else
+		    sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrj[k]);
+		}
+	    }
+	  else if (i == 1)
+	    {
+	      sprintf (era, "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ");
+	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
+	    }
+	  else
+	    {
+	      sprintf (era, "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ");
+	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
+	    }
+	}
+}
+
+static int
+do_test (void)
+{
+  int i, j, k, result = 0;
+  struct tm ttm;
+  char date[11], buf[100];
+  size_t r, e;
+
+  mkreftable ();
+  for (i = 0; i < nlocales; i++)
+    {
+      if (setlocale (LC_ALL, locales[i]) == NULL)
+	{
+	  printf ("locale %s does not exist, skipping...\n", locales[i]);
+	  continue;
+	}
+      printf ("[%s]\n", locales[i]);
+      for (j = 0; j < nformats; j++)
+	{
+	  for (k = 0; k < ndates; k++)
+	    {
+	      ttm.tm_mday = dates[k].d;
+	      ttm.tm_mon  = dates[k].m;
+	      ttm.tm_year = dates[k].y;
+	      strftime (date, sizeof (date), "%F", &ttm);
+	      r = strftime (buf, sizeof (buf), formats[j], &ttm);
+	      e = strlen (ref[i][j][k]);
+	      printf ("%s\t\"%s\"\t\"%s\"", date, formats[j], buf);
+	      if (strcmp (buf, ref[i][j][k]) != 0)
+		{
+		  printf ("\tshould be \"%s\"", ref[i][j][k]);
+		  if (r != e)
+		    printf ("\tgot: %zu, expected: %zu", r, e);
+		  result = 1;
+		}
+	      else
+		printf ("\tOK");
+	      putchar ('\n');
+	    }
+	  putchar ('\n');
+	}
+    }
+  return result;
+}
+
+#include <support/test-driver.c>
-- 
2.12.2

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

* [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-06  6:30 [PATCH v5 0/5] strftime: Improve the width of alternate representation for year [BZ #23758] TAMUKI Shoichi
@ 2019-01-06  6:36 ` TAMUKI Shoichi
  2019-01-09 10:08   ` Rafal Luzynski
  2019-01-06  6:36 ` [PATCH v5 3/5] strftime: Pass the additional flags from "%EY" to "%Ey" " TAMUKI Shoichi
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-06  6:36 UTC (permalink / raw)
  To: libc-alpha

At first, make an unrelated changes for the consistency.

ChangeLog:

	[BZ #23758]
	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
	macro, also add a missing space after the cast of _NL_CURRENT.
---
 time/strftime_l.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/time/strftime_l.c b/time/strftime_l.c
index 6919749c630..7ba4179de3e 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -820,7 +820,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 	  if (modifier == L_('O'))
 	    goto bad_format;
 #ifdef _NL_CURRENT
-	  if (! (modifier == 'E'
+	  if (! (modifier == L_('E')
 		 && (*(subfmt =
 		       (const CHAR_T *) _NL_CURRENT (LC_TIME,
 						     NLW(ERA_D_T_FMT)))
@@ -917,7 +917,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 #ifdef _NL_CURRENT
 	  if (! (modifier == L_('E')
 		 && (*(subfmt =
-		       (const CHAR_T *)_NL_CURRENT (LC_TIME, NLW(ERA_D_FMT)))
+		       (const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(ERA_D_FMT)))
 		     != L_('\0'))))
 	    subfmt = (const CHAR_T *) _NL_CURRENT (LC_TIME, NLW(D_FMT));
 	  goto subformat;
@@ -1262,7 +1262,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 	  DO_NUMBER (1, tp->tm_wday);
 
 	case L_('Y'):
-	  if (modifier == 'E')
+	  if (modifier == L_('E'))
 	    {
 #if HAVE_STRUCT_ERA_ENTRY
 	      struct era_entry *era = _nl_get_era_entry (tp HELPER_LOCALE_ARG);
-- 
2.12.2

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

* [PATCH v5 3/5] strftime: Pass the additional flags from "%EY" to "%Ey" [BZ #23758]
  2019-01-06  6:30 [PATCH v5 0/5] strftime: Improve the width of alternate representation for year [BZ #23758] TAMUKI Shoichi
  2019-01-06  6:36 ` [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. " TAMUKI Shoichi
@ 2019-01-06  6:36 ` TAMUKI Shoichi
  2019-01-06  6:36 ` [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 " TAMUKI Shoichi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-06  6:36 UTC (permalink / raw)
  To: libc-alpha

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

ChangeLog:

	[BZ #23758]
	* time/strftime_l.c (__strftime_internal): Add argument yr_spec to
	override padding for "%Ey".
	If an optional flag ('_' or '-') is specified to "%EY", the "%Ey" in
	subformat is interpreted as if decorated with the appropriate flag.
---
 time/strftime_l.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/time/strftime_l.c b/time/strftime_l.c
index cbe08e7afb4..12d7c0e8744 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -434,7 +434,7 @@ static CHAR_T const month_name[][10] =
 #endif
 
 static size_t __strftime_internal (CHAR_T *, size_t, const CHAR_T *,
-				   const struct tm *, bool *
+				   const struct tm *, int *, bool *
 				   ut_argument_spec
 				   LOCALE_PARAM) __THROW;
 
@@ -456,8 +456,9 @@ my_strftime (CHAR_T *s, size_t maxsize, const CHAR_T *format,
   tmcopy = *tp;
   tp = &tmcopy;
 #endif
+  int yr_spec = 0;		/* Override padding for "%Ey".  */
   bool tzset_called = false;
-  return __strftime_internal (s, maxsize, format, tp, &tzset_called
+  return __strftime_internal (s, maxsize, format, tp, &yr_spec, &tzset_called
 			      ut_argument LOCALE_ARG);
 }
 #ifdef _LIBC
@@ -466,7 +467,7 @@ libc_hidden_def (my_strftime)
 
 static size_t
 __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
-		     const struct tm *tp, bool *tzset_called
+		     const struct tm *tp, int *yr_spec, bool *tzset_called
 		     ut_argument_spec LOCALE_PARAM)
 {
 #if defined _LIBC && defined USE_IN_EXTENDED_LOCALE_MODEL
@@ -838,11 +839,12 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 	  {
 	    CHAR_T *old_start = p;
 	    size_t len = __strftime_internal (NULL, (size_t) -1, subfmt,
-					      tp, tzset_called ut_argument
-					      LOCALE_ARG);
+					      tp, yr_spec, tzset_called
+					      ut_argument LOCALE_ARG);
 	    add (len, __strftime_internal (p, maxsize - i, subfmt,
-					   tp, tzset_called ut_argument
-					   LOCALE_ARG));
+					   tp, yr_spec, tzset_called
+					   ut_argument LOCALE_ARG));
+	    *yr_spec = 0;
 
 	    if (to_uppcase)
 	      while (old_start < p)
@@ -1273,6 +1275,8 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 # else
 		  subfmt = era->era_format;
 # endif
+		  if (pad != 0)
+		    *yr_spec = pad;
 		  goto subformat;
 		}
 #else
@@ -1294,6 +1298,8 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 	      if (era)
 		{
 		  int delta = tp->tm_year - era->start_date[0];
+		  if (*yr_spec != 0)
+		    pad = *yr_spec;
 		  DO_NUMBER (2, (era->offset
 				 + delta * era->absolute_direction));
 		}
-- 
2.12.2

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

* [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 [BZ #23758]
  2019-01-06  6:30 [PATCH v5 0/5] strftime: Improve the width of alternate representation for year [BZ #23758] TAMUKI Shoichi
  2019-01-06  6:36 ` [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. " TAMUKI Shoichi
  2019-01-06  6:36 ` [PATCH v5 3/5] strftime: Pass the additional flags from "%EY" to "%Ey" " TAMUKI Shoichi
@ 2019-01-06  6:36 ` TAMUKI Shoichi
  2019-01-09 10:13   ` Rafal Luzynski
  2019-01-06  6:36 ` [PATCH v5 4/5] strftime: Add tst-strfitme2 " TAMUKI Shoichi
  2019-01-06  6:54 ` [PATCH v5 5/5] strftime: Document the description for "%EC", "%Ey", and "%EY" " TAMUKI Shoichi
  4 siblings, 1 reply; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-06  6:36 UTC (permalink / raw)
  To: libc-alpha

The Japanese era name is scheduled to be changed on May 1, 2019.
Prior to this, change the alternative representation for year in
strftime to pad the number with zero to keep it constant width, so
that prevent the trouble we saw in the past from becoming obvious
again from the year after the era name changes onward.

Since only one Japanese era name is used by each emperor's reign as
lately, it is rare that the year ends in one digit or lasts more than
three digits.  In addition, the default width of month, day, hour,
minute, and second is 2, so adjust the default width of year the same
as them, and then the whole display balance is improved.  Therefore,
it would be reasonable to set the default width padding with zero of
"%Ey" to 2.

ChangeLog:

	[BZ #23758]
	* time/strftime_l.c (__strftime_internal): Set the default width
	padding with zero of "%Ey" to 2.
---
 time/strftime_l.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/time/strftime_l.c b/time/strftime_l.c
index 7ba4179de3e..cbe08e7afb4 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -1294,7 +1294,7 @@ __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 	      if (era)
 		{
 		  int delta = tp->tm_year - era->start_date[0];
-		  DO_NUMBER (1, (era->offset
+		  DO_NUMBER (2, (era->offset
 				 + delta * era->absolute_direction));
 		}
 #else
-- 
2.12.2

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

* [PATCH v5 5/5] strftime: Document the description for "%EC", "%Ey", and "%EY" [BZ #23758]
  2019-01-06  6:30 [PATCH v5 0/5] strftime: Improve the width of alternate representation for year [BZ #23758] TAMUKI Shoichi
                   ` (3 preceding siblings ...)
  2019-01-06  6:36 ` [PATCH v5 4/5] strftime: Add tst-strfitme2 " TAMUKI Shoichi
@ 2019-01-06  6:54 ` TAMUKI Shoichi
  4 siblings, 0 replies; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-06  6:54 UTC (permalink / raw)
  To: libc-alpha

Document the description for "%EC", "%Ey", and "%EY" for strftime.
Also, fix the wording to "alternative" rather than "alternate".

ChangeLog:

	[BZ #23758]
	* NEWS: Mention the change.
	* manual/time.texi (strftime): Document the desctiption for "%EC",
	"%Ey", and "%EY". Also, fix the wording to "alternative" rather than
	"alternate".
---
 NEWS             |  7 +++++++
 manual/time.texi | 21 ++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 4c8bc924ce3..835f5c2ba22 100644
--- a/NEWS
+++ b/NEWS
@@ -52,6 +52,13 @@ Major new features:
     - C-SKY ABIV2 soft-float little-endian
     - C-SKY ABIV2 hard-float little-endian
 
+* Improve the width of alternative representation for year in
+  strftime.  For %Ey conversion specifier, the default action is now
+  to pad the number with zero to keep minimum 2 digits, similar to %y.
+  Also, the optional flag (either _ or -) can be used for %EY, so that
+  the internal %Ey is interpreted as if decorated with the appropriate
+  flag.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * The glibc.tune tunable namespace has been renamed to glibc.cpu and the
diff --git a/manual/time.texi b/manual/time.texi
index 4d154452eba..6c3bfebb8a0 100644
--- a/manual/time.texi
+++ b/manual/time.texi
@@ -1339,7 +1339,7 @@ POSIX.2-1992 and by @w{ISO C99}, are:
 
 @table @code
 @item E
-Use the locale's alternate representation for date and time.  This
+Use the locale's alternative representation for date and time.  This
 modifier applies to the @code{%c}, @code{%C}, @code{%x}, @code{%X},
 @code{%y} and @code{%Y} format specifiers.  In a Japanese locale, for
 example, @code{%Ex} might yield a date format based on the Japanese
@@ -1347,7 +1347,7 @@ Emperors' reigns.
 
 @item O
 With all format specifiers that produce numbers: use the locale's
-alternate numeric symbols.
+alternative numeric symbols.
 
 With @code{%B}, @code{%b}, and @code{%h}: use the grammatical form for
 month names that is appropriate when the month is named by itself,
@@ -1355,7 +1355,7 @@ 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
+If the format supports the modifier but no alternative representation
 is available, it is ignored.
 
 The conversion specifier ends with a format specifier taken from the
@@ -1393,6 +1393,9 @@ The preferred calendar time representation for the current locale.
 The century of the year.  This is equivalent to the greatest integer not
 greater than the year divided by 100.
 
+If the @code{E} modifier is specified (@code{%EC}), the locale's
+alternative representation for year (the era name) is used instead.
+
 This format was first standardized by POSIX.2-1992 and by @w{ISO C99}.
 
 @item %d
@@ -1568,10 +1571,22 @@ The preferred time of day representation for the current locale.
 The year without a century as a decimal number (range @code{00} through
 @code{99}).  This is equivalent to the year modulo 100.
 
+If the @code{E} modifier is specified (@code{%Ey}), the locale's
+alternative representation for year (the era year) is used instead.
+The default action is to pad the number with zero to keep minimum 2
+digits, similar to @code{%y}.
+
 @item %Y
 The year as a decimal number, using the Gregorian calendar.  Years
 before the year @code{1} are numbered @code{0}, @code{-1}, and so on.
 
+If the @code{E} modifier is specified (@code{%EY}), the locale's
+alternative representation for year (generally the combination of
+@code{%EC} and @code{%Ey}) is used instead.  In this case, the
+optional flag (either @code{_} or @code{-}) can be used, so that the
+internal @code{%Ey} is interpreted as if decorated with the
+appropriate flag.
+
 @item %z
 @w{RFC 822}/@w{ISO 8601:1988} style numeric time zone (e.g.,
 @code{-0600} or @code{+0100}), or nothing if no time zone is
-- 
2.12.2

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

* Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-06  6:36 ` [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. " TAMUKI Shoichi
@ 2019-01-09 10:08   ` Rafal Luzynski
  2019-01-09 10:21     ` Siddhesh Poyarekar
  2019-01-10  0:46     ` TAMUKI Shoichi
  0 siblings, 2 replies; 17+ messages in thread
From: Rafal Luzynski @ 2019-01-09 10:08 UTC (permalink / raw)
  To: TAMUKI Shoichi, libc-alpha; +Cc: Siddhesh Poyarekar

Hello Tamuki Shoichi,

Thank you for the next version of your patches.

1. Please remove any reference to [BZ #23758] from this patch
because it is not related with the bug.  The changes are minor
and not visible for the users therefore they don't need any
Bugzilla report, documentation, etc.

2. Regarding the subject of this email, which is also the first
line of the commit message, I would write something like this:

"strftime: Consequently use the "L_" macro with character literals."

As always, I am not a native speaker so other people may provide
better hints.

6.01.2019 07:33 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> At first, make an unrelated changes for the consistency.

If it is the part of the commit message then something like:

"Make unrelated changes for the consistency."

(The core problem is that "an" is incorrect for plural numbers.)

> 
> ChangeLog:
> 
> 	[BZ #23758]
> 	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
> 	macro, also add a missing space after the cast of _NL_CURRENT.

Good but again, Bugzilla mention should be removed and "missing
uses" seems incorrect to me, "Add "L_" macros" or "Use "L_" macros"
is sufficient and seems correct to me.

I cut the patch body here because it is correct and trivial, also
I think it does not introduce any changes in the binary code but
it's good because it improves the readability of the source code.
Therefore I think it is OK to push it now with the changes above.
But due to the freeze period I'd like to hear "OK" from Siddhesh
therefore I'm adding CC: to him.

Regards,

Rafal

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

* Re: [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 [BZ #23758]
  2019-01-06  6:36 ` [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 " TAMUKI Shoichi
@ 2019-01-09 10:13   ` Rafal Luzynski
  2019-01-09 10:25     ` Siddhesh Poyarekar
  2019-01-10  0:46     ` TAMUKI Shoichi
  0 siblings, 2 replies; 17+ messages in thread
From: Rafal Luzynski @ 2019-01-09 10:13 UTC (permalink / raw)
  To: TAMUKI Shoichi, libc-alpha; +Cc: Siddhesh Poyarekar

This patch is actually just a one-liner and it fixes the problem.
I would prefer if the patch 5/5 was split and the documentation
relating to the width of %Ey was merged with this patch and pushed
together but we can also push it now and work on the documentation
later.

Siddhesh (CC:), are you OK to push this patch now and work on
the documentation later?

Other people are welcome to review the commit comment and provide
feedback related with the language correctness.

Regards,

Rafal

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

* Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-09 10:08   ` Rafal Luzynski
@ 2019-01-09 10:21     ` Siddhesh Poyarekar
  2019-01-09 22:58       ` Rafal Luzynski
  2019-01-10  0:46     ` TAMUKI Shoichi
  1 sibling, 1 reply; 17+ messages in thread
From: Siddhesh Poyarekar @ 2019-01-09 10:21 UTC (permalink / raw)
  To: Rafal Luzynski, TAMUKI Shoichi, libc-alpha

On 09/01/19 3:32 PM, Rafal Luzynski wrote:
> Hello Tamuki Shoichi,
> 
> Thank you for the next version of your patches.
> 
> 1. Please remove any reference to [BZ #23758] from this patch
> because it is not related with the bug.  The changes are minor
> and not visible for the users therefore they don't need any
> Bugzilla report, documentation, etc.
> 
> 2. Regarding the subject of this email, which is also the first
> line of the commit message, I would write something like this:
> 
> "strftime: Consequently use the "L_" macro with character literals."
> 
> As always, I am not a native speaker so other people may provide
> better hints.
> 
> 6.01.2019 07:33 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
>>
>> At first, make an unrelated changes for the consistency.
> 
> If it is the part of the commit message then something like:
> 
> "Make unrelated changes for the consistency."
> 
> (The core problem is that "an" is incorrect for plural numbers.)
> 
>>
>> ChangeLog:
>>
>> 	[BZ #23758]
>> 	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
>> 	macro, also add a missing space after the cast of _NL_CURRENT.
> 
> Good but again, Bugzilla mention should be removed and "missing
> uses" seems incorrect to me, "Add "L_" macros" or "Use "L_" macros"
> is sufficient and seems correct to me.
> 
> I cut the patch body here because it is correct and trivial, also
> I think it does not introduce any changes in the binary code but
> it's good because it improves the readability of the source code.
> Therefore I think it is OK to push it now with the changes above.
> But due to the freeze period I'd like to hear "OK" from Siddhesh
> therefore I'm adding CC: to him.

OK.

Siddhesh

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

* Re: [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 [BZ #23758]
  2019-01-09 10:13   ` Rafal Luzynski
@ 2019-01-09 10:25     ` Siddhesh Poyarekar
  2019-01-09 23:02       ` Rafal Luzynski
  2019-01-10  0:46     ` TAMUKI Shoichi
  1 sibling, 1 reply; 17+ messages in thread
From: Siddhesh Poyarekar @ 2019-01-09 10:25 UTC (permalink / raw)
  To: Rafal Luzynski, TAMUKI Shoichi, libc-alpha

On 09/01/19 3:37 PM, Rafal Luzynski wrote:
> This patch is actually just a one-liner and it fixes the problem.
> I would prefer if the patch 5/5 was split and the documentation
> relating to the width of %Ey was merged with this patch and pushed
> together but we can also push it now and work on the documentation
> later.
> 
> Siddhesh (CC:), are you OK to push this patch now and work on
> the documentation later?

Patch is OK for inclusion in 2.29, but please push it with the 
documentation; we don't want a situation where a flag is in but not the 
documentation.

Siddhesh

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

* Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-09 10:21     ` Siddhesh Poyarekar
@ 2019-01-09 22:58       ` Rafal Luzynski
  2019-01-11  4:54         ` TAMUKI Shoichi
  0 siblings, 1 reply; 17+ messages in thread
From: Rafal Luzynski @ 2019-01-09 22:58 UTC (permalink / raw)
  To: Siddhesh Poyarekar, TAMUKI Shoichi, libc-alpha

9.01.2019 11:21 Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> 
> On 09/01/19 3:32 PM, Rafal Luzynski wrote:
> > [...]
> > Therefore I think it is OK to push it now with the changes above.
> > But due to the freeze period I'd like to hear "OK" from Siddhesh
> > therefore I'm adding CC: to him.
> 
> OK.

This means: Tamuki Shoichi, please push this patch (only this first one)
with the changes as suggested above. [1]

Do you need an assistance for this?

Regards,

Rafal

[1] https://sourceware.org/ml/libc-alpha/2019-01/msg00193.html

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

* Re: [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 [BZ #23758]
  2019-01-09 10:25     ` Siddhesh Poyarekar
@ 2019-01-09 23:02       ` Rafal Luzynski
  0 siblings, 0 replies; 17+ messages in thread
From: Rafal Luzynski @ 2019-01-09 23:02 UTC (permalink / raw)
  To: Siddhesh Poyarekar, TAMUKI Shoichi, libc-alpha

9.01.2019 11:25 Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> [...]
> Patch is OK for inclusion in 2.29, but please push it with the 
> documentation; we don't want a situation where a flag is in but not the 
> documentation.

That means: please don't push this patch nor anything else (except the patch
1/5, as I mentioned in the previous email).  We need to work on the
documentation first and push only when everything is ready.

Regards,

Rafal

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

* Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-09 10:08   ` Rafal Luzynski
  2019-01-09 10:21     ` Siddhesh Poyarekar
@ 2019-01-10  0:46     ` TAMUKI Shoichi
  1 sibling, 0 replies; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-10  0:46 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha; +Cc: Siddhesh Poyarekar

Hello Rafal,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
Date: Wed, 9 Jan 2019 11:02:25 +0100 (CET)

> 1. Please remove any reference to [BZ #23758] from this patch
> because it is not related with the bug.  The changes are minor
> and not visible for the users therefore they don't need any
> Bugzilla report, documentation, etc.

OK.  I will do that as an independent patch.

> 2. Regarding the subject of this email, which is also the first
> line of the commit message, I would write something like this:
> 
> "strftime: Consequently use the "L_" macro with character literals."
> 
> As always, I am not a native speaker so other people may provide
> better hints.

Thank you.

> > At first, make an unrelated changes for the consistency.
> 
> If it is the part of the commit message then something like:
> 
> "Make unrelated changes for the consistency."
> 
> (The core problem is that "an" is incorrect for plural numbers.)

Thank you.  Also, "At first" is also incorrect, should be "First" (or
"First of all").  But I will not add any of them this time because of
an independent patch.

> > ChangeLog:
> > 
> > 	[BZ #23758]
> > 	* time/strftime_l.c (__strftime_internal): Add missing uses of L_
> > 	macro, also add a missing space after the cast of _NL_CURRENT.
> 
> Good but again, Bugzilla mention should be removed and "missing
> uses" seems incorrect to me, "Add "L_" macros" or "Use "L_" macros"
> is sufficient and seems correct to me.

OK.  I will correct in the next patch.

Regards,
TAMUKI Shoichi

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

* Re: [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 [BZ #23758]
  2019-01-09 10:13   ` Rafal Luzynski
  2019-01-09 10:25     ` Siddhesh Poyarekar
@ 2019-01-10  0:46     ` TAMUKI Shoichi
  1 sibling, 0 replies; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-10  0:46 UTC (permalink / raw)
  To: Rafal Luzynski, libc-alpha; +Cc: Siddhesh Poyarekar

Hello Rafal,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 [BZ #23758]
Date: Wed, 9 Jan 2019 11:07:32 +0100 (CET)

> This patch is actually just a one-liner and it fixes the problem.
> I would prefer if the patch 5/5 was split and the documentation
> relating to the width of %Ey was merged with this patch and pushed
> together but we can also push it now and work on the documentation
> later.

As Siddhesh mentioned, I will revise the patch to include the
documentation.

Regards,
TAMUKI Shoichi

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

* Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-09 22:58       ` Rafal Luzynski
@ 2019-01-11  4:54         ` TAMUKI Shoichi
  2019-01-11 12:07           ` Rafal Luzynski
  2019-01-11 14:21           ` Siddhesh Poyarekar
  0 siblings, 2 replies; 17+ messages in thread
From: TAMUKI Shoichi @ 2019-01-11  4:54 UTC (permalink / raw)
  To: Rafal Luzynski, Siddhesh Poyarekar, libc-alpha

Hello Rafal,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
Date: Wed, 9 Jan 2019 23:58:01 +0100 (CET)

> > > Therefore I think it is OK to push it now with the changes above.
> > > But due to the freeze period I'd like to hear "OK" from Siddhesh
> > > therefore I'm adding CC: to him.
> > 
> > OK.
> 
> This means: Tamuki Shoichi, please push this patch (only this first one)
> with the changes as suggested above. [1]
> 
> Do you need an assistance for this?

I have sent the latest patches reflecting your suggest.

If it does not matter, I will push the patch (first, only the first
one).  This is my first time to push to publish the changes upstream.
Please let me know if there is my sourceware username "tamuki".

Regards,
TAMUKI Shoichi

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

* Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-11  4:54         ` TAMUKI Shoichi
@ 2019-01-11 12:07           ` Rafal Luzynski
  2019-01-11 14:21           ` Siddhesh Poyarekar
  1 sibling, 0 replies; 17+ messages in thread
From: Rafal Luzynski @ 2019-01-11 12:07 UTC (permalink / raw)
  To: TAMUKI Shoichi, Siddhesh Poyarekar, libc-alpha

11.01.2019 05:54 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> Hello Rafal,
> 
> From: Rafal Luzynski <digitalfreak@lingonborough.com>
> Subject: Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc.
> [BZ #23758]
> Date: Wed, 9 Jan 2019 23:58:01 +0100 (CET)
> 
> > [...]
> > Do you need an assistance for this?
> 
> I have sent the latest patches reflecting your suggest.
> 
> If it does not matter, I will push the patch (first, only the first
> one).

Yes, only the first one, with unrelated changes.

> This is my first time to push to publish the changes upstream.
> Please let me know if there is my sourceware username "tamuki".

I am unable to verify this because I don't have any access to the
administrative resources of sourceware.  Other people should do it.

To other people: is there anybody able to assist Tamuki Shoichi
with his first commit to the main repo?  I'm afraid I am unable
to do it today.

Regards,

Rafal

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

* Re: [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. [BZ #23758]
  2019-01-11  4:54         ` TAMUKI Shoichi
  2019-01-11 12:07           ` Rafal Luzynski
@ 2019-01-11 14:21           ` Siddhesh Poyarekar
  1 sibling, 0 replies; 17+ messages in thread
From: Siddhesh Poyarekar @ 2019-01-11 14:21 UTC (permalink / raw)
  To: TAMUKI Shoichi, Rafal Luzynski, libc-alpha

On 11/01/19 10:24 AM, TAMUKI Shoichi wrote:
> If it does not matter, I will push the patch (first, only the first
> one).  This is my first time to push to publish the changes upstream.
> Please let me know if there is my sourceware username "tamuki".

Have you requested an account[1]?  If yes then you should have received 
confirmation and access details.  If not, Rafal should be able to commit 
for you.

Siddhesh

[1] https://sourceware.org/cgi-bin/pdw/ps_form.cgi

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

end of thread, other threads:[~2019-01-11 14:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-06  6:30 [PATCH v5 0/5] strftime: Improve the width of alternate representation for year [BZ #23758] TAMUKI Shoichi
2019-01-06  6:36 ` [PATCH v5 1/5] strftime: Add missing uses of L_ macro, etc. " TAMUKI Shoichi
2019-01-09 10:08   ` Rafal Luzynski
2019-01-09 10:21     ` Siddhesh Poyarekar
2019-01-09 22:58       ` Rafal Luzynski
2019-01-11  4:54         ` TAMUKI Shoichi
2019-01-11 12:07           ` Rafal Luzynski
2019-01-11 14:21           ` Siddhesh Poyarekar
2019-01-10  0:46     ` TAMUKI Shoichi
2019-01-06  6:36 ` [PATCH v5 3/5] strftime: Pass the additional flags from "%EY" to "%Ey" " TAMUKI Shoichi
2019-01-06  6:36 ` [PATCH v5 2/5] strftime: Set the default width of "%Ey" to 2 " TAMUKI Shoichi
2019-01-09 10:13   ` Rafal Luzynski
2019-01-09 10:25     ` Siddhesh Poyarekar
2019-01-09 23:02       ` Rafal Luzynski
2019-01-10  0:46     ` TAMUKI Shoichi
2019-01-06  6:36 ` [PATCH v5 4/5] strftime: Add tst-strfitme2 " TAMUKI Shoichi
2019-01-06  6:54 ` [PATCH v5 5/5] strftime: Document the description for "%EC", "%Ey", and "%EY" " TAMUKI Shoichi

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