On Wed, 23 Feb 2005, Charles Bailey wrote:
> Attached is a patchlet that, when expat fails to parse an hunk of XML,
> appends at least part of the offending hunk to the error message. It
> provides a better jumping-off point for further debugging than the
> current message, especially since the line number often isn't all that
> meaningful. It's not intended to flag the error specifically, or even
> to insure it's actually in the output fragment, since it'd be too hard
> to reliably identify the problem. (For that matter, I can envision
> cases where the error is actually in a previously parsed hunk of XML,
> but doesn't become apparent until a tag mismatches in the current hunk
> or the like.) By way of example, the current error message:
>
I think your approach is good enough in this respect.
> A few comments:
>
> - The error message now extends over more than one line. I understand that svn
> generally avoids this, but I think it's necessary to supply a
> large enough fragment
> that the user can make a well-educated guess about the location of
> the problem.
> The presence of \ns in svn-generated XML would also make it tough
> to guarantee
> that any meaningful fragment stayed on one line. Finally, trying
> to flatten the XML to
> fit on a single logical line would go well past 80 cols, which I
> think would be more of a
> problem than multiple lines that fit into an 80 column display.
>
We generally try to avoid line breaks in error messages. That's because of
GUI issues. But with l10n and long filenames, we can't possibly keep
messages within 80 chars. IN this case, I think it is a good idea to keep
the newline befre the XML.
> - I chose a maximum fragment length of 240 chars
arbitrarily, as long > enough to give the
> reader some context and short enough to avoid a huge trail of text.
> It seemed like a
> reasonable balance to me.
>
Agree.
> - I placed delimiters before and after the XML with the thought that
> it might help in some
> situations where the XML itself started with odd characters; again,
> this is relatively
> arbitrary. It'd arguably be of some help for parsing the error
> (the delimiters aren't
> legal XML), but as this is a fatal and (one hopes) uncommon error,
> I don't think many
> people will be writing tools to anticipate it. If the consensus is
> that they just add noise,
> they're easy enough to drop.
>
IMO, just drop them.
> 'Nuff said about a minor patch for an infrequent occurrence. Chalk it
> up to it being my first foray onto this list.
>
Good start:-) A few commets below. (Also, could you please try to get the
MIME type of your attachments as some text mime type Makes it easier to
review.)
Display fragment of offending XML on "Malformed XML" error
Missing period at end of sentence and blank line after the summary.
* subversion/libsvn_subr/xml.c
(svn_xml_parse): Append reasonable hunk of XML to error message
We usually put the asterisk in the first column.
Index: subversion/libsvn_subr/xml.c
===================================================================
--- subversion/libsvn_subr/xml.c (revision 12919)
+++ subversion/libsvn_subr/xml.c (working copy)
@@ -417,9 +417,10 @@
{
err = svn_error_createf
(SVN_ERR_XML_MALFORMED, NULL,
- _("Malformed XML: %s at line %d"),
+ _("Malformed XML: %s at line %d; XML %s:\n>>>>>\n%.240s\n<<<<<\n"),
Drop the trailing newline.
XML_ErrorString (XML_GetErrorCode (svn_parser->parser)),
- XML_GetCurrentLineNumber (svn_parser->parser));
+ XML_GetCurrentLineNumber (svn_parser->parser),
+ len > 240 ? "starts" : "is", buf);
You can't use message fragment like this in internationalized messages.
Think of what happens when the _() marked string gets translated, but no
the "is" or "starts":-) You have tomove the condition outside the message,
creating two complete messages instead.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Feb 24 14:16:38 2005