Julian Foad <julian.f...@wandisco.com> writes:
> On Tue, 2010-11-09, Kamesh Jayachandran wrote:
>
>> On 11/08/2010 11:59 AM, Noorul Islam K M wrote:
>> > [[[
>> > Canonicalize paths before passing them to svn_client_list2.
>> >
>> > * subversion/svn/list-cmd.c
>> > (svn_cl__list): Canonicalize url or dirent before passing over to API.
>> >
>> > * subversion/tests/cmdline/basic_tests.py
>> > (ls_url_special_characters, test_list): New test
>> >
>> > Patch by: Noorul Islam K M<noorul{_AT_}collab.net>
>> > ]]]
>> >
>> Thanks Noorul. I tweaked for a cosmetic consistency and committed in
>> r1033045.
>
> Hi Noorul. These fixes are useful, but I can see a pattern emerging:
> probably every time the command-line client calls svn_opt_parse_path(),
> it is going to want the resulting "true path" to be canonicalized,
> wither as a URL or as a WC path.
>
>> Index: subversion/svn/list-cmd.c
>> ===================================================================
>>
>> SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
>> subpool));
>>
>> + if (svn_path_is_url(truepath))
>> + truepath = svn_uri_canonicalize(truepath, subpool);
>> + else
>> + truepath = svn_dirent_canonicalize(truepath, subpool);
>
> So I think it would be good if you could create a function that
> encapsulates the svn_opt_parse_path() and the new "if(url)..."
> statements. It could be called svn_cl__parse_path(), and it could live
> in subversion/svn/util.c.
>
> Then the svn client code could call it instead of svn_opt_parse_path().
>
I implemented the same and attached is the patch.
Log
[[[
New wrapper function svn_cl__opt_parse_path() for
svn_opt_parse_path(), which canonicalizes dirent/URL.
Make use of new function in "ls" and "cat" commands.
* subversion/svn/cl.h
(svn_cl__opt_parse_path): New function.
* subversion/svn/util.c
(svn_cl__opt_parse_path): New function.
* subversion/svn/list-cmd.c
(svn_cl__list),
* subversion/svn/cat-cmd.c
(svn_cl__cat): Use wrapper function svn_cl__opt_parse_path() instead
of svn_opt_parse_path()
Suggested by: Julian Foad <julian.foad{_AT_}wandisco.com>
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Thanks and Regards
Noorul
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1033355)
+++ subversion/svn/cl.h (working copy)
@@ -810,6 +810,13 @@
const apr_array_header_t *targets,
apr_pool_t *pool);
+/* Like svn_opt_parse_path(), but canonicalizes dirent/URL */
+svn_error_t *
+svn_cl__opt_parse_path(svn_opt_revision_t *rev,
+ const char **truepath,
+ const char *path,
+ apr_pool_t *pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: subversion/svn/cat-cmd.c
===================================================================
--- subversion/svn/cat-cmd.c (revision 1033355)
+++ subversion/svn/cat-cmd.c (working copy)
@@ -27,7 +27,6 @@
/*** Includes. ***/
-#include "svn_path.h"
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_error.h"
@@ -69,14 +68,9 @@
SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
/* Get peg revisions. */
- SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
- subpool));
+ SVN_ERR(svn_cl__opt_parse_path(&peg_revision, &truepath, target,
+ subpool));
- if (svn_path_is_url(truepath))
- truepath = svn_uri_canonicalize(truepath, subpool);
- else
- truepath = svn_dirent_canonicalize(truepath, subpool);
-
SVN_ERR(svn_cl__try(svn_client_cat2(out, truepath, &peg_revision,
&(opt_state->start_revision),
ctx, subpool),
Index: subversion/svn/list-cmd.c
===================================================================
--- subversion/svn/list-cmd.c (revision 1033355)
+++ subversion/svn/list-cmd.c (working copy)
@@ -268,14 +268,9 @@
SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
/* Get peg revisions. */
- SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
- subpool));
+ SVN_ERR(svn_cl__opt_parse_path(&peg_revision, &truepath, target,
+ subpool));
- if (svn_path_is_url(truepath))
- truepath = svn_uri_canonicalize(truepath, subpool);
- else
- truepath = svn_dirent_canonicalize(truepath, subpool);
-
if (opt_state->xml)
{
svn_stringbuf_t *sb = svn_stringbuf_create("", pool);
Index: subversion/svn/util.c
===================================================================
--- subversion/svn/util.c (revision 1033355)
+++ subversion/svn/util.c (working copy)
@@ -1324,3 +1324,19 @@
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_cl__opt_parse_path(svn_opt_revision_t *rev,
+ const char **truepath,
+ const char *path /* UTF-8! */,
+ apr_pool_t *pool)
+{
+ SVN_ERR(svn_opt_parse_path(rev, truepath, path, pool));
+
+ if (svn_path_is_url(*truepath))
+ *truepath = svn_uri_canonicalize(*truepath, pool);
+ else
+ *truepath = svn_dirent_canonicalize(*truepath, pool);
+
+ return SVN_NO_ERROR;
+}
