[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: [PATCH] Implement '--include-externals' option to 'svn list'

From: Bert Huijben <bert_at_qqmail.nl>
Date: Thu, 15 Nov 2012 09:47:25 +0100

> -----Original Message-----
> From: vijay [mailto:vijay_at_collab.net]
> Sent: donderdag 15 november 2012 08:53
> To: Bert Huijben
> Cc: 'Subversion Development'
> Subject: Re: [PATCH] Implement '--include-externals' option to 'svn list'
>
> On Tuesday 13 November 2012 06:45 PM, Bert Huijben wrote:
> >
> >
> >> -----Original Message-----
> >> From: vijay [mailto:vijay_at_collab.net]
> >> Sent: dinsdag 13 november 2012 14:06
> >> To: Subversion Development
> >> Subject: Re: [PATCH] Implement '--include-externals' option to 'svn
list'
> >>
> >> On Tuesday 13 November 2012 03:02 AM, Stefan Sperling wrote:
> >>>> Attached the updated patch and log message.
> >>>
> >>>> + /* Notify that we're about to handle an external. */
> >>>> + SVN_ERR(list_func(baton, NULL, NULL, NULL, NULL,
> >>>> + externals_parent_url,
> >>>> + item->target_dir, iterpool));
> >>>
> >>> The docstring of svn_client_list_func2_t doesn't say if it is valid
> >>> for path or dirent to be NULL.
> >>>
> >>> However, you're passing NULL for these parameters before listing the
> >>> external. Do we really need these two extra list_func() calls before
> >>> and after listing the external? I was expecting them to go away.
> >>
> >>
> >> If we are removing these list_func() calls, how can we pass the
> >> arguments external_parent_url and external_target to the callback
> >> function? Is there any way to pass those arguments via
svn_client_list3()?
> >
> > Can't you just add the new argument to every call to list_func() that
> > applies to an external?
>
>
> There is no separate list_func() call to list external items. We are
> just calling svn_client_list3() recursively for each external, which in
> turn calls list_func() using get_dir_contents(). I am struggling a bit
> hard to carry forward external information via svn_client_list3(). Can
> we add two more arguments external_parent_url and external_target to
> svn_client_list3()?

You could just create a static function with those extra arguments and make
the outer svn_client_list3() call into that function.
Please try to avoid adding implementation details to the public api, unless
absolutely necessary.
(We have such wrappers in almost every case where we use externals)

This model would also allow future improvements like re-using the
ra-session, without updating the public function prototype.

        Bert
Received on 2012-11-15 09:48:14 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.