On Sat, 2010-12-25, Noorul Islam K M wrote: > Julian Foad <julian.f...@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