incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [PATCH] Update test 22.locale.time.put.mt.cpp to validate results [take 2]
Date Mon, 13 Aug 2007 23:03:15 GMT
Travis Vitek wrote:
>  
> 
> Martin Sebor wrote:
>> Now that I've committed the patch... I have a question about the bit
>> below that I noticed too late:
>>
>> [...]
>>> @@ -160,19 +214,117 @@
>>>  static int
>>>  run_test (int, char**)
>> [...]
>>> +    const char* const possible_locale_options[] = {
>>> +        locale_list, "C\0", 0
>>> +    };
>> Is the purpose of this code to exercise the "C" locale in addition
>> to all the named locales returned from rw_locales()?
>>
>> If so, it's a valuable enhancement to the test since the base facet
>> (i.e., time_put as opposed to time_put_byname) wasn't necessarily
>> being exercised by the test before this change. Good catch!
>>
> Well that isn't the intent and I don't think that is what it does.
> 
> There was some discussion [http://tinyurl.com/2jkqmd] about what to do
> in the case where the no valid locale could be located, either because
> the user specified invalid locale names, or because the system didn't
> have any valid locales. So the test attempts to use the provided or
> enumerated list of locales, and if none of them could be loaded, the "C"
> locale will be used. If, after attempting to load the "C" locale, no
> locale could be loaded a rw_fatal diagnostic will be issued.

I see. I think we should keep it simple. If the user specifies
--locales="foo bar" on the command line and neither foo nor bar
is valid, I'd just give a warning (or an error if you prefer)
that the locales were not valid and be done with it. I'd rather
not try to build too much "intelligence" into the tests.

[...]
> As suggested below, we could modify rw_locales method to optionally
> insert the "C" locale to the head of the locale list. This would ensure
> that the test always had at least one valid locale when invoked without
> the --locales option. It would also nicely generate an rw_fatal
> diagnostic if the user didn't provide a list of valid locales.

Yes, let's do that.

> 
[...]
> Sounds good. If we all agee, then I'll make up a patch for rw_locales()
> and for each of the tests that I added last week.
> 
> I'm thinking that rw_locales should take a bool that indicates the "C"
> locale should be included at the head of the list. Ideally the default
> value would be true, but for compatibility it should be set to false.
> Hmmm. I'm thinking false and the tests that want the new behavior can be
> updated later.

I wouldn't worry about compatibility. The function already returns
the "C" locale on all platforms, we'd just be making it the first
one on all of them (you should probably skip it the next time it
comes up).

> 
> I will also need to add support for a command line option to enable or
> disable this behavior. I'm thinking --use-c-locale=# or --no-c-locale#
> depending on what we decide for the default value mentioned above. Does
> that sound okay?

I don't see the need for that. The user can specify the exact list
of locales using the --locales option.

Martin

Mime
View raw message