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

Re: [PATCH] Trace update, and cancel update editors patch

From: Greg Stein <gstein_at_lyra.org>
Date: 2001-11-30 09:14:09 CET

On Thu, Nov 29, 2001 at 04:33:33PM -0800, Bill Tutt wrote:
> Log:
> UI apps need trace editors. Seemed like a shame to force folks to
> constantly rewrite this useful bit of code. So, we now take postpone
> the actual printing to stdout in the cmdline client to a callback
> function. (for the trace-update editor only atm, trace-commit to
> follow shortly) The diff below shows trace-update still in the
> cmdline client directory. The intent of the patch, and the MSVC++
> .dsp patch shows the intent is to move the trace-update and
> trace-commit editors into libsvn_client. Additoinally, it's a shame
> that all editors constantly need to recalculate the current editor
> state. Why can't every editor just have access to the appropriate
> data? Of course, it'd be even handier if you could cache the data up
> front, that way you wouldn't be constantly recalculating the
> identical data with different editors using common code. But that
> isn't happening now. :)

The log should be about the change. Commentary can go elsewhere :-)

> Additionally, UI apps need asynchronous access to SVN, and therefore
> need a cancelation editor.

This patch is awfully big. Break it down into the distinct parts. One patch
for the trace editor. One to intro the cancellation. etc.

> When these patches are approved, I can commit the current state of
> the COM layer. That diff isn't included since I need to remove and
> readd all of the VB source files as bianry files until line-ending
> conversions are implemented.

Again, not relevant for a log msg.

> * libsvn_client/libsvn_client.dsp: Add cancel-update.c. Add trace-update.c.
> * clients/cmdline/subversion_client.dsp: Add mkdir-cmd.c Remove trace-update.c.

trace-update wasn't moved, so the file move in the .dsp shouldn't there.

> * libsvn_client/trace-update.c: (edit_baton): Rename baton to trace_edit_baton.
> MSVC's debugger didn't like it when there was more than one identically
> named struct (bad user expericence issues..) Add trace_ui vtable memeber
> and a baton for the callback.

Please make this a separate patch. It is hard to review with a rename *and*
functional changes in the same patch. If you do a rename *only*, then you
can check in the rename with a message describing the rename and saying "no
functional change." Further patches can do the functional change, and
they'll be much easier to review.


I'll review the bulk of the patch after it has been busted up into pieces.

One note: the cancel editor is horrifically repetitive. Write a single
function to take the cancel_baton and make the call to check for cancelling
and returning the appropriate error. Each of the editor functions would look
something like:

some_function(void *baton, ...)
  return check_cancel(baton);


static svn_error_t *check_cancel(cancel_baton *cb)
  if (cb->should_i_cancel(cb->inner_baton))
    return ...
  return SVN_NO_ERROR;

The editor functions are really simple. Not even a need for a local since
the void* simply auto-casts to a cancel_baton as it is passed to

> +/* Used by the trace_update editor in libsvn_client
> + */
> +void
> +svn_cl__print_message(void *ui_baton,
> + svn_stringbuf_t *msg)
> +{
> + fputs(msg->data, stdout);
> +}

1) that should be const svn_string_t *, or just const char *. No need for a
   darned stringbuf.

2) use fwrite if you have a string(buf), since you already have the length.


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:50 2006

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