Re: More i18n

[ludo@chbouib.org (Ludovic Courtès)]

Hi Kevin,

I basically agree with most of your remarks.  Here are some I don't
quite agree with or that deserve discussion.

Kevin Ryde <user42@zip.com.au> writes:

>> +* i18n Introduction::             Introduction to Guile's i18n support.
>
> "i18n" is a horrible geek-ish abbreviation, much better written in
> full in the manual.

Agreed, but that's only the name of the Info node.  Changing it here
would make the line too long for Info.

>> +@defvr {Scheme Variable} %global-locale
>
> I'm worried if that should be a variable, rather than a function.

I wondered too.  My conclusion is that there's nothing to be afraid of:
after all, it's a special object and we have full control over it
(pretty much like the EOF object, for instance[*]).

>> +This function is based on the C library's @code{strtod} function
>
> Not any more?  Or just in a broadly similar style?

It's based on `strtod', really---I think you're mixing this and
`number->locale-string'.

>> +@deffn {Scheme Procedure} locale-yes-regexp [locale]
>> +@deffnx {Scheme Procedure} locale-no-regexp [locale]
>
> These aren't gnu extensions are they?

No, they're not:

  http://www.opengroup.org/onlinepubs/009695399/basedefs/langinfo.h.html

>> +(define (locale-encoding . locale)
>> +  (apply nl-langinfo CODESET locale))
>
> I think you should implement almost all these basic bits in C, and not
> have a big nl-langinfo.  It'll be easier for a future localeconv
> conditionals, or some of the DOS apis.  It'll also be smaller and
> faster to load and run.
>
>> +(define-vector-langinfo-mapping locale-day-short
>> +  (ABDAY_1 ABDAY_2 ABDAY_3 ABDAY_4 ABDAY_5 ABDAY_6 ABDAY_7)
>> +  ("Sun" "Mon" "Tue" "Wed" "Thu" "Fri" "Sat"))
>
> These in C too I think.

I'm not sure about it.  Writing all those bits in C would have clear
short-term advantages, mainly faster loading.  That said, beside the
fact that I'm a bit lazy and don't really feel like rewriting the thing
in C ATM ;-), I think the above Scheme constructs are easier to work
with (to maintain, to add `localeconv' support, etc.) than a C
equivalent.  So I'd be tempted to keep them as is.

(Besides, I don't think it's very high priority, especially since it
doesn't have any impact on the API itself.)

>> number formatting ...
>>
>> +    (let loop ((index (- strlen 1))
>> +               (grouping grouping)
>> +               (since-separator 0)
>> +               (result ""))
>
> Consider breaking out of the loop when grouping is null, and in any
> case going group by group instead of character by character.

Yes, indeed!

> The result could be built as a list of strings to be concatenated at
> the end too (string-concatenate/reverse or whatever it is).

What does it buy us to apply `string-append' at the end rather than
`string-append' at each step?

>>  ;;; Local Variables:
>>  ;;; coding: latin-1
>
> The fallbacks are all ascii aren't they?

I prefer to explicitly specify the encoding of non-ASCII files.


Thanks for the detailed review!

Ludo'.

[*] Actually, the EOF object _is_ available in R6RS, but it is returned
    by the `eof-object' procedure.  The following post sheds some light
    on this decision:
    http://www.r6rs.org/r6rs-editors/2006-July/001536.html .  I'm not
    sure this reasoning would apply well here: We already have two
    implementations of `%global-locale' and both can happily deal with
    it.



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel