On Mon, 2011-08-29 at 13:27 +0200, Stefan Sperling wrote:
> On Fri, Aug 26, 2011 at 05:27:16PM +0100, Julian Foad wrote:
> > I (Julian Foad) wrote:
> > > I was just cleaning up this editor, stripping out this "target" path
> > > completely, so that it would simply pass relative paths out to the diff
> > > callbacks and to the notification functions. The merge code knows what
> > > WC path this diff relates to, and so can add the prefix itself inside
> > > its callbacks. That will keep the knowledge about the WC completely
> > > separate from the repos-diff editor, which is a good thing.
> > [...]
> >
> > Attached is my patch in progress to strip WC path info from the
> > repos-repos diff editor.
>
> I agree with your layering concerns.
> There is one point I am not sure about. You haven't explained
> how you want to address the following:
>
> > @@ -1110,30 +1095,9 @@ close_file(void *file_baton,
[...]
> > - notify = svn_wc_create_notify(moved_to_abspath ? moved_to_abspath
> > - : b->wcpath,
> > - action, scratch_pool);
> > + notify = svn_wc_create_notify(b->path, action, scratch_pool);
>
> You cannot just use b->path here. There is a chicken-and-egg problem.
>
> If the node was moved locally, this layer doesn't know about the new
> location of the node, unless it runs svn_wc__node_was_moved_away()
>
> I agree that calling it here is bad layering.
> But I think notifications should show the new paths.
>
> b->path is the old location of the node.
Agreed, so far.
> The layer which modifies the WC already knows about the new path
> but it does not perform notification.
> As I wrote in the comment your patch removes, it would be good to pass
> the proper path up to layer which performs the notifications.
As we discussed in IRC just now, this isn't so hard. merge.c is in
control of modifying the WC and also in control of notifications. It
tells the repos-diff editor to send notifications to
merge.c:notification_receiver(). So it can do the transformation from
repos path to WC moved-to path there.
> This would imply a change in the editor interface.
Doesn't look like that's needed. I'll proceed.
- Julian
> (It is becoming very apparent now that editor v1 was not designed
> with moves in mind...)
Received on 2011-08-30 12:52:11 CEST