[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: Stefan Sperling <stsp_at_elego.de>
Date: Mon, 12 Nov 2012 22:32:44 +0100

On Sat, Nov 10, 2012 at 07:11:02PM +0530, vijay wrote:
> I have removed the two boolean parameters notify_external_start and
> notify_external_end from svn_client_list_func2 implementation. Now,
> the callback keeps track of last seen external information and print
> details based on the change. I have updated the docstring of
> svn_client_list_func2_t also.

Thanks!

> 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.

> +
> + /* 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.
Received on 2012-11-12 22:33:35 CET

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