Danny Trebbien wrote on Sat, Feb 12, 2011 at 17:37:53 -0800:
> Attached is a patch that adds a new test to the subst_translate-test
> test suite (defined in
> `subversion/tests/libsvn_subr/subst_translate-test.c`). Also attached
> is a log message.
>
> The purpose of the new test is to test svn_subst_translate_string2()
> with ENCODING set to NULL. In this case, VALUE is reencoded from the
> system-default language encoding to UTF-8.
Okay.
> [[[
> Add a test of svn_subst_translate_string2() to the subst_translate-test test
> suite.
>
> * subversion/tests/libsvn_subr/subst_translate-test.c
> (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().
> ]]]
> Index: subversion/tests/libsvn_subr/subst_translate-test.c
> ===================================================================
> --- subversion/tests/libsvn_subr/subst_translate-test.c (revision
> 1070074)
> +++ subversion/tests/libsvn_subr/subst_translate-test.c (working copy)
> @@ -21,6 +21,10 @@
> * ====================================================================
> */
>
> +#include <locale.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> #include "../svn_test.h"
>
> #include "svn_types.h"
> @@ -99,7 +103,42 @@ test_svn_subst_translate_string2(apr_pool_t *pool)
> return SVN_NO_ERROR;
> }
>
> +/* Test that when ENCODING is NULL, the system-default language encoding is
> used. */
> static svn_error_t *
> +test_svn_subst_translate_string2_null_encoding(apr_pool_t *pool)
> +{
> + {
> + char orig_lc_all[1024] = { '\0' };
> + svn_string_t *new_value = NULL;
> + svn_boolean_t translated_to_utf8 = FALSE;
> + svn_boolean_t translated_line_endings = TRUE;
> + /* 'Æ', which is 0xc6 in both ISO-8859-1 and Windows-1252 */
> + svn_string_t *source_string = svn_string_create("\xc6", pool);
> +
> + strncpy(orig_lc_all, setlocale(LC_ALL, NULL), sizeof orig_lc_all);
sizeof() with parens please.
> + if ((! setlocale(LC_ALL, "English.1252")) &&
> + (! setlocale(LC_ALL, "German.1252")) &&
> + (! setlocale(LC_ALL, "French.1252")) &&
What are these three? Are they Windows syntax?
> + (! 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.");
The error message is too verbose, and doesn't fit within 80 columns.
> +
> + SVN_ERR(svn_subst_translate_string2(&new_value, &translated_to_utf8,
> + &translated_line_endings,
> + source_string, NULL, FALSE,
> + pool, pool));
> + SVN_TEST_STRING_ASSERT(new_value->data, "\xc3\x86");
> + SVN_TEST_ASSERT(translated_to_utf8 == TRUE);
> + SVN_TEST_ASSERT(translated_line_endings == FALSE);
> +
> + SVN_TEST_ASSERT(setlocale(LC_ALL, orig_lc_all) != NULL);
> + }
So you only restore the locale if the test passed. How much does it
matter?
> +
> + return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> test_repairing_svn_subst_translate_string2(apr_pool_t *pool)
> {
> {
> @@ -171,6 +210,8 @@ struct svn_test_descriptor_t test_funcs[] =
> SVN_TEST_NULL,
> SVN_TEST_PASS2(test_svn_subst_translate_string2,
> "test svn_subst_translate_string2()"),
> + SVN_TEST_PASS2(test_svn_subst_translate_string2_null_encoding,
> + "test svn_subst_translate_string2(), ENCODING = 0"),
I know you are limited to 50 chars, but this "= 0" sacrifices
readability IMHO. IMO, there are clearer alternatives, e.g.:
"test svn_subst_translate_string2(encoding=NULL)"
> SVN_TEST_PASS2(test_repairing_svn_subst_translate_string2,
> "test repairing svn_subst_translate_string2()"),
> SVN_TEST_PASS2(test_svn_subst_translate_cstring2,
