You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Neels Janosch Hofmeyr <ne...@elego.de> on 2009/06/11 01:23:56 UTC
editor v2 and replace
Hi Greg,
IMHO it would be nice to have an svn_editor_cb_replace_t in editor v2.
I'm currently faced with a merge that replaces an item. The item is first
deleted and later added in a given merge range. Thus, merge first calls
file_deleted() and then file_added() (current "diff editor" callbacks).
That's a little awkward for
- displaying "R" instead of two lines "D" and "A" (diff --summarize?)
- making sense of a tree conflict. I'm currently fixing exactly this case,
where a merge replaces a file on top of a locally modified file. The current
code tries to create two tree-conflicts on the same item and fails.
It would need generic arguments for mixed node kinds...
If you agree, I'll be glad to add that, so you can review it to shreds ;)
~Neels
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361126
Re: editor v2 and replace
Posted by Neels Janosch Hofmeyr <ne...@elego.de>.
Hi again,
how about making that "is_replace" flag something like an svn_node_kind_t?
Then the receiver already knows what kind of node it is supposed to be
replacing and doesn't need to figure that out painfully.
A clean add would simply pass svn_node_none.
I don't know what svn_node_unknown would represent, though, and it seems
we'd also need to represent a "symlink" and an "absent" node kind... In fact
I don't grok the "add_absent" callback, and how symlinks are treated.
Are we going to extend svn_node_kind_t? What else would make sense?
A pseudo-diff to illustrate:
[[[
svn_editor_add_directory(svn_editor_t *editor,
const char *relpath,
const apr_array_header_t *children,
- apr_hash_t *props);
+ apr_hash_t *props,
+ svn_node_kind_t replaces);
svn_error_t *
svn_editor_add_file(svn_editor_t *editor,
const char *relpath,
- apr_hash_t *props);
+ apr_hash_t *props,
+ svn_node_kind_t replaces);
svn_error_t *
svn_editor_add_symlink(svn_editor_t *editor,
const char *relpath,
const char *target,
- apr_hash_t *props);
+ apr_hash_t *props,
+ svn_node_kind_t replaces);
svn_error_t *
svn_editor_add_absent(svn_editor_t *editor,
const char *relpath,
- svn_node_kind_t kind);
+ svn_node_kind_t kind,
+ svn_node_kind_t replaces);
]]]
Thanks some more,
~Neels
Neels Janosch Hofmeyr wrote:
> Greg Stein wrote:
>> Hmm. I figured that with the "add follows" flag, then you could know
>> to avoid dual-notifications, and just wait for the add. But yah...
>> then you'd need to remember (at add time) that a delete had happened.
>> More cross-call state.
>>
>> Adding a flag to the add calls should work. Tho I hate the "replace"
>> terminology. Maybe "overwrite=TRUE" is better? (and, of course,
>
> Hm, I much prefer the "replace" terminology. The item was removed, and
> another one added in its place. Replace, right?
>
> If I read it the first time, "overwrite" would probably not convey its real
> meaning to me, I guess I would associate it with an internal helper thing...
> but maybe that's just me.
>
>> failure if you're trying to overwrite, and nothing is there)
>>
>> Bikeshed. But yah. Extending the add calls sounds better. Thanks!
>
> Yes, that's very nice, thanks guys!
>
> ~Neels
>
>> Cheers,
>> -g
>>
>> On Thu, Jun 11, 2009 at 15:43, C. Michael Pilato<cm...@collab.net> wrote:
>>> You'll still got two editor calls, though. Could we instead just add an
>>> "is_replace" flag to the add operation? (Drivers would not call the delete
>>> in that situation.)
>>>
>>> Greg Stein wrote:
>>>> Yeah... I can see how the separation makes things more difficult.
>>>>
>>>> Rather than a replace, with various node kind magic... how about a
>>>> flag on the remove() operation which says "add follows" ? That should
>>>> keep things much simpler, and we wouldn't need a bunch of variants or
>>>> generic args.
>>>>
>>>> Cheers,
>>>> -g
>>>>
>>>> On Thu, Jun 11, 2009 at 03:23, Neels J Hofmeyr<ne...@elego.de> wrote:
>>>>> Hi Greg,
>>>>>
>>>>> IMHO it would be nice to have an svn_editor_cb_replace_t in editor v2.
>>>>>
>>>>> I'm currently faced with a merge that replaces an item. The item is first
>>>>> deleted and later added in a given merge range. Thus, merge first calls
>>>>> file_deleted() and then file_added() (current "diff editor" callbacks).
>>>>>
>>>>> That's a little awkward for
>>>>>
>>>>> - displaying "R" instead of two lines "D" and "A" (diff --summarize?)
>>>>>
>>>>> - making sense of a tree conflict. I'm currently fixing exactly this case,
>>>>> where a merge replaces a file on top of a locally modified file. The current
>>>>> code tries to create two tree-conflicts on the same item and fails.
>>>>>
>>>>> It would need generic arguments for mixed node kinds...
>>>>> If you agree, I'll be glad to add that, so you can review it to shreds ;)
>>>>>
>>>>> ~Neels
>>>>>
>>>>>
>>>>>
>>>> ------------------------------------------------------
>>>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361206
>>> --
>>> C. Michael Pilato <cm...@collab.net>
>>> CollabNet <> www.collab.net <> Distributed Development On Demand
>>>
>>>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361477
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361480
Re: editor v2 and replace
Posted by Neels Janosch Hofmeyr <ne...@elego.de>.
Greg Stein wrote:
> Hmm. I figured that with the "add follows" flag, then you could know
> to avoid dual-notifications, and just wait for the add. But yah...
> then you'd need to remember (at add time) that a delete had happened.
> More cross-call state.
>
> Adding a flag to the add calls should work. Tho I hate the "replace"
> terminology. Maybe "overwrite=TRUE" is better? (and, of course,
Hm, I much prefer the "replace" terminology. The item was removed, and
another one added in its place. Replace, right?
If I read it the first time, "overwrite" would probably not convey its real
meaning to me, I guess I would associate it with an internal helper thing...
but maybe that's just me.
> failure if you're trying to overwrite, and nothing is there)
>
> Bikeshed. But yah. Extending the add calls sounds better. Thanks!
Yes, that's very nice, thanks guys!
~Neels
>
> Cheers,
> -g
>
> On Thu, Jun 11, 2009 at 15:43, C. Michael Pilato<cm...@collab.net> wrote:
>> You'll still got two editor calls, though. Could we instead just add an
>> "is_replace" flag to the add operation? (Drivers would not call the delete
>> in that situation.)
>>
>> Greg Stein wrote:
>>> Yeah... I can see how the separation makes things more difficult.
>>>
>>> Rather than a replace, with various node kind magic... how about a
>>> flag on the remove() operation which says "add follows" ? That should
>>> keep things much simpler, and we wouldn't need a bunch of variants or
>>> generic args.
>>>
>>> Cheers,
>>> -g
>>>
>>> On Thu, Jun 11, 2009 at 03:23, Neels J Hofmeyr<ne...@elego.de> wrote:
>>>> Hi Greg,
>>>>
>>>> IMHO it would be nice to have an svn_editor_cb_replace_t in editor v2.
>>>>
>>>> I'm currently faced with a merge that replaces an item. The item is first
>>>> deleted and later added in a given merge range. Thus, merge first calls
>>>> file_deleted() and then file_added() (current "diff editor" callbacks).
>>>>
>>>> That's a little awkward for
>>>>
>>>> - displaying "R" instead of two lines "D" and "A" (diff --summarize?)
>>>>
>>>> - making sense of a tree conflict. I'm currently fixing exactly this case,
>>>> where a merge replaces a file on top of a locally modified file. The current
>>>> code tries to create two tree-conflicts on the same item and fails.
>>>>
>>>> It would need generic arguments for mixed node kinds...
>>>> If you agree, I'll be glad to add that, so you can review it to shreds ;)
>>>>
>>>> ~Neels
>>>>
>>>>
>>>>
>>> ------------------------------------------------------
>>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361206
>>
>> --
>> C. Michael Pilato <cm...@collab.net>
>> CollabNet <> www.collab.net <> Distributed Development On Demand
>>
>>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361477
Re: editor v2 and replace
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Jun 11, 2009 at 19:27, Stefan Sperling<st...@elego.de> wrote:
> On Thu, Jun 11, 2009 at 07:12:37PM +0200, Greg Stein wrote:
>> On Thu, Jun 11, 2009 at 18:44, Stefan Sperling<st...@elego.de> wrote:
>> > I'd like to point that I'd also like an atomic editor
>> > call for move(), please!
>> >
>> > Can we do that?
>>
>> RTF<Include File>
>
> You mean grep move notes/editor-v2.txt ?
> Neat :)
subversion/include/svn_editor.h
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361398
Re: editor v2 and replace
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jun 11, 2009 at 07:12:37PM +0200, Greg Stein wrote:
> On Thu, Jun 11, 2009 at 18:44, Stefan Sperling<st...@elego.de> wrote:
> > I'd like to point that I'd also like an atomic editor
> > call for move(), please!
> >
> > Can we do that?
>
> RTF<Include File>
You mean grep move notes/editor-v2.txt ?
Neat :)
Stefan
Re: editor v2 and replace
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Jun 11, 2009 at 18:44, Stefan Sperling<st...@elego.de> wrote:
> On Thu, Jun 11, 2009 at 06:29:13PM +0200, Greg Stein wrote:
>> Hmm. I figured that with the "add follows" flag, then you could know
>> to avoid dual-notifications, and just wait for the add. But yah...
>> then you'd need to remember (at add time) that a delete had happened.
>> More cross-call state.
>>
>> Adding a flag to the add calls should work. Tho I hate the "replace"
>> terminology. Maybe "overwrite=TRUE" is better? (and, of course,
>> failure if you're trying to overwrite, and nothing is there)
>>
>> Bikeshed. But yah. Extending the add calls sounds better. Thanks!
>
> I'd like to point that I'd also like an atomic editor
> call for move(), please!
>
> Can we do that?
RTF<Include File>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361382
Re: editor v2 and replace
Posted by Neels Janosch Hofmeyr <ne...@elego.de>.
Stefan Sperling wrote:
> On Thu, Jun 11, 2009 at 06:29:13PM +0200, Greg Stein wrote:
>> Hmm. I figured that with the "add follows" flag, then you could know
>> to avoid dual-notifications, and just wait for the add. But yah...
>> then you'd need to remember (at add time) that a delete had happened.
>> More cross-call state.
>>
>> Adding a flag to the add calls should work. Tho I hate the "replace"
>> terminology. Maybe "overwrite=TRUE" is better? (and, of course,
>> failure if you're trying to overwrite, and nothing is there)
>>
>> Bikeshed. But yah. Extending the add calls sounds better. Thanks!
>
> I'd like to point that I'd also like an atomic editor
> call for move(), please!
>
> Can we do that?
>
> Stefan
[[[
$ grep move include/svn_editor.h
typedef svn_error_t *(*svn_editor_cb_move_t)(
svn_editor_setcb_move(svn_editor_t *editor,
svn_editor_cb_move_t callback,
svn_editor_cb_move_t cb_move;
svn_editor_move(svn_editor_t *editor,
]]]
So, yes. ;)
~Neels
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361478
Re: editor v2 and replace
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jun 11, 2009 at 06:29:13PM +0200, Greg Stein wrote:
> Hmm. I figured that with the "add follows" flag, then you could know
> to avoid dual-notifications, and just wait for the add. But yah...
> then you'd need to remember (at add time) that a delete had happened.
> More cross-call state.
>
> Adding a flag to the add calls should work. Tho I hate the "replace"
> terminology. Maybe "overwrite=TRUE" is better? (and, of course,
> failure if you're trying to overwrite, and nothing is there)
>
> Bikeshed. But yah. Extending the add calls sounds better. Thanks!
I'd like to point that I'd also like an atomic editor
call for move(), please!
Can we do that?
Stefan
Re: editor v2 and replace
Posted by Neels Janosch Hofmeyr <ne...@elego.de>.
Karl Fogel wrote:
> Neels Janosch Hofmeyr <ne...@elego.de> writes:
>> as to the real-life consequences, I'm still mostly grey. Haven't dug a lot
>> of code about this. But I've seen the cases where the current callbacks just
>> don't work well. Let's philosophize:
>>
>> Currently, combining separate editor calls to detect a replace has to be
>> implemented by each receiver that needs to know. Even if the driver has that
>> information right there, it still *has* to call delete() first, then add().
>>
>> Adding an atomic replace to the editor callbacks puts the load of the
>> combining work on the driver. That's also the intention.
>
> That sounds good. Actually, it's a more succinct way of expressing what
> I was trying to say.
>
>> Actually, we don't necessarily force *every* driver to make a replace
>> atomic. I could imagine that some "editor pipe" in the code just agrees to
>> not use that replace flag and remain issuing delete() and add() separately.
>> Then again, I can't currently imagine a case where that first driver in the
>> line would not have the replace information ready to pick up.
>
> So there will now be two ways to express the same action, via the
> editor? That is, you can call replace(A, B), or you can call delete(A)
> and then add(B), and the result should be exactly the same either way?
Admittedly, that's a little shaky. I think we should write a comment like this:
* A replace should be represented by a call to add(..., is_replace=TRUE)
* instead of a delete() followed by an add(). If you want to use the
* non-atomic way (a delete() followed by an add()), first consider it
* carefully and discuss with the Community. If it is accepted, it should
* be clearly marked as not using the standard way of operating the editor.
Also, I think the cases where the driver wants to send separate calls don't
worry that much about atomicity by definition. ...at least that's how it
*should* be :)
>
> It's a little worrisome to have two paths to the same result, but I
> don't think it's fatal... and see below about atomicity.
>
>> If a callback receiver doesn't need the atomic replace, it will just call
>> its own delete() callback before doing the usual add() (given that add()
>> will receive an is_replace or similar flag).
>
> Sure. Callback implementations are up to the callbacks, as long as they
> satisfy the promises of the editor v2 interface.
>
> Atomic replacement in the standard working copy is possible: you write
> intermediate data in a certain way, such that if svn is interrupted at
> any point before the replacement is completed, subsequent 'svn cleanup'
> *either* restores to the pre-replace status quo or completes the
> replacement.
>
> So perhaps the difference between achieving a replacement by calling
> replace() versus by calling delete()-then-add() could be that the former
> promises atomicity? (Atomicity through 'svn cleanup' is permitted.)
My 'atomic' is a little lower level. I'm currently worrying about how to
invoke the callbacks, not whether the resulting replace operations are
really being carried out atomically, yet. But this sounds good.
I've just seen incredibly spread out code in the callbacks implementations
that should actually just be a couple of consecutive lines. These might be
caching an awful lot of delete operations... it's Not Good.
Thanks a lot for your feedback :)
~Neels
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2362454
Re: editor v2 and replace
Posted by Karl Fogel <kf...@red-bean.com>.
Neels Janosch Hofmeyr <ne...@elego.de> writes:
> as to the real-life consequences, I'm still mostly grey. Haven't dug a lot
> of code about this. But I've seen the cases where the current callbacks just
> don't work well. Let's philosophize:
>
> Currently, combining separate editor calls to detect a replace has to be
> implemented by each receiver that needs to know. Even if the driver has that
> information right there, it still *has* to call delete() first, then add().
>
> Adding an atomic replace to the editor callbacks puts the load of the
> combining work on the driver. That's also the intention.
That sounds good. Actually, it's a more succinct way of expressing what
I was trying to say.
> Actually, we don't necessarily force *every* driver to make a replace
> atomic. I could imagine that some "editor pipe" in the code just agrees to
> not use that replace flag and remain issuing delete() and add() separately.
> Then again, I can't currently imagine a case where that first driver in the
> line would not have the replace information ready to pick up.
So there will now be two ways to express the same action, via the
editor? That is, you can call replace(A, B), or you can call delete(A)
and then add(B), and the result should be exactly the same either way?
It's a little worrisome to have two paths to the same result, but I
don't think it's fatal... and see below about atomicity.
> If a callback receiver doesn't need the atomic replace, it will just call
> its own delete() callback before doing the usual add() (given that add()
> will receive an is_replace or similar flag).
Sure. Callback implementations are up to the callbacks, as long as they
satisfy the promises of the editor v2 interface.
Atomic replacement in the standard working copy is possible: you write
intermediate data in a certain way, such that if svn is interrupted at
any point before the replacement is completed, subsequent 'svn cleanup'
*either* restores to the pre-replace status quo or completes the
replacement.
So perhaps the difference between achieving a replacement by calling
replace() versus by calling delete()-then-add() could be that the former
promises atomicity? (Atomicity through 'svn cleanup' is permitted.)
> Is that Go? :)
Oh, I wasn't trying to stand in the way of anything :-). Just wanted to
voice a design concern on the working copy side, that's all.
-Karl
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361916
Re: editor v2 and replace
Posted by Neels Janosch Hofmeyr <ne...@elego.de>.
Karl Fogel wrote:
> Greg Stein <gs...@gmail.com> writes:
>> Hmm. I figured that with the "add follows" flag, then you could know
>> to avoid dual-notifications, and just wait for the add. But yah...
>> then you'd need to remember (at add time) that a delete had happened.
>> More cross-call state.
>>
>> Adding a flag to the add calls should work. Tho I hate the "replace"
>> terminology. Maybe "overwrite=TRUE" is better? (and, of course,
>> failure if you're trying to overwrite, and nothing is there)
>>
>> Bikeshed. But yah. Extending the add calls sounds better. Thanks!
>
> I know I'm sort of chiming in from the late Jurassic here, but:
>
> While +1 on 'replace' as a single operation in editor v2, I think it's
> important that the editor call sequence match up the working-copy
> metadata. In other words, if someone replaces 'foo' with 'bar' in their
> working copy...
>
> $ svn rm foo
> $ svn mv bar foo
>
> ...then the working copy (v2) metadata should clearly indicate on both
> sides that it's a replacement, and when the editor call happens, it
> should record (also on both sides) that the local state has now been
> "satisfied" by the editor.
>
> This could be a little tricky, because you don't know which side you'll
> encounter first in a commit or update crawl. I guess what I'm saying
> is, encountering either side should be like encountering both, as far as
> the working copy is concerned: the single editor call should update both
> sides.
>
> Is that too hand-wavy to be meaningful? Do you see what I'm getting at?
> (Or is this already taken for granted?)
Hi Karl,
as to the real-life consequences, I'm still mostly grey. Haven't dug a lot
of code about this. But I've seen the cases where the current callbacks just
don't work well. Let's philosophize:
Currently, combining separate editor calls to detect a replace has to be
implemented by each receiver that needs to know. Even if the driver has that
information right there, it still *has* to call delete() first, then add().
Adding an atomic replace to the editor callbacks puts the load of the
combining work on the driver. That's also the intention.
Actually, we don't necessarily force *every* driver to make a replace
atomic. I could imagine that some "editor pipe" in the code just agrees to
not use that replace flag and remain issuing delete() and add() separately.
Then again, I can't currently imagine a case where that first driver in the
line would not have the replace information ready to pick up.
If a callback receiver doesn't need the atomic replace, it will just call
its own delete() callback before doing the usual add() (given that add()
will receive an is_replace or similar flag).
Is that Go? :)
~Neels
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361879
Re: editor v2 and replace
Posted by Karl Fogel <kf...@red-bean.com>.
Greg Stein <gs...@gmail.com> writes:
> Hmm. I figured that with the "add follows" flag, then you could know
> to avoid dual-notifications, and just wait for the add. But yah...
> then you'd need to remember (at add time) that a delete had happened.
> More cross-call state.
>
> Adding a flag to the add calls should work. Tho I hate the "replace"
> terminology. Maybe "overwrite=TRUE" is better? (and, of course,
> failure if you're trying to overwrite, and nothing is there)
>
> Bikeshed. But yah. Extending the add calls sounds better. Thanks!
I know I'm sort of chiming in from the late Jurassic here, but:
While +1 on 'replace' as a single operation in editor v2, I think it's
important that the editor call sequence match up the working-copy
metadata. In other words, if someone replaces 'foo' with 'bar' in their
working copy...
$ svn rm foo
$ svn mv bar foo
...then the working copy (v2) metadata should clearly indicate on both
sides that it's a replacement, and when the editor call happens, it
should record (also on both sides) that the local state has now been
"satisfied" by the editor.
This could be a little tricky, because you don't know which side you'll
encounter first in a commit or update crawl. I guess what I'm saying
is, encountering either side should be like encountering both, as far as
the working copy is concerned: the single editor call should update both
sides.
Is that too hand-wavy to be meaningful? Do you see what I'm getting at?
(Or is this already taken for granted?)
-Karl
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361851
Re: editor v2 and replace
Posted by Greg Stein <gs...@gmail.com>.
Hmm. I figured that with the "add follows" flag, then you could know
to avoid dual-notifications, and just wait for the add. But yah...
then you'd need to remember (at add time) that a delete had happened.
More cross-call state.
Adding a flag to the add calls should work. Tho I hate the "replace"
terminology. Maybe "overwrite=TRUE" is better? (and, of course,
failure if you're trying to overwrite, and nothing is there)
Bikeshed. But yah. Extending the add calls sounds better. Thanks!
Cheers,
-g
On Thu, Jun 11, 2009 at 15:43, C. Michael Pilato<cm...@collab.net> wrote:
> You'll still got two editor calls, though. Could we instead just add an
> "is_replace" flag to the add operation? (Drivers would not call the delete
> in that situation.)
>
> Greg Stein wrote:
>> Yeah... I can see how the separation makes things more difficult.
>>
>> Rather than a replace, with various node kind magic... how about a
>> flag on the remove() operation which says "add follows" ? That should
>> keep things much simpler, and we wouldn't need a bunch of variants or
>> generic args.
>>
>> Cheers,
>> -g
>>
>> On Thu, Jun 11, 2009 at 03:23, Neels J Hofmeyr<ne...@elego.de> wrote:
>>> Hi Greg,
>>>
>>> IMHO it would be nice to have an svn_editor_cb_replace_t in editor v2.
>>>
>>> I'm currently faced with a merge that replaces an item. The item is first
>>> deleted and later added in a given merge range. Thus, merge first calls
>>> file_deleted() and then file_added() (current "diff editor" callbacks).
>>>
>>> That's a little awkward for
>>>
>>> - displaying "R" instead of two lines "D" and "A" (diff --summarize?)
>>>
>>> - making sense of a tree conflict. I'm currently fixing exactly this case,
>>> where a merge replaces a file on top of a locally modified file. The current
>>> code tries to create two tree-conflicts on the same item and fails.
>>>
>>> It would need generic arguments for mixed node kinds...
>>> If you agree, I'll be glad to add that, so you can review it to shreds ;)
>>>
>>> ~Neels
>>>
>>>
>>>
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361206
>
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361357
Re: editor v2 and replace
Posted by "C. Michael Pilato" <cm...@collab.net>.
You'll still got two editor calls, though. Could we instead just add an
"is_replace" flag to the add operation? (Drivers would not call the delete
in that situation.)
Greg Stein wrote:
> Yeah... I can see how the separation makes things more difficult.
>
> Rather than a replace, with various node kind magic... how about a
> flag on the remove() operation which says "add follows" ? That should
> keep things much simpler, and we wouldn't need a bunch of variants or
> generic args.
>
> Cheers,
> -g
>
> On Thu, Jun 11, 2009 at 03:23, Neels J Hofmeyr<ne...@elego.de> wrote:
>> Hi Greg,
>>
>> IMHO it would be nice to have an svn_editor_cb_replace_t in editor v2.
>>
>> I'm currently faced with a merge that replaces an item. The item is first
>> deleted and later added in a given merge range. Thus, merge first calls
>> file_deleted() and then file_added() (current "diff editor" callbacks).
>>
>> That's a little awkward for
>>
>> - displaying "R" instead of two lines "D" and "A" (diff --summarize?)
>>
>> - making sense of a tree conflict. I'm currently fixing exactly this case,
>> where a merge replaces a file on top of a locally modified file. The current
>> code tries to create two tree-conflicts on the same item and fails.
>>
>> It would need generic arguments for mixed node kinds...
>> If you agree, I'll be glad to add that, so you can review it to shreds ;)
>>
>> ~Neels
>>
>>
>>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361206
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361285
Re: editor v2 and replace
Posted by Greg Stein <gs...@gmail.com>.
Yeah... I can see how the separation makes things more difficult.
Rather than a replace, with various node kind magic... how about a
flag on the remove() operation which says "add follows" ? That should
keep things much simpler, and we wouldn't need a bunch of variants or
generic args.
Cheers,
-g
On Thu, Jun 11, 2009 at 03:23, Neels J Hofmeyr<ne...@elego.de> wrote:
> Hi Greg,
>
> IMHO it would be nice to have an svn_editor_cb_replace_t in editor v2.
>
> I'm currently faced with a merge that replaces an item. The item is first
> deleted and later added in a given merge range. Thus, merge first calls
> file_deleted() and then file_added() (current "diff editor" callbacks).
>
> That's a little awkward for
>
> - displaying "R" instead of two lines "D" and "A" (diff --summarize?)
>
> - making sense of a tree conflict. I'm currently fixing exactly this case,
> where a merge replaces a file on top of a locally modified file. The current
> code tries to create two tree-conflicts on the same item and fails.
>
> It would need generic arguments for mixed node kinds...
> If you agree, I'll be glad to add that, so you can review it to shreds ;)
>
> ~Neels
>
>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361206