Re: [Libreoffice] [PATCH] More numbering types: one, two, three, ...

Hi Stephan

First, I think extending this from Persian to English already shows the
> biggest flaw of this approach:  Do you want to extend in in this way for all
> languages supported by LibO?

Yes, I was trying to make it more generic so can be used by more languages.
But maybe I should not,

 I would consider such extension to additional languages a localization
> task, a task that typically only consists of translating string resources.

Well at least for persian it is almost translating strings, but there is
some exceptions in translation
which prevent me from using a word by word translation.


> Here, however, someone doing localization would need to add new constants
> to NumberingType.idl and would need to add code to
> defaultnumberingprovider.cxx.  That does not feel right.
OK, I'm just started to work with libreoffice code ;) Just searched for a
similar feature which
recently has been added, and used it as a templated to how to write code.
Please, Would you mind tell me what is right in this case? How should I
write code for localizing numbers
which best fits?

That said, concentrating on details of the code:
> - At least I do not like macros very much.  But at least DEFINE_WORD_TABLE
> is local to a single .cxx.
OK

- In C++, no need for
>  typedef struct {…} NumberWordTable;
>
> Instead, use the shorter
>
>  struct NumberWordTable {…};
OK, thanks to mention.

- "the second table is used for irregular cardinal numbers is not empty":
> should probably read "if not empty"?
Sorry for typos


> - In
>
>  sal_Unicode *(table1[2][8]);
>
> the superfluous parentheses are IMO confusing, and the sal_Unicode data
> should really be const, so rather
>
>  sal_Unicode const * table1[2][8];
>
> - For the Persian characters (that cannot be given visibly in an ASCII-only
> .cxx file, anyway) the practice of specifying sal_Unicode strings as
> sequences of individual characters (a la {0x0635,0x062f,0}) appears
> acceptable.  However, for English strings, {'o','n','e',0} vs. "one" is hard
> to tolerate.  Maybe all the data should be specified as UTF-8 instead, using
> literal "…" strings (the literal Persian UTF-16 characters like 0x0635
> become a little harder to maintain, having to encode them as UTF-8 like
> "\xXX\xYY"), and explicitly converting to rtl::OUString when used.
Thanks,
Yes, but I think as you said it is better to use UTF-8, one of my problems
was defining a two dimensional
array to hold strings with variable length, first I used
(sal_Unicode[]){0x0635, ...}, but a feedback from
community was "It's not C++03 compatible", so used constant length arrays
which do not like them.
But by changing it to UTF-8, it even makes the persian strings more
tolerable.

Thanks for useful comments ;)

Thanks

> -Stephan