"Peter N. Lundblad" <peter@famlundblad.se> writes:
> Index: subversion/libsvn_subr/utf.c
> ===================================================================
> --- subversion/libsvn_subr/utf.c (revision 12917)
> +++ subversion/libsvn_subr/utf.c (arbetskopia)
> @@ -222,13 +228,27 @@
> return SVN_NO_ERROR;
> }
> if (apr_err != APR_SUCCESS)
> - /* Can't use svn_error_wrap_apr here because it calls functions in
> - this file, leading to infinite recursion. */
> - return svn_error_createf
> - (apr_err, NULL, _("Can't create a converter from '%s' to '%s'"),
> - (topage == APR_LOCALE_CHARSET ? _("native") : topage),
> - (frompage == APR_LOCALE_CHARSET ? _("native") : frompage));
> -
> + {
> + const char *errstr;
> + /* Can't use svn_error_wrap_apr here because it calls functions in
> + this file, leading to infinite recursion. */
> + /* ### Can this first case happen? */
> + if (frompage == APR_LOCALE_CHARSET && topage == APR_LOCALE_CHARSET)
> + errstr = _("Can't create a character converter from native to native");
> + else if (frompage == APR_LOCALE_CHARSET)
> + errstr = apr_psprintf (pool,
> + _("Can't create a character converter from "
> + "native encoding to '%s'"), topage);
> + else if (topage == APR_LOCALE_CHARSET)
> + errstr = apr_psprintf (pool,
> + _("Can't create a character converter from "
> + "'%s' to native encoding"), frompage);
> + else
> + errstr = apr_psprintf (pool,
> + _("Can't create a character converter from "
> + "'%s' to '%s'"), frompage, topage);
> + return svn_error_create (apr_err, NULL, errstr);
> + }
> return SVN_NO_ERROR;
> }
Heh, this is good -- you not only made the error string easier to
understand, you also fixed a bug in the original error construction
(topage and frompage were reversed).
> @@ -356,10 +424,32 @@
>
> /* If we exited the loop with an error, return the error. */
> if (apr_err)
> - /* Can't use svn_error_wrap_apr here because it calls functions in
> - this file, leading to infinite recursion. */
> - return svn_error_create (apr_err, NULL, _("Can't recode string"));
> -
> + {
> + const char *errstr;
> + svn_error_t *err;
> +
> + /* Can't use svn_error_wrap_apr here because it calls functions in
> + this file, leading to infinite recursion. */
> + /* ### Can this happen? */
> + if (node->frompage == APR_LOCALE_CHARSET
> + && node->topage == APR_LOCALE_CHARSET)
> + errstr = _("Can't convert string from native to native encoding:");
> + else if (node->frompage == APR_LOCALE_CHARSET)
> + errstr = apr_psprintf
> + (pool, _("Can't convert string from native encoding to '%s':"),
> + node->topage);
> + else if (node->topage == APR_LOCALE_CHARSET)
> + errstr = apr_psprintf
> + (pool, _("Can't convert string from '%s' to native encoding:"),
> + node->frompage);
> + else
> + errstr = apr_psprintf
> + (pool, _("Can't convert string from '%s' to '%s':"),
> + node->frompage, node->topage);
> + err = svn_error_create (apr_err, NULL, fuzzy_escape (src_data,
> + src_length, pool));
> + return svn_error_create (apr_err, err, errstr);
> + }
Hmmm, but here we have the same logic duplicated. It could be
abstracted out, passing the two pages and the first part of the string
as parameters.
Regarding the new fuzzy_escape() function: I suspect we will want to
coordinate it with VK Sameer's almost-equivalent function in his patch
for issue #2147. That's gotten good commentary on the dev list and is
about ready for commit, just need a spare moment to take care of it.
For now, I'd say commit your fuzzy_escape() as-is, and whoever does
the #2147 commit can try to merge the two.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Feb 4 19:09:01 2005