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

Re: [PATCH REPOST] Trace and canelation editor changes part 4

From: Greg Stein <gstein_at_lyra.org>
Date: 2002-01-25 03:28:57 CET

On Mon, Jan 21, 2002 at 02:53:10AM -0800, Bill Tutt wrote:
>...
> +/* Returns an editor that returns SVN_ERR_USER_CANCELED if should_i_cancel
> + * ever returns true. Should be composed before any editor that does
> + * any work at all from a UI of SVN. */
> +svn_error_t *
> +svn_client_get_cancellation_editor(const svn_delta_edit_fns_t **editor,
> + void **edit_baton,
> + svn_boolean_t (*should_i_cancel)(void *),
> + void *inner_baton,
> + apr_pool_t *pool);

How about a typedef for that function type? It would allow you to reuse it
elsewhere.

>...
> +static svn_error_t *
> +should_i_cancel(struct cancel_baton *cb)
> +{
> + if (cb->should_i_cancel(cb->inner_baton))
> + {
> + return svn_error_create (SVN_ERR_USER_CANCELED,
> + 0,
> + 0,
> + cb->pool,
> + "User canceled operation.");

The third param is clearer as NULL, since that param is a ptr.

>...
> +static svn_error_t *
> +open_root (void *cancel_baton, svn_revnum_t base_revision, void **root_baton)

Lines shouldn't go past column 80.
The first param is the edit baton. It would be clearer as edit_baton.

> +{
> + struct cancel_baton *cb = cancel_baton;

Especially since you clarify its real meaning right here.

>...
> +static svn_error_t *
> +add_directory (svn_stringbuf_t *name,
> + void *parent_baton,
> + svn_stringbuf_t *copyfrom_path,
> + svn_revnum_t copyfrom_revision,
> + void **child_baton)
> +{
> + svn_error_t *err = SVN_NO_ERROR;
> +
> + err = should_i_cancel(parent_baton);
> + if (err)
> + {
> + return err;
> + }

SVN_ERR() is very handy here. Same in a number of other places in the file.

>...
> +static svn_error_t *
> +close_directory (void *dir_baton)
> +{
> + return should_i_cancel(dir_baton);
> +}
> +
> +
> +static svn_error_t *
> +close_file (void *file_baton)
> +{
> + return should_i_cancel(file_baton);
> +}
> +
> +
> +static svn_error_t *
> +close_edit (void *edit_baton)
> +{
> + return should_i_cancel(edit_baton);
> +}

The above three functions (and maybe some others?) all have the same
signature and body. May as well collapse them.

With the changes above, I'm +1 on applying this.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:59 2006

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.