Greg Hudson wrote:
>I propose the following changes:
>
> 1. Make svn_handle_error() only print the generic error text if the
> message field is NULL.
>
I wonder if that's overkill? We already hide the generic message for
wrapped errors that have the same error code.
> 2. Make svn_handle_error() word-wrap the error field to 70 columns
> (or to the display width minus some margin, but that might be too
> irritating to handle portably);
>
Yes.
> also allow the error message to
> contain newlines for those rare cases when a message needs line
> structure,
>
Plese no. Think of what happens when a GUI tries to display this message.
> such as when it provides an example command. All
> output lines will begin with "svn: ", of course.
>
This can easily be achieved by wrapping svn_error_t's. Since the generic
error code is already hidden in this case, you'll get exactly what you want.
> 3. Document guidelines for error message text. Something like:
>
> The error message, if present, should give specific information
> about what went wrong. If you have no specific information to
> add to the error code, pass NULL as the message; svn_strerror()
> on the code will provide a generic error message. (If you do
> pass in a specific error message, provide a complete
> description of the error; do not assume that the user will see
> the generic message.) High-level functions such as those in
> libsvn_client should attempt to provide simple remediation
> instructions for the error if possible.
>
+1
> Error messages should be capitalized, but should not contain a
> sentence-ending period unless there are multiple sentences in
> the message. Messages should either be a sentence ("Working
> directory is out of date") or a clear noun phrase ("Unknown
> command"). Don't worry about making messages too long; it is
> the display function's responsibility to wrap them as
> necessary.
>
+1
> Messages may include internal newlines, but should
> only use them if the line structure is meaningful, such as when
> the message contains an example command to run. Under no
> circumstances should a message contain a final newline.
>
-1, see above
> I don't think any of these guidelines should prevent problems for
> localization.
>
They might; sometimes the order of sentences differs from language to
language. So the guideline should be that when you use wrapped messages
(i.e., where line structure is important), these should only contain
examples (complete commands, or single URLs/paths); and they should
always be the last thing to be printed.
> The guidelines may also want to say something about what you
> should think about when you're writing an error message for an
> error which is going to be wrapped around a lower-level error.
> Should the programmer be able to expect that the order of error
> messages in this case? Are we displaying them in the right order
> currently? (Right now we start at the highest level and
>
And what? :-)
> 4. Slowly change error messages to meet the guidelines in (3). I
> think it would be too much work to do a complete pass at the same
> time as (1)-(3), although it would be good to search the source
> tree for very short error messages in an effort to find the cases
> where code is relying on the generic text being displayed.
>
Yes. BTW, we already have an issue about revamping error messages.
> 5. Some peripherally related technical issues:
>
> Document that svn_error_create() can accept a NULL message.
> Either:
> - Document that apr_pstrdup() can accept a NULL string, or
> - Stop unconditionally apr_pstrdup()ing the message argument in
> error.c.
>
> There's also something screwy about the interaction between
> svn_strerror() and handle_error(). It looks like handle_error()
> is assuming that we should convert Subversion error messages from
> UTF8 to native but not APR error messages, which leads to the
> terrifying conclusion that svn_strerror() is returning UTF8 text
> for Subversion error codes and native text for APR error codes.
> I'm not sure how to sort this out.
>
Short of getting some I18N rules into APR, I don't believe it's
possible. It would be enough to declare (for now) that apr_strerror
returns ASCII-only strings, in which case they're automagically UFF-8,
too. No, I have no idea what happens in an EBCDIC environment then.
> Finally, there's no need for handle_error() to be recursive; in C
> code, it's usually clearer to iterate over a linked list with a
> for loop.
>
Just goes to show that it was written by people who'd at one time
contemplated writing SVN in lisp. :-)
>Comments? I note that most of these changes could be made by someone
>without an intimate familiarity with the Subversion inernals (i.e. not
>me). (1)-(3) and (5) together would make a good bite-sized task.
>
>
I agree. This could be added to issue 897, along with the guidelines
that are already there.
--
Brane Čibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jun 22 12:57:34 2003