Right now, our error messages suck. They're sometimes non-intuitive
and unhelpful, but in addition to that, they're almost universally
ugly. In another thread, someone pointed out that they're ugly enough
to look like uncaught internal exceptions, which is no good.
I think there are a couple of causes for the ugliness problem:
(1) No guidelines for what message fields should look like.
(2) We always print the generic error text in addition to the
message field, for the highest-level error in the chain.
Also, we don't have any way of presenting long, detailed error
messages as may be appropriate for higher-level errors.
Unfortunately, because of (2), we may have grown into a state where
some message fields are too vague, because the programmer expected
that the generic error text would be displayed along with it. So
getting out of this state may involve some amount of transition pain.
I propose the following changes:
1. Make svn_handle_error() only print the generic error text if the
message field is NULL.
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); also allow the error message to
contain newlines for those rare cases when a message needs line
structure, such as when it provides an example command. All
output lines will begin with "svn: ", of course.
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.
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. 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.
I don't think any of these guidelines should prevent problems for
localization.
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
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.
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.
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.
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.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jun 21 20:28:24 2003