Thanks, r1073377.  See below for some comments.

Danny Trebbien wrote on Sun, Feb 20, 2011 at 09:58:07 -0800:
> Attached is version 3 of the patch and log message.
> 
> 
> >> >> +        (! setlocale(LC_ALL, "en_US.ISO-8859-1")) &&
> >> >> +        (! setlocale(LC_ALL, "en_GB.ISO-8859-1")) &&
> >> >> +        (! setlocale(LC_ALL, "de_DE.ISO-8859-1")))
> >> >> +      return svn_error_createf(SVN_ERR_TEST_SKIPPED, NULL, "None of 
> >> >> the locales English.1252, German.1252, French.1252, en_US.ISO-8859-1, 
> >> >> en_GB.ISO-8859-1, and de_DE.ISO-8859-1 are installed.");
...
> > So, sorry, but I'm not yet convinced that listing all the locales in the
> > log message is a good idea.
> 
> Okay.  I changed it to "Tried %d locales, but none are installed."
> 

Thanks.

> >> +test_svn_subst_translate_string2_null_encoding_helper(apr_pool_t *pool)
> >> +{
> >> +  {
> >> +    svn_string_t *new_value = NULL;
> >> +    svn_boolean_t translated_to_utf8 = FALSE;
> >> +    svn_boolean_t translated_line_endings = TRUE;
> >
> > Why did you initialize these two variables?  The API will always set
> > them for you, so you shouldn't initialize them.
> 
> It's part of the test to check that the API sets them.
> 

Fair enough.

> [[[
> Add a test of svn_subst_translate_string2() to the subst_translate test suite.
> 
> As discussed at:
>   http://thread.gmane.org/gmane.comp.version-control.subversion.devel/125782
>     Message-ID: <AANLkTimo=npf_fqb+pbpkeifxjbnsorgykz5npcax...@mail.gmail.com>
> 
> * subversion/tests/libsvn_subr/subst_translate-test.c
>   (test_svn_subst_translate_string2_null_encoding_helper): New function. It is
>     the core of the new svn_subst_translate_string2_null_encoding test.
>   (test_svn_subst_translate_string2_null_encoding): New test that tests
>     svn_subst_translate_string2() with ENCODING set to NULL.
>   (test_funcs): Add test_svn_subst_translate_string2_null_encoding().
> ]]]

I tweaked this a bit (e.g., adding ARRAY_LEN() which you neglected to).

> +++ subversion/tests/libsvn_subr/subst_translate-test.c       (working copy)
> +/* The body of the svn_subst_translate_string2_null_encoding test. It should
> +   only be called by test_svn_subst_translate_string2_null_encoding(), as 
> this
> +   code assumes that the process locale has been changed to a locale that 
> uses
> +   either CP-1252 or ISO-8859-1 for the default narrow string encoding. */
>  static svn_error_t *
> +test_svn_subst_translate_string2_null_encoding_helper(apr_pool_t *pool)


"narrow" string?  As opposed to a "wide-character" string?

> +test_svn_subst_translate_string2_null_encoding(apr_pool_t *pool)
> +{
> +  char orig_lc_all[256] = { '\0' };
> +  svn_error_t *test_result;
...
> +  strncpy(orig_lc_all, setlocale(LC_ALL, NULL), sizeof (orig_lc_all));
> +

Typically we use pools (apr_pstrdup()).  I haven't changed this.

Reply via email to