Re: [boost] [locale] Review: Tutorial

Subject: Re: [boost] [locale] Review: Tutorial
From: Artyom (artyomtnk_at_[hidden])
Date: 2011-04-17 17:10:28

> From: Gevorg Voskanyan <v_gevorg_at_[hidden]>
> 
> index.html:
> 
> "Correct case conversion, case folding and normalization."  Please insert a 
>link 
>
> to a resource explaining the difference between case  conversion and case 
> folding.
> 

Noted.

> std_locales.html:
> 
> [snip]

Noted, thanks 

> 
> "// Set default locale as global" This  comment is misleading. The default 
>locale 
>
> in C++ programs is the 'classic'  locale. std::locale("") is the user's 
>preferred 
>
> locale. Or perhaps we can  use the system's default locale term, but I think 
> "user's preferred" is  better.
> 

May be it is better the "system locale"

> formatting_and_parsing.html:
> 
> Strictly speaking, "the  iostream STL library" is not correct formulation. 
>Should 
>
> be e.g. "the  standard I/O streams".
> 
> why "double now=time(0);" and not "time_t  now=time(0);" ?
> 

No specific reason.

> s/tommorrow/tomorrow/
> 
> s/display in local time  format/display in local time  zone/
> 
> s/as::locale_time/as::local_time/
> s/Locale time is/Local time  is/
> 

Thanks

> conversions.html:
> 
> Add code example calling to_lower and  fold_case on "grüßen" to help users 
> understand the difference between the  two.
> 

Very good point, thanks, very good example!


> messages_formatting.html:
> 
> s/Is change to/Is changed  to/
> 
> Is there a default search path for dictionaries (/usr/share/locale  under 
>*nix), 
>
> or should an application always call add_messages_path()  ?

Always, because it depends on actual installation
prefix.

Usually it is done with some compile time constant.

> 
> "The source text is not copied." Sometimes 'not' is highlighted, and  sometimes 
>
> 'is'. So what does it try to say after all? That the provided  string argument 

> must not die before the translated string is  fetched?

Already noted there is a small error.
std::string is copied no need to keep it, while "char const *" should not
die - usual static text.

> 
> s/tanslate/translate/
> 
> s/Japaneese/Japanese/
> 
> ja  << msg missing semicolon
> 
> s/main()/int main()/
> 
> std::wstring  msg = translate("Do you want to open the 
> file?").str<wchar_t>(some_locale) missing semicolon
> 
> s/GNU  Gettext catalogs has/GNU Gettext catalogs have/
> 
> "is \c n 1 or not" looks  like a doxygen formatting problem. Such problems 
>exist 
>
> in some other places  as well.
> 
> s/Occured/Occurred/
> 
> s/managment/management/
> 

Noted, Will be fixed.

> "Is  there any reason to prefer the Boost.Locale implementation to the original 
>
> GNU Gettext runtime library? In either case I would probably need some of  the 

> GNU tools." Maybe add a c. point about  license
> 

Good point, strange I had forgot the license, probably I mentioned
it in some other place.

> charset_handling.html:
> 
> s/does not provides/does not  provide/
> 
> localized_text_formatting.html:
> 
> s/when we create a  messages/when we create  messages/
> 
> localized_text_formatting.html:
> 
> wcout <<  wformat(L"Today {1,date} I would meet {2} at home") % time(0) % name 

> <<endl  missing semicolon
> 
> int a_and_b = a+b;   please  change to   int a_plus_b =  a+b;
> 
> dates_times_timezones.html:
> 
> s/they fully aware/they are fully  aware/   Or rather, what does 'they' refer 
>to 
>
> here?
> 
> s/than  setting "current day of week"/then setting "current day of week"/
> 
> add  << between some_point and " is the " 
> 
> s/We recommended you look/We  recommend you to look/
> 

Noted, thanks,

> And what date-time range does the ICU backend  support?
> 

Actually time before BC and long after, virtually limited by
range of double.

> I'd like to echo other reviewers that printing date/time should  not print a 
> number by default.

Yes, will be fixed.

> 
> s/alway/always/
> 
> s/and over that  build dates/and over that builds  dates/
> 
> working_with_multiple_locales.html:
> 
> s/get.locale_cache_enabled(true);/gen.locale_cache_enabled(true);/
> s/std::locale  ar=get("ar_EG.UTF-8");/std::locale ar=gen("ar_EG.UTF-8");/
> 
> s/as  parameters in various functions/as parameters to various  functions/
> 
> using_localization_backends.html:
> 
> s/Case handing/Case  handling/
> 
> recommendations_and_myths.html:
> 
> s/it probably the  most/it is probably the most/
> 
> s/and may other applications/and many other  applications/
> 
> building_boost_locale.html:
> 
> s/If your icu build in  placed/If your icu build is placed/
> 
> s/if you relay on auto-linking/if you  rely on auto-linking/
> 
> appendix.html:
> 
> "is depend" substitute with  "is dependent" or "depends"
> 
> s/not bring read performance advantage/not  bring real performance advantage/
> 
> s/would work only of standard  facets/would work only if standard facets/
> 
> s/Boost locale fully  supports/Boost Locale fully supports/
> 
> s/may broke/may  break/
> 
> s/tje/the/
> 

Noted, will be fixed.

> "including all Western, Cyrillic, Hebrew, Thai,  Arabic and CJK encodings." 
> Shouldn't be  "characters"?
> 

Yes

> s/std::sting/std::string/
> 
> s/not indented to be used  in production/not intended to be used in 
production/
> 
> General  remarks:
> 
> Usage of comments in code snippets is inconsistent in the sense  that sometimes 
>
> the comment comes after the code is addresses, the other  times it comes 
>before. 
>
> One of those styles should be used all over the place  - preferably comments 
> coming first as that is the common practice in other  Boost libraries.
> 

Ok, I'll try to make them more consistent

> Would like to see examples of using Boost.Locale facets  directly.
> 

Yes, good point, yet most facets are quite low level and can be
used indirectly more easily

> Would like to see examples of user-defined I/O  inserters/extractors that 
>respect 
>
> the current state of Boost.Locale  formatting as altered by manipulators. E.g. 
>a 
>
> "class money" example which  uses the Boost.Locale currency formatting info for 
>
> its I/O,

Good point.

> or a "class  bigint" example which uses the Boost.Locale number 
> formatting info for its  I/O.
> Bonus points if they fallback to std facets if 
> corresponding  Boost.Locale info is missing from the  stream!

Actually it is not quite possible, at least with ICU backend as
it supports only double, int32 and int64 types, so it wouldn't
be possible to do it.

It can be done somehow for non-ICU backends that provide std::numpunct,

(it is not enough for ICU case as it is too primitive to handle
all required options)

Thanks, Very good points.
  Artyom