[Freeciv-Dev] Re: [PATCH] I18n of int_to_text() (PR#517)

[Jeff Mallatt <jjm@xxxxxxxxxxxx>]

At 2000/08/22 16:45 , Gaute (B) Strokkenes wrote:
>Jeff Mallatt <jjm@xxxxxxxxxxxx> writes:
>[From the diff:]
>>  lc_grouping = (*(lc->grouping)) ? lc->grouping : "\3\0";
>>  lc_thousands_sep = (*(lc->thousands_sep)) ? lc->thousands_sep : ",";
>
>This is broken.

Not necessarily...

>  A priori, a locale doesn't _have_ to use grouping at
>all, and I'm willing to bet a substantial amount of money that there
>are locales out there that don't.

Sure.  But, pending a definitive interpretation of 'lc->grouping', a strict
interpretation of the vague wording "0 means that the previous element is
used" implies that an 'lc->grouping' of { '\0' } is *illegal* (there is no
"previous element").  The (arguably only) legal way to represent "no
grouping" is { CHAR_MAX } (in fact, this is exactly what my machine returns
-- yes, I'm one of those lazy North Americans :).

However...

>  If the idea is to to cater for the
>case where an American has to forgot to set up the environment
>properly, it's probably a better idea to check if LC_NUMERIC == "C".

You are correct, here.  I agree that your new code is much better.

>I've made an updated patch:
>
>* Checks if LC_NUMERIC == "C" as above.
>* Replaces the cached info with apropriate constants if we're
>  configured with --disable-nls.  This makes init_nls() a lot cleaner.
>* Bomb-proof asserts.
>* Even more comments.

Great.  (Unfortunately, the patch I received didn't apply cleanly.  It was
only the last two hunks that didn't work, so I applied them manually.)

>It also removes support for negative numbers.  The reasons for this
>are:
>
>1) It's hard to do properly.  localeconv()->negative_sign is actually
>   LC_MONETARY rather LC_NUMERIC, and there a slew of other members
>   that describe how to use it.  Since int_to_text() is not used to
>   format monetary amounts (Freeciv uses gold as a universal currency
>   anyway) this is bound to break, one way or another.
>2) Freeciv does not need to format number anywhere anyway.
>   (int_to_text() is only used for populations etc.)
>
>Note that the old int_to_text() did not support negative numbers.
>
>If, on pain of death, support for negative numbers is deemed to be
>necessary, I say we take the cheap way out and just prepend "-".

I think this is fine.  The only place that comes to mind where large
negative numbers may be needed is in the display of the year, and that is
handled by always using "BC" (or the localized equivalent) and simply never
grouping.  (If you have a city with a food surplus of less than -999, it is
really in trouble. :)

Attached is a patch which applies cleanly, removes another #ifdef and
reduces the code in init_nls() even further.

i18n-int_to_text-7.diff.gz
Description: Binary data

jjm