You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2004/12/01 18:48:37 UTC

Re: svn commit: r12100 - trunk/subversion/libsvn_client

breser@tigris.org writes:
> --- trunk/subversion/libsvn_client/diff.c	(original)
> +++ trunk/subversion/libsvn_client/diff.c	Tue Nov 30 22:07:43 2004
> @@ -1198,6 +1196,47 @@
>    return SVN_NO_ERROR;
>  }
>  
> +/* Struct used for as the baton for calling merge_delete_notify_func(). */
> +typedef struct merge_delete_notify_baton_t
> +{
> +  svn_client_ctx_t *ctx;
> +
> +  /** path to skip */
> +  const char *path_skip;
> +} merge_delete_notify_baton_t;
> +
> +/* Notify callback function that wraps the normal callback
> + * function to remove a notification that will be sent twice
> + * and set the proper action. */
> +static void
> +merge_delete_notify_func (void *baton,
> +                          const char *path,
> +                          svn_wc_notify_action_t action,
> +                          svn_node_kind_t kind,
> +                          const char *mime_type,
> +                          svn_wc_notify_state_t content_state,
> +                          svn_wc_notify_state_t prop_state,
> +                          svn_revnum_t revision)
> +{
> +  merge_delete_notify_baton_t *mdb = (merge_delete_notify_baton_t *) baton;
> +
> +  /* Skip the notification for the path we called svn_client__wc_delete() with,
> +   * because it will be outputed by repos_diff.c:delete_item */  
> +  if (!strcmp(path,mdb->path_skip))
> +    return;

Style comment: we generally -- though not universally, I admit -- have
been using "strcmp (a, b) != 0", as more intuitive to read.  For some
people, anyway.  Also, note the space-before-paren-thang.

> +  /* svn_client__wc_delete() is written primarily for scheduling operations not
> +   * update operations.  Since merges are update operations we need to alter
> +   * the delete notification to show as an update not a schedule so alter 
> +   * the action. */
> +  if (action == svn_wc_notify_delete)
> +    action = svn_wc_notify_update_delete;
> +
> +  if (mdb->ctx->notify_func)
> +    (*mdb->ctx->notify_func) (mdb->ctx->notify_baton, path, action, kind,
> +                              mime_type, content_state, prop_state, revision);
> +}

Very clear comment, nice.

>  /* A svn_wc_diff_callbacks2_t function. */
>  static svn_error_t *
>  merge_dir_deleted (svn_wc_adm_access_t *adm_access,
> @@ -1227,20 +1266,29 @@
>    switch (kind)
>      {
>      case svn_node_dir:
> -      svn_path_split (path, &parent_path, NULL, merge_b->pool);
> -      SVN_ERR (svn_wc_adm_retrieve (&parent_access, adm_access, parent_path,
> -                                    merge_b->pool));
> -      err = svn_client__wc_delete (path, parent_access, merge_b->force,
> -                                   merge_b->dry_run, merge_b->ctx, subpool);
> -      if (err && state)
> -        {
> -          *state = svn_wc_notify_state_obstructed;
> -          svn_error_clear (err);
> -        }
> -      else if (state)
> -        {
> -          *state = svn_wc_notify_state_changed;
> -        }
> +      {
> +        merge_delete_notify_baton_t mdb;
> +
> +        mdb.ctx = merge_b->ctx;
> +        mdb.path_skip = path;
> +
> +        svn_path_split (path, &parent_path, NULL, merge_b->pool);
> +        SVN_ERR (svn_wc_adm_retrieve (&parent_access, adm_access, parent_path,
> +                                      merge_b->pool));
> +        err = svn_client__wc_delete (path, parent_access, merge_b->force,
> +                                     merge_b->dry_run,
> +                                     merge_delete_notify_func, (void *) &mdb,
> +                                     merge_b->ctx, subpool);

Is that "(void *)" cast really needed?

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12100 - trunk/subversion/libsvn_client

Posted by kf...@collab.net.
kfogel@collab.net writes:
> > +   * because it will be outputed by repos_diff.c:delete_item */  
> > +  if (!strcmp(path,mdb->path_skip))
> > +    return;
> 
> Style comment: we generally -- though not universally, I admit -- have
> been using "strcmp (a, b) != 0", as more intuitive to read.  For some
> people, anyway.  Also, note the space-before-paren-thang.

Oops!  I wrote "!=" when I meant "==" in my review, sorry about that
Ben.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org