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 Stein <gs...@lyra.org> on 2003/02/22 05:45:10 UTC
Re: svn commit: rev 5001 - in trunk/subversion: include libsvn_wc libsvn_client libsvn_delta
On Thu, Feb 20, 2003 at 08:58:44PM -0600, rooneg@tigris.org wrote:
> Author: rooneg
> Date: 2003-02-20 20:58:29 -0600 (Thu, 20 Feb 2003)
> New Revision: 5001
>
> Added:
> trunk/subversion/include/svn_cancel.h
Why the new header? We've got way too many as it is :-(
I'd say add it to svn_delta.h or to svn_types.h.
>...
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
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
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 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 Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> On Saturday, February 22, 2003, at 12:45 AM, Greg Stein wrote:
>
> > On Thu, Feb 20, 2003 at 08:58:44PM -0600, rooneg@tigris.org wrote:
> >>
> >> Added:
> >> trunk/subversion/include/svn_cancel.h
> >
> > Why the new header? We've got way too many as it is :-(
> >
> > I'd say add it to svn_delta.h or to svn_types.h.
>
> well, i didn't put it in svn_delta.h since i personally would never
> think to look for it there, since cancellation doesn't intrisically
> have anything to do with tree deltas (the cancellation editor does,
> but not the callback itself). svn_types.h sounds much more reasonable
> though, i just hadn't thought of that. i'll move it in there today.
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.
--
Philip Martin
---------------------------------------------------------------------
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:45 AM, Greg Stein wrote:
> On Thu, Feb 20, 2003 at 08:58:44PM -0600, rooneg@tigris.org wrote:
>> Author: rooneg
>> Date: 2003-02-20 20:58:29 -0600 (Thu, 20 Feb 2003)
>> New Revision: 5001
>>
>> Added:
>> trunk/subversion/include/svn_cancel.h
>
> Why the new header? We've got way too many as it is :-(
>
> I'd say add it to svn_delta.h or to svn_types.h.
well, i didn't put it in svn_delta.h since i personally would never
think to look for it there, since cancellation doesn't intrisically
have anything to do with tree deltas (the cancellation editor does, but
not the callback itself). svn_types.h sounds much more reasonable
though, i just hadn't thought of that. i'll move it in there today.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org