You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/03/02 11:37:39 UTC

Re: svn commit: r36241 - trunk/subversion/libsvn_wc

On Mon, Mar 2, 2009 at 12:11, Bert Huijben <rh...@sharpsvn.net> wrote:
>...
> +++ trunk/subversion/libsvn_wc/status.c Mon Mar  2 03:11:09 2009        (r36241)
> @@ -281,7 +281,7 @@ assemble_status(svn_wc_status2_t **statu
>       if (entry && entry->url)
>         abs_path = entry->url + strlen(repos_root);
>       else if (parent_entry && parent_entry->url)
> -        abs_path = svn_path_join(parent_entry->url + strlen(repos_root),
> +        abs_path = svn_uri_join(parent_entry->url + strlen(repos_root),
>                                 svn_dirent_basename(path, pool), pool);

Maybe use svn_path_url_add_component2() ? ... that basename isn't encoded.

Which leads to a question... maybe our uri_is_canonical() should try
to ensure that the URI is encoded properly?

And another meta issue: URI is a poor name. The functions only deal with URLs.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1255935


RE: svn commit: r36241 - trunk/subversion/libsvn_wc

Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: maandag 2 maart 2009 12:38
> To: dev@subversion.tigris.org
> Subject: Re: svn commit: r36241 - trunk/subversion/libsvn_wc
> 
> On Mon, Mar 2, 2009 at 12:11, Bert Huijben <rh...@sharpsvn.net> wrote:
> >...
> > +++ trunk/subversion/libsvn_wc/status.c Mon Mar  2 03:11:09 2009
>  (r36241)
> > @@ -281,7 +281,7 @@ assemble_status(svn_wc_status2_t **statu
> >       if (entry && entry->url)
> >         abs_path = entry->url + strlen(repos_root);
> >       else if (parent_entry && parent_entry->url)
> > -        abs_path = svn_path_join(parent_entry->url + strlen(repos_root),
> > +        abs_path = svn_uri_join(parent_entry->url + strlen(repos_root),
> >                                 svn_dirent_basename(path, pool), pool);
> 
> Maybe use svn_path_url_add_component2() ? ... that basename isn't encoded.
> 
> Which leads to a question... maybe our uri_is_canonical() should try
> to ensure that the URI is encoded properly?
> 
> And another meta issue: URI is a poor name. The functions only deal with URLs.

Agree and agree.

We have three kinds of paths which until recently all used the svn_path_*() functions.

* local paths (dirents)
* repository paths (not encoded)
* repository urls (should be encoded following the uri/url rules).


The original design of svn_dirent_uri.h replaces local paths by dirents and tries to implement the other two with svn_uri_*. (Leaving the escaping problems to the users of the api).

We might need a third set of functions that handles the escaping/encoding rules automatically.


I think we can pull the rest of svn_uri_* from 1.6 if we need to revisit this design.

	Bert

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1255965