[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] svn info - changing hard coded error message to specific one.

From: Noorul Islam K M <noorul_at_collab.net>
Date: Sat, 25 Dec 2010 11:37:31 +0530

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 and Regards
Noorul

Index: subversion/tests/cmdline/basic_tests.py
===================================================================
--- subversion/tests/cmdline/basic_tests.py (revision 1051915)
+++ subversion/tests/cmdline/basic_tests.py (working copy)
@@ -2260,7 +2260,8 @@
 
   # Check for the correct error message
   for line in errput:
- if re.match(".*\(Not a valid URL\).*", line):
+ if re.match(".*" + idonotexist_url + ".*non-existent in revision 1.*",
+ line):
       return
 
   # Else never matched the expected error output, so the test failed.
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);
- }
 
           svn_error_clear(err);
           err = NULL;
Received on 2010-12-25 07:08:38 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.