On Wed, Apr 03, 2002 at 02:49:50PM -0600, sussman@tigris.org wrote:
>...
> Stop using the trace-commit editor in a half-XXXed way. Instead, send
> detailed feedback to the application that *looks* like the old
> trace-commit editor output. This should cause our python tests to
> pass again with the new commit system.
"should" or "do" ? :-)
I presume this was coordinated with Mike?
btw, I *really* like this much better than the trace editor. It is
definitely much more straight-forward, and (IMO) will be easier for clients
to use.
>...
> +++ trunk/subversion/include/svn_wc.h Wed Apr 3 14:49:50 2002
>...
> @@ -73,8 +73,12 @@
> svn_wc_notify_restore,
> svn_wc_notify_revert,
> svn_wc_notify_resolve,
> - svn_wc_notify_update
> -
> + svn_wc_notify_update,
> + svn_wc_notify_commit_modified,
> + svn_wc_notify_commit_added,
> + svn_wc_notify_commit_deleted,
> + svn_wc_notify_commit_replaced,
> + svn_wc_notify_commit_postfix_txdelta,
> } svn_wc_notify_action_t;
The last enumerated constant in a list cannot have a trailing comma. This
keeps coming up...
[ there are compilers which don't like it, even tho GCC seems fine ]
>...
> +++ trunk/subversion/include/svn_client.h Wed Apr 3 14:49:50 2002
> @@ -463,6 +463,10 @@
> AFTER_EDIT_BATON are pre- and post-commit hook editors. They are
> optional; pass four NULLs here if you don't need them.
>
> + Additionally, NOTIFY_FUNC/BATON will be called as the commit
> + progresses, as a way of describing actions to the application
> + layer.
Is it reasonable now to remove the before/after editor concept [from across
the interface] ? i.e. stop composing editors and shift to a pure
notification system?
>...
> +++ trunk/subversion/libsvn_client/commit_util.c Wed Apr 3 14:49:50 2002
>...
> @@ -595,6 +598,28 @@
> /* Get the parent dir_baton. */
> parent_baton = ((void **) db_stack->elts)[*stack_ptr - 1];
>
> + /* If a feedback table was supplied by the application layer,
> + describe what we're about to do to this item. */
> + if (notify_func)
> + {
> + /* Convert an absolute path into a relative one (for feedback.) */
> + const char *path = item->path->data + (display_dir->len + 1);
> +
> + if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)
> + && (item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD))
> + (*notify_func) (notify_baton, svn_wc_notify_commit_replaced, path);
> +
> + else if (item->state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)
> + (*notify_func) (notify_baton, svn_wc_notify_commit_deleted, path);
> +
> + else if (item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)
> + (*notify_func) (notify_baton, svn_wc_notify_commit_added, path);
> +
> + else if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_TEXT_MODS)
> + || (item->state_flags & SVN_CLIENT_COMMIT_ITEM_PROP_MODS))
> + (*notify_func) (notify_baton, svn_wc_notify_commit_modified, path);
This sequence of else/if makes it look like the "state_flags" should
actually be an enumerated constant, rather than bit fields.
>...
> +++ trunk/subversion/clients/cmdline/feedback.c Wed Apr 3 14:49:50 2002
> @@ -32,11 +32,19 @@
> #include "cl.h"
>
>
> +struct notify_baton
> +{
> + apr_pool_t *pool;
> + svn_boolean_t sent_first_txdelta;
> +};
> +
> +
> static void
> notify_added (void *baton, const char *path)
> {
> /* the pool (BATON) is typically the global pool; don't keep filling it */
> - apr_pool_t *subpool = svn_pool_create (baton);
> + struct notify_baton *nb = (struct notify_baton *) baton;
> + apr_pool_t *subpool = svn_pool_create (nb->pool);
The comment doesn't match the code any more.
[ should I reiterate my position on comments? :-) ]
>...
> +static void
> +notify_commit_postfix_txdelta (void *baton,
> + const char *path)
> +{
> + struct notify_baton *nb = (struct notify_baton *) baton;
Note that the cast here and above aren't needed. Casting from 'void *' to
'struct notify_baton *' is automatically done by the compiler.
> + if (nb->sent_first_txdelta == FALSE)
You compare against FALSE, but initialize this field to 0. I'd recommend
just using the ! operator.
>...
> @@ -121,7 +166,12 @@
>
> void *svn_cl__make_notify_baton (apr_pool_t *pool)
> {
> - return (void *)pool;
> + struct notify_baton *nb = apr_pcalloc (pool, sizeof(*nb));
> +
> + nb->pool = pool;
> + nb->sent_first_txdelta = 0;
If you're using pcalloc(), there isn't a need to set to zero. Alternatively,
keep the assignment, but use palloc().
> +
> + return (void *)nb;
The cast isn't needed.
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 Wed Apr 3 23:37:47 2002