Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]

[TAMUKI Shoichi <tamuki at linet dot gr dot jp>]

Hello Rafal-san,

From: TAMUKI Shoichi <tamuki@linet.gr.jp>
Subject: Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
Date: Wed, 20 Mar 2019 18:21:51 +0900

> Since the code for these *_TW locales are exactly same, so we can
> unify these as follows:
> 
> | 	  else if (i >= 3 && i <= 7)  /* {zh,cmn,hak,nan,lzh}_TW  */
> | 	    {
> | 	      [...]
> | 	    }

I forgot to say:

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
Date: Fri, 15 Mar 2019 12:49:27 +0100 (CET)

> @@ -104,6 +116,36 @@ mkreftable (void)
>  	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
>  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
>  	    }
> +	  else if (i == 3)  /* cmn_TW  */
> +	    {
> +	      if (is_before (&dates[k], 1, 1, 1912))
> +		era = "\xe6\xb0\x91\xe5\x89\x8d";
> +	      else
> +		era = "\xe6\xb0\x91\xe5\x9c\x8b";
> +	      if (dates[k].y == 1912)
> +		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);

In your patch, "dates[k].y == 1912" is used instead of "yrc[k] == 1"
to check the first year of the era.  I think it is not desirable to
condition a specific year (1912) here, as it would be cumbersome to
maintain the code if there is an era change in the future.

Yes, I understand that this is because we want to exclude the first
year of before Minguo.  This can be easily solved by making before
Minguo a negative value.  When referring to the actual year, the
absolute value is calculated.

Furthermore, the code can be made common by preparing a variable that
stores the year suffix string for each locale.

So maybe something like this:

| static void
| mkreftable (void)
| {
|   int i, j, k, yr;
|   const char *era, *sfx;
|   static const int yrj[] =
|     {
|       43, 44, 45, 1, 2,
|       63, 64, 1, 2, 9, 10, 22, 23
|     };
|   static const int yrb[] =
|     {
|       2453, 2454, 2455, 2455, 2456,
|       2531, 2532, 2532, 2533, 2540, 2541, 2553, 2554
|     };
|   static const int yrc[] =
|     {
|       -2, -1, 1, 1, 2,
|       77, 78, 78, 79, 86, 87, 99, 100
|     };
| 
|   for (i = 0; i < array_length (locales); i++)
|     for (j = 0; j < array_length (formats); j++)
|       for (k = 0; k < array_length (dates); k++)
| 	{
| 	  if (i == 0)  /* ja_JP  */
| 	    {
| 	      if (is_before (&dates[k], 30, 7, 1912))
| 		era = "\xe6\x98\x8e\xe6\xb2\xbb";
| 	      else if (is_before (&dates[k], 25, 12, 1926))
| 		era = "\xe5\xa4\xa7\xe6\xad\xa3";
| 	      else if (is_before (&dates[k], 8, 1, 1989))
| 		era = "\xe6\x98\xad\xe5\x92\x8c";
| 	      else
| 		era = "\xe5\xb9\xb3\xe6\x88\x90";
| 	      yr = yrj[k], sfx = "\xe5\xb9\xb4";
| 	    }
| 	  else if (i == 1)  /* lo_LA  */
| 	    {
| 	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
| 	      yr = yrb[k], sfx = "";
| 	    }
| 	  else if (i == 2)  /* th_TH  */
| 	    {
| 	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
| 	      yr = yrb[k], sfx = "";
| 	    }
| 	  else if (i >= 3 && i <= 7)  /* {zh,cmn,hak,nan,lzh}_TW  */
| 	    {
| 	      if (is_before (&dates[k], 1, 1, 1912))
| 		era = "\xe6\xb0\x91\xe5\x89\x8d";
| 	      else
| 		era = "\xe6\xb0\x91\xe5\x9c\x8b";
| 	      yr = yrc[k], sfx = "\xe5\xb9\xb4";
| 	    }
| 	  else
| 	    assert (0);  /* Unreachable.  */
| 	  if (yr == 1)
| 	    sprintf (ref[i][j][k], "%s\xe5\x85\x83%s", era, sfx);
| 	  else if (j == 0)
| 	    sprintf (ref[i][j][k], "%s%02d%s", era, abs (yr), sfx);
| 	  else if (j == 1)
| 	    sprintf (ref[i][j][k], "%s%2d%s", era, abs (yr), sfx);
| 	  else
| 	    sprintf (ref[i][j][k], "%s%d%s", era, abs (yr), sfx);
| 	}
| }
| 

In order to use abs function, we need to add #include <stdlib.h>:

| #include <array_length.h>
| #include <stdbool.h>
| #include <assert.h>
| #include <stdlib.h>
| #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",
|     "zh_TW.UTF-8", "cmn_TW.UTF-8", "hak_TW.UTF-8",
|     "nan_TW.UTF-8", "lzh_TW.UTF-8"
|   };
| 

In the initialization of *locales[], I prefer the indentation style
above.

Question:

In the typedef struct, which indentation style is appropriate?

| typedef struct
| {
|   const int d, m, y;
| } date_t;
| 

or

| typedef struct
|   {
|     const int d, m, y;
|   } date_t;
| 

?

Regards,
TAMUKI Shoichi