On Sep 15, 2009, at 7:26 AM, Daniel Näslund wrote:
> Hyurum, thanks for the quick feedback. One question below.
>
>>> Index: subversion/include/svn_wc.h
>>> ===================================================================
>>> --- subversion/include/svn_wc.h (revision 39314)
>>> +++ subversion/include/svn_wc.h (arbetskopia)
>>> @@ -1906,7 +1906,7 @@
>>> * property name.
>>> *
>>> */
>>> - svn_error_t *(*file_changed)(svn_wc_adm_access_t *adm_access,
>>> + svn_error_t *(*file_changed)(svn_wc_context_t *wc_ctx,
>>
>> I wonder if we need to include the wc_ctx argument here at all, and
>> instead just let callers squirrel away a wc_ctx in their baton if
>> they
>> are going to need it in the callback. (This would also mean you
>> wouldn't need to construct the temporary context below.)
>
> It sounds like a good idea to stuff it away in the baton. Saves me a
> lot
> of work when the callback functions call each other. But the wc_ctx
> would only be visible to the caller then, and I need a wc_ctx to do
> the
> wrapping below. Or do you mean that I would let the adm_access
> argument
> stay and just remove the use of it as I've done in this patch? I wish
> there was a way to remove the adm_access, store the wc_ctx in a baton
> and still be able to create a wrapper in libsvn_wc/deprecated.c.
> Have I
> missed something?
You could create the wc_ctx from the anchor access baton in
svn_wc_get_diff_editor5() (following the pattern elsewhere in
deprecated.c) and store that in the diff_callbacks3_wrapper_baton.
You'd then have it available in the various wrapper callbacks to
create the shadow access batons. Does that solve your problem?
>>> svn_wc_notify_state_t *contentstate,
>>> svn_wc_notify_state_t *propstate,
>>> svn_boolean_t *tree_conflicted,
>>> Index: subversion/libsvn_wc/deprecated.c
>>> ===================================================================
>>> --- subversion/libsvn_wc/deprecated.c (revision 39314)
>>> +++ subversion/libsvn_wc/deprecated.c (arbetskopia)
>>> @@ -39,6 +39,7 @@
>>> #include "props.h"
>>>
>>> #include "svn_private_config.h"
>>> +#include "private/svn_wc_private.h"
>>>
>>> /* baton for traversal_info_update */
>>> struct traversal_info_update_baton
>>> @@ -1275,7 +1276,7 @@
>>> /* An svn_wc_diff_callbacks4_t function for wrapping
>>> * svn_wc_diff_callbacks3_t. */
>>> static svn_error_t *
>>> -wrap_4to3_file_changed(svn_wc_adm_access_t *adm_access,
>>> +wrap_4to3_file_changed(svn_wc_context_t *wc_ctx,
>>> svn_wc_notify_state_t *contentstate,
>>> svn_wc_notify_state_t *propstate,
>>> svn_boolean_t *tree_conflicted,
>>> @@ -1291,11 +1292,21 @@
>>> void *diff_baton)
>>> {
>>> struct diff_callbacks3_wrapper_baton *b = diff_baton;
>>> + svn_wc_adm_access_t *adm_access;
>>>
>>> - return b->callbacks3->file_changed(adm_access, contentstate,
>>> propstate,
>>> - tree_conflicted, path,
>>> tmpfile1, tmpfile2,
>>> - rev1, rev2, mimetype1,
>>> mimetype2,
>>> - propchanges, originalprops, b-
>>>> baton);
>>> + SVN_ERR(svn_wc__adm_open_in_context(&adm_access, wc_ctx, path,
>>> + TRUE, /* write_lock */
>>> + -1, /* levels_to_lock */
>>> + NULL, /* svn_cancel_func_t */
>>> + NULL, /* cancel_baton */
>>> + wc_ctx->state_pool));
>
> /Daniel
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395019
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395113
Received on 2009-09-15 16:57:42 CEST