Julian Foad <julian.f...@wandisco.com> writes:

> Noorul Islam K M wrote:
>
>> Julian Foad <julian.f...@wandisco.com> writes:
>> >   * "svn info ^/b" -> "Not a valid URL"; should be "path '/b' not found
>> > in revision REV"?
>> 
>> Patch attached.
>
> Hi Noorul.
>
>> Make "svn info" to display the actual error message returned by API when
>> illegal URL is passed.
>> 
>> * subversion/svn/info-cmd.c
>>   (svn_cl__info): Display the actual error message returned by
>>   svn_client_info3() instead of a custom one.
> [...]
>> Index: subversion/svn/info-cmd.c
>> ===================================================================
>> --- subversion/svn/info-cmd.c        (revision 1041293)
>> +++ subversion/svn/info-cmd.c        (working copy)
>> @@ -584,12 +584,8 @@
>>              }
>>            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)));
>> +                SVN_ERR(svn_cmdline_fprintf (stderr, subpool, 
>> +                                                 _("%s\n\n"), 
>> err->message));
>
> Unfortunately we cannot assume that err->message is a good user-friendly
> description of the problem.  It appears that it *is* in the specific
> case we're looking at:
>
> $ svn info ^/b
> URL 'https://svn.apache.org/repos/asf/b' non-existent in revision
> 1041775
>
> That's lovely.  But in other cases it's not:
>
> $ svn info hhh://aa.cc.dd/foo
> traced call
>
> See how err->message is just "traced call" in this case.  We can't even
> assume it is not NULL, in fact.
>
> I can think of two options.  We could try to use one of the
> svn_error_*() functions to print out a "nice" description of the actual
> returned error.  Alternatively, we could ignore 'err' and print a simple
> message here, like the existing code is doing but saying something more
> helpful than "Not a valid URL".  What do you think?
>
> Does the documentation for svn_client_info3() say under what conditions
> it returns the error code SVN_ERR_RA_ILLEGAL_URL?  Does that help?
>

Attached is the updated patch using svn_err_best_message() 

Log
[[[

Make "svn info" to display the actual error message returned by API when
illegal URL is passed.

* subversion/svn/info-cmd.c
  (svn_cl__info): Display the actual error message returned by
    svn_client_info3() instead of a custom one.

Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]

Thanks and Regards
Noorul

Index: subversion/svn/info-cmd.c
===================================================================
--- subversion/svn/info-cmd.c   (revision 1041944)
+++ subversion/svn/info-cmd.c   (working copy)
@@ -504,6 +504,7 @@
   svn_opt_revision_t peg_revision;
   svn_info_receiver_t receiver;
   const char *path_prefix;
+  char errbuf[256];
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -584,12 +585,9 @@
             }
           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)));
+              SVN_ERR(svn_cmdline_fprintf(
+                stderr, subpool, _("%s\n\n"), 
+                svn_err_best_message(err, errbuf, sizeof(errbuf))));
             }
           else
             {

Reply via email to