On Wed, Nov 7, 2012 at 3:02 PM, vijay <vi...@collab.net> wrote: > On Tuesday 06 November 2012 08:09 PM, Stefan Fuhrmann wrote: > >> On Mon, Nov 5, 2012 at 5:24 PM, vijay <vi...@collab.net >> <mailto:vi...@collab.net>> wrote: >> >> Hi, >> >> This patch implements '--include-externals' option to 'svn list' >> [Issue #4225] [1]. >> >> All tests pass with 'make check' & 'make davautocheck'. >> >> Attached the patch and log message. >> >> Please review this patch and share your thoughts. >> >> Thanks in advance for your time. >> >> Thanks & Regards, >> Vijayaguru >> >> [1] >> http://subversion.tigris.org/_**_issues/show_bug.cgi?id=4225<http://subversion.tigris.org/__issues/show_bug.cgi?id=4225> >> >> >> <http://subversion.tigris.org/**issues/show_bug.cgi?id=4225<http://subversion.tigris.org/issues/show_bug.cgi?id=4225> >> > >> >> >> Hi Vijay, >> >> Not sure whether these points have already been >> discussed in previous threads, but the following >> two points caught my attention: >> >> +typedef svn_error_t *(*svn_client_list_func2_t)( >> + void *baton, >> + const char *path, >> + const svn_dirent_t *dirent, >> + const svn_lock_t *lock, >> + const char *abs_path, >> >> o.k. >> >> + svn_boolean_t notify_external_start, >> + svn_boolean_t notify_external_end, >> + const char *external_parent_url, >> + const char *external_target, >> >> Maybe, this should be modeled to create a more "seamless" >> appearance. Only keep the external_parent_url member. >> If it is NULL, this entry has not been pulled in here by >> some external. Otherwise it contains the parent URL as >> defined by your patch. >> >> I don't see the see the need to expose more information. >> Why would you need to group externals? The external_target >> member should be given implicitly by path / dirent. >> Am I missing something here? >> > > > The external_target member will not be given by path / dirent. > We will get it by parsing the externals description. > > Suppose that path1 in repo1 has svn:externals set to path2 in repo2. > > When we list path1 with externals included, > > 1. First, list path1. > 2. Then, process all the externals defined under path1. Parse through the > individual external description and populate external_target. > > For example, > > external description under path1: > external_desc = exdir http://<url-of-path2-in-repo2> > > external_target = exdir > external_parent_url = http://<url-of-path1-in-repo1> > > url of external = http://<url-of-path2-in-repo2> > > 3. List the entries by reaching 'url of external'.
OK, I now see what you are trying to do here - I had read to much into the code. However, in this form, the added functionality is of limited use, IMHO. I understand that what you list is basically which paths you would get for a "svn co $url". While this is fine, I see two issues with it: * trees don't get overlayed. Example: $>svn ls $URL -R sub1 sub1/fileA sub1/fileB sub2 $>svn propget "svn:externals" $URL http://somewhere/ sub1/subsub $>svn ls $URL -R --include-externals sub1 sub1/fileA sub1/fileB sub2 sub1/subsub [external]. Desired output sub1 sub1/fileA sub1/fileB sub1/subsub [external @$URL]. sub2 * result of ls on a sub-folder results in less output $>svn ls $URL/sub1 -R --include-externals fileA fileB Desired output: fileA fileB subsub [external @$URL]. So, it is hard to build e.g. explorer-like GUIs based on this information. The GUI would still need to collect, remember and "splice" the externals manually. That would render your extension almost useless. -- Stefan^2. -- Certified & Supported Apache Subversion Downloads: * http://www.wandisco.com/subversion/download *
