Re: [boost] [locale] Formal review of Boost.Locale library starts tomorrow

Subject: Re: [boost] [locale] Formal review of Boost.Locale library starts tomorrow
From: Artyom (artyomtnk_at_[hidden])
Date: 2011-04-08 14:36:08

> 

> AMDG
> 
> Some minor nits when reading the documentation:
> 
> Minor point:  The canonical documentation link
> is libs/locale/index.html.  This should  redirect
> to wherever the documentation actually resides.
> 

I'll do it when it goes to trunk.

> [snip] 
> direct translation:
> the declarations of ngettext and friends  are
> missing the int n parameter, which is  rather
> important.

Yes, good point there is indeed should be "int n" there.
Noted, will be fixed.

> 
> Character Set Conversions:
> 
> Would it make  sense to have an additional
> alternative of replacing characters  that
> couldn't be converted with some default
> text?  I'm used to  seeing ??? and it seems
> as though if the text can't be converted
> it's  better to see that there's something
> there than to have it silently  disappear.
> 

Ok, there are several problems with this.

Boost.Locale supports 3 character set
conversion backends:

   a) ICU
   b) iconv
   c) Win32API (MultiByteTo... 

Only ICU can handle such substitution, and there
is no simple way to do this with iconv and Win32 API
especially in Win32 API (in one of the directions)
where you don't really know where was the problem.

So you have an option to ignore invalid code points
or abort the conversion.

> Boundary analysis:
> 
> The last example can't possibly be  correct.
> It uses map.begin() and map.end() to
> initialize  map.

Yes you are right there is a mistake, it should be:

    mapping<iterator> map(sentence,text.begin(),text.end(),sentence_term);

And also 

   for(int i=0;i<2 && *p!=text.end();i++)

> [snip] some lingual comments

Thanks, I'll fix all these language mistakes.

Artyom