In non-maintainer mode errors should be targeted at *users*.
The more cryptic error codes and all caps strings we put in errors the less readable / understandable the messages become. And the more likely users are to contact support or to call the product not user friendly.
These errors should be readable/understandable without numbers/error strings. One specific number might help in Google, but a list of errors codes works against us here.
(Well, maybe we want more traffic on users@ and commercial support to understand errors? And more users switching to Git because Subversion becomes so hard to use for non developers...)
Maintainer mode is a different story. For diagnosing errors from the buildbot/test suite the error codes are very useful and the full names make sense. But we also need apr/serf/os errors mapped for them to be more useful than just the numbers.
In the bindings, mapping the errors to an enum is probably better than a mapping to strings, as bindings are likely to use similar ignore specific error codes just like we do in our code.
And bindings can’t parse the error texts as these can/should be localized.
Bert
Sent from Windows Mail
From: Daniel Shahaf
Sent: Saturday, April 13, 2013 2:23 AM
To: dev_at_subversion.apache.org
Cc: C. Michael Pilato; Julian Foad
Stefan Sperling wrote on Sat, Apr 13, 2013 at 01:55:01 +0200:
> On Fri, Apr 12, 2013 at 02:34:26PM -0400, C. Michael Pilato wrote:
> > svn: E180003: Unable to connect to a repository at URL
> > 'file:///home/cmpilato/tests/arch'
> > svn: E180012: Unable to open an ra_local session to URL
> > svn: E180001: Unable to open repository 'file:///home/cmpilato/tests/arch'
> > (E1800001 = SVN_ERR_RA_LOCAL_REPOS_OPEN_FAILED)
> > (E1800003 = SVN_ERR_SOMETHING_ELSE)
> > (E1800012 = SVN_ERR_YET_ANOTHER_FAILURE)
> >
> > Thoughts?
>
> Of the suggestions given in this thread so far, I like this suggestion
> best (and I've read the entire thread).
>
> Today on IRC I said that I'd rather see symbolic names instead
> of error numbers. I still think that might be nice. The current
> proposals in this thread seem to focus on displaying *both* numbers
> and their symbolic names and I think that's too much verbosity.
Here's a quick shot at that.
% $svn diff -x-p
Index: subversion/libsvn_subr/error.c
===================================================================
--- subversion/libsvn_subr/error.c (revision 1467481)
+++ subversion/libsvn_subr/error.c (working copy)
@@ -562,8 +562,7 @@ svn_handle_error2(svn_error_t *err,
apr_pool_create(&subpool, err->pool);
empties = apr_array_make(subpool, 0, sizeof(apr_status_t));
- tmp_err = err;
- while (tmp_err)
+ for (tmp_err = err; tmp_err; tmp_err = tmp_err->child)
{
svn_boolean_t printed_already = FALSE;
@@ -589,9 +588,21 @@ svn_handle_error2(svn_error_t *err,
APR_ARRAY_PUSH(empties, apr_status_t) = tmp_err->apr_err;
}
}
+ }
- tmp_err = tmp_err->child;
- }
+ for (tmp_err = err; tmp_err; tmp_err = tmp_err->child)
+ if (! (tmp_err->child && tmp_err->apr_err == tmp_err->child->apr_err))
+ {
+ const char *symbolic_name = svn_error_symbolic_name(tmp_err->apr_err);
+ if (symbolic_name && !strncmp(symbolic_name, "SVN_ERR_", 8))
+ symbolic_name += 8;
+ else if (! symbolic_name)
+ symbolic_name = "N/A";
+ svn_error_clear(svn_cmdline_fprintf(stream, err->pool,
+ "(E%06d = %s)\n",
+ tmp_err->apr_err,
+ symbolic_name));
+ }
svn_pool_destroy(subpool);
% $svnadmin create /
svnadmin: E200011: Repository creation failed
svnadmin: E200011: Could not create top-level directory
svnadmin: E200011: '/' exists and is non-empty
(E200011 = DIR_NOT_EMPTY)
%
Daniel
Received on 2013-04-13 11:04:56 CEST