[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Get rid of "Can't recode string"

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2005-02-04 22:38:51 CET

On Fri, 4 Feb 2005 kfogel@collab.net wrote:

> "Peter N. Lundblad" <peter@famlundblad.se> writes:
> > Index: subversion/libsvn_subr/utf.c
> 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).
>
Ha, didn't even notice that. I'm not sure this happens very oftne.

> > @@ -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.
>
No, it couldn't, since then it might be hard to translate into some
languages. IN general, we must avoid message framgents because we don't
know that words or even parts of sentences come in the same order in
different languages. The classical example is
printf("%d file%s processed", n, N== 1 ? "" : "s");
but it applies here too. We've been removed such cases during _() work.
YOu may read more abou thtat in the gettext manual.

> 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.
>

They could use the same format, but they escape different characters. I'm
not sure there is much to share here, if we don't make it table-dirven
ofcourse.

Thanks for reviewing,
//Peter

(BTW, I am going through your 12801 review, but I have to do it in
pieces:-)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Feb 4 22:39:26 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.