[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: vijay <vijay_at_collab.net>
Date: Tue, 13 Nov 2012 18:36:07 +0530

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()?

>
>> +
>> + /* List the external */
>> + SVN_ERR(wrap_external_error(ctx, item->target_dir,
>> + svn_client_list3(resolved_url,
>> + &item->peg_revision,
>> + &item->revision,
>> + depth, dirent_fields,
>> + fetch_locks,
>> + TRUE,
>> + list_func, baton, ctx,
>> + iterpool),
>> + iterpool));
>> +
>> + /* Notify that we are done with external handling. It is helpful
>> + when list is run in xml mode. */
>> + SVN_ERR(list_func(baton, NULL, NULL, NULL, NULL,
>> + externals_parent_url,
>> + item->target_dir, iterpool));
>
>
>> -/* This implements the svn_client_list_func_t API, printing a single dirent
>> +/* This implements the svn_client_list_func2_t API, printing a single dirent
>> in XML format. */
>> static svn_error_t *
>> print_dirent_xml(void *baton,
>> @@ -141,16 +174,46 @@
>> const svn_dirent_t *dirent,
>> const svn_lock_t *lock,
>> const char *abs_path,
>> - apr_pool_t *pool)
>> + const char *external_parent_url,
>> + const char *external_target,
>> + apr_pool_t *scratch_pool)
>> {
>
> Renaming 'pool' to 'scratch_pool' here is correct, but it causes
> too many unrelated changes in this patch for my taste.
> I'd prefer to apply this change in a separate patch.
>

I will send a separate patch for this change.

Thanks & Regards,
Vijayaguru
Received on 2012-11-13 14:06:59 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.