On Sat, 2010-12-25, Noorul Islam K M wrote:
> Julian Foad <julian.foad_at_wandisco.com> writes:
>
> > On Fri, 2010-12-24, C. Michael Pilato wrote:
> >
> >> On 12/23/2010 07:27 AM, Julian Foad wrote:
> >> >>From IRC:
> >> >
> >> > <Bert> julianf: We changed error codes in libsvn_wc all over the place.
> >> > I don't think we see strict error codes as part of the documented
> >> > behavior, unless it is in the function documentation
> >>
> >> I agree with this. Any time a specific error code is intended for use by an
> >> API function as a specific message to the caller, we document that fact and
> >> that behavior becomes part of the contract. Undocumented error codes are
> >> fair game for changing over time.
> >
> > OK, then that's fine by me too.
> >
> > Noorul wrote:
> >> Daniel Shahaf wrote:
> >> > Why do you have to place the "svn: " prefix here explicitly?
> >> Normally
> >> > svn_handle_error2() would do that for you. This is a red flag ("is
> >> > a wheel being reinvented here?") for me.
> >>
> >> We are actually consuming the error here to print it and proceed with
> >> the other targets.
> >
> > Noorul, "svn_handle_error2" has a "fatal" flag that controls whether it
> > terminates the program or not; setting fatal=FALSE would enable you to
> > continue processing the other targets. But it prints "the error stack"
> > which is not what you want here - we want just a single error message.
> >
> > Try using "svn_handle_warning2" instead. That function appears to do
> > almost exactly what you want here; the only differences I can see is it
> > inserts "warning: " before the message, which I think is perfect for
> > this usage, and it doesn't print the extra "\n", which is easily
> > rectified.
> >
>
> Please find updated patch attached. I used svn_handle_warning2 with
> "svn: " as prefix because it is used in this fashion everywhere. All
> tests pass with this patch.
Thanks, Noorul.
I see that this patch contains two logically separate changes:
1. Fix the regression that "svn info" has stopped skipping unversioned
paths, which is caused by WC-NG producing a new error code.
2. Display the specific error message from the API instead of a generic
"(Not a valid URL)" or "(Not a versioned resource)".
I split your patch into two, and committed part 1 in r1055484 and part 2
in r1055494.
> Index: subversion/svn/info-cmd.c
> ===================================================================
> --- subversion/svn/info-cmd.c (revision 1051915)
> +++ subversion/svn/info-cmd.c (working copy)
> @@ -566,29 +566,14 @@
> {
> /* If one of the targets is a non-existent URL or wc-entry,
> don't bail out. Just warn and move on to the next target. */
> - if (err->apr_err == SVN_ERR_UNVERSIONED_RESOURCE
> - || err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
> + if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND ||
> + err->apr_err == SVN_ERR_RA_ILLEGAL_URL)
> {
> - SVN_ERR(svn_cmdline_fprintf
> - (stderr, subpool,
> - _("%s: (Not a versioned resource)\n\n"),
> - svn_path_is_url(truepath)
> - ? truepath
> - : svn_dirent_local_style(truepath, pool)));
> + svn_handle_warning2(stderr, err, "svn: ");
> + svn_error_clear(svn_cmdline_fprintf(stderr, subpool, "\n"));
> }
> - else if (err->apr_err == SVN_ERR_RA_ILLEGAL_URL)
> - {
> - SVN_ERR(svn_cmdline_fprintf
> - (stderr, subpool,
> - _("%s: (Not a valid URL)\n\n"),
> - svn_path_is_url(truepath)
> - ? truepath
> - : svn_dirent_local_style(truepath, pool)));
> - }
> else
> - {
> return svn_error_return(err);
> - }
I kept those braces, for symmetry with the "if" clause.
> svn_error_clear(err);
> err = NULL;
- Julian
Received on 2011-01-05 16:57:21 CET