You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2005/02/22 18:29:13 UTC

Re: svn commit: r13112 - in trunk/subversion: libsvn_fs_base libsvn_subr

dionisos@tigris.org writes:

> Author: dionisos
> Date: Tue Feb 22 12:07:20 2005
> New Revision: 13112

> --- trunk/subversion/libsvn_subr/utf.c	(original)
> +++ trunk/subversion/libsvn_subr/utf.c	Tue Feb 22 12:07:20 2005
> @@ -544,8 +540,8 @@
>                                               (unsigned char)last[i]), NULL);
>  
>    return svn_error_createf (APR_EINVAL, NULL,
> -                            _("Valid UTF-8 data\n(hex:%s)\n"
> -                              "followed by invalid UTF-8 sequence\n(hex:%s)"),
> +                            _("Valid UTF-8 data (hex:%s)"
> +                              "followed by invalid UTF-8 sequence (hex:%s)"),
>                              valid_txt, invalid_txt);
>  }

Your change is not really compatible with the comment in the code:

  /* We will display at most 24 valid octets (this may split a leading
     multi-byte character) as that should fit on one 80 character line. */

While I would agree that one line error messages are generally better,
I think this one might be an exception.  Is there any particular
reason to avoid "\n", i18n perhaps?

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r13112 - in trunk/subversion: libsvn_fs_base libsvn_subr

Posted by Erik Huelsmann <e....@gmx.net>.
> "Erik Huelsmann" <e....@gmx.net> writes:
> 
> >> While I would agree that one line error messages are generally better,
> >> I think this one might be an exception.  Is there any particular
> >> reason to avoid "\n", i18n perhaps?
> >
> > Our i18n code compensates for the problem that some platforms use CRLF,
> so
> > that's not really the reason. \n characters don't go well with GUI and
> other
> > applications which don't have 80 character wide displays.
> 
> I suppose I prefer the error string with \n characters, but it's
> marginal and I don't mind if it changes.  What I do care about is that
> the comment and the code match, and at present they don't, so you
> should change one or the other.

I thought you objected to the code change. I'll change the comment.

bye,


Erik.

-- 
DSL Komplett von GMX +++ Superg�nstig und stressfrei einsteigen!
AKTION "Kein Einrichtungspreis" nutzen: http://www.gmx.net/de/go/dsl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r13112 - in trunk/subversion: libsvn_fs_base libsvn_subr

Posted by Philip Martin <ph...@codematters.co.uk>.
"Erik Huelsmann" <e....@gmx.net> writes:

>> While I would agree that one line error messages are generally better,
>> I think this one might be an exception.  Is there any particular
>> reason to avoid "\n", i18n perhaps?
>
> Our i18n code compensates for the problem that some platforms use CRLF, so
> that's not really the reason. \n characters don't go well with GUI and other
> applications which don't have 80 character wide displays.

I suppose I prefer the error string with \n characters, but it's
marginal and I don't mind if it changes.  What I do care about is that
the comment and the code match, and at present they don't, so you
should change one or the other.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r13112 - in trunk/subversion: libsvn_fs_base libsvn_subr

Posted by Erik Huelsmann <e....@gmx.net>.
> dionisos@tigris.org writes:
> 
> > Author: dionisos
> > Date: Tue Feb 22 12:07:20 2005
> > New Revision: 13112
> 
> > --- trunk/subversion/libsvn_subr/utf.c	(original)
> > +++ trunk/subversion/libsvn_subr/utf.c	Tue Feb 22 12:07:20 2005
> > @@ -544,8 +540,8 @@
> >                                               (unsigned char)last[i]),
> NULL);
> >  
> >    return svn_error_createf (APR_EINVAL, NULL,
> > -                            _("Valid UTF-8 data\n(hex:%s)\n"
> > -                              "followed by invalid UTF-8
> sequence\n(hex:%s)"),
> > +                            _("Valid UTF-8 data (hex:%s)"
> > +                              "followed by invalid UTF-8 sequence
> (hex:%s)"),
> >                              valid_txt, invalid_txt);
> >  }
> 
> Your change is not really compatible with the comment in the code:
> 
>   /* We will display at most 24 valid octets (this may split a leading
>      multi-byte character) as that should fit on one 80 character line. */
> 
> While I would agree that one line error messages are generally better,
> I think this one might be an exception.  Is there any particular
> reason to avoid "\n", i18n perhaps?

Our i18n code compensates for the problem that some platforms use CRLF, so
that's not really the reason. \n characters don't go well with GUI and other
applications which don't have 80 character wide displays.

Potentially many of our error strings surpass 70 or 80 characters as very
many will have filenames inserted into the string.

Also, there currently still is a problem with our md5 mismatch error
strings: many of them use \n-s to allign the md5s, but on GUIs which *do*
wrap lines nicely, there still is no guarantee of fixed width fonts...

Altogether, the entire keep-errors-below 80 characters idea may just not be
as viable as it looks. :-(


bye,

Erik.

-- 
DSL Komplett von GMX +++ Superg�nstig und stressfrei einsteigen!
AKTION "Kein Einrichtungspreis" nutzen: http://www.gmx.net/de/go/dsl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org