You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2003/02/22 15:40:17 UTC
Re: svn commit: rev 5001 - in trunk/subversion: include libsvn_wc
libsvn_client libsvn_delta
On Sat, 2003-02-22 at 09:41, Philip Martin wrote:
> I don't think svn_types.h is correct. The "cancellation" editor
> doesn't really have anything to do with cancellation, it's simply an
> editor that calls a given function before dispatching to another
> editor. If you renamed it the "prefix" editor it would live quite
> naturally in svn_delta.h. It could even be generalised to have both
> prefix and suffix functions.
I'm not sure if making it more abstract will make our code clearer or
not. It's an interesting idea, though.
But it seems wrong to, having accepted the argument that the
cancellation editor should live in libsvn_delta, exclude it from
svn_delta.h and move it to a header file which doesn't even know the
nceessary types (svn_delta_editor_t).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 5001 - in trunk/subversion: include libsvn_wc libsvn_client libsvn_delta
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Saturday, February 22, 2003, at 12:15 PM, Greg Hudson wrote:
> On Sat, 2003-02-22 at 12:11, Garrett Rooney wrote:
>> but the cancellation editor is in svn_delta.h right now. the only
>> thing left in svn_cancel.h is the typedef for svn_cancel_func_t,
>> that's
>> what greg is proposing we move to svn_types.h.
>
> Ah, I failed to understand. Still, it seems like it belongs right next
> to the declaration of svn_delta_get_cancellation_editor.
see, i disagree on that point. there's nothing about the cancellation
callback that implies it has anything to do with editors, that's just
an implementation detail. i don't see why svn_client.h should be
including svn_delta.h just to get a typedef that has nothing to do with
libsvn_delta other than being used by an editor that lives in
libsvn_delta.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 5001 - in trunk/subversion: include libsvn_wc
libsvn_client libsvn_delta
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sat, 2003-02-22 at 12:11, Garrett Rooney wrote:
> but the cancellation editor is in svn_delta.h right now. the only
> thing left in svn_cancel.h is the typedef for svn_cancel_func_t, that's
> what greg is proposing we move to svn_types.h.
Ah, I failed to understand. Still, it seems like it belongs right next
to the declaration of svn_delta_get_cancellation_editor.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 5001 - in trunk/subversion: include libsvn_wc libsvn_client libsvn_delta
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Saturday, February 22, 2003, at 10:40 AM, Greg Hudson wrote:
> On Sat, 2003-02-22 at 09:41, Philip Martin wrote:
>> I don't think svn_types.h is correct. The "cancellation" editor
>> doesn't really have anything to do with cancellation, it's simply an
>> editor that calls a given function before dispatching to another
>> editor. If you renamed it the "prefix" editor it would live quite
>> naturally in svn_delta.h. It could even be generalised to have both
>> prefix and suffix functions.
>
> I'm not sure if making it more abstract will make our code clearer or
> not. It's an interesting idea, though.
>
> But it seems wrong to, having accepted the argument that the
> cancellation editor should live in libsvn_delta, exclude it from
> svn_delta.h and move it to a header file which doesn't even know the
> nceessary types (svn_delta_editor_t).
but the cancellation editor is in svn_delta.h right now. the only
thing left in svn_cancel.h is the typedef for svn_cancel_func_t, that's
what greg is proposing we move to svn_types.h.
as for making it more generic, i agree with greg h, it seems like it'll
just make the code less clear. right now it's painfully obvious from
the function name that we're wrapping the editor for cancellation
purposes, if we make it into svn_delta_get_prefix_editor, it's much
less clear.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org