You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Noorul Islam K M <no...@collab.net> on 2010/11/08 06:29:11 UTC
Fix for issue 3609 - ls command
Followup to r1030010
Log
[[[
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>
]]]
Re: Fix for issue 3609 - ls command
Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2010-11-10, Noorul Islam K M wrote:
> Julian Foad <ju...@wandisco.com> writes:
[...]
> > 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.
Thanks! Committed revision 1033411. (I tweaked the log message wording
a little bit to my taste.)
- Julian
> 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>
>
> ]]]
Re: Fix for issue 3609 - ls command
Posted by Noorul Islam K M <no...@collab.net>.
Julian Foad <ju...@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
Re: Fix for issue 3609 - ls command
Posted by Noorul Islam K M <no...@collab.net>.
Julian Foad <ju...@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
Re: Fix for issue 3609 - ls command
Posted by Julian Foad <ju...@wandisco.com>.
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().
- Julian
Re: Fix for issue 3609 - ls command
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 11/08/2010 11:59 AM, Noorul Islam K M wrote:
> Followup to r1030010
>
> Log
>
> [[[
>
> 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.
With regards
Kamesh Jayachandran