[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r33944 - in branches/tc-merge-notify/subversion: include libsvn_client libsvn_wc

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 30 Oct 2008 12:45:19 +0000

On Thu, 2008-10-30 at 03:10 +0100, Neels J. Hofmeyr wrote:
>
> Stephen Butler wrote:
> > Quoting "Neels J. Hofmeyr" <neels_at_elego.de>:
> >
> >> Hey tree-conflicts folks,
> >>
> >> please review and/or jump right in and fix stuff.
> >>
> >> Thanks!
> >> ~Neels
> [...]
> > But I think there's some extra changes needed to support
> > skipping the victims. We need to separate tree conflict
> > notifications from all of the others.
> >
> > I propose that we remove the (new) tree_conflicted field from
> > svn_wc_notify_t, and add one or more notify-actions instead.
> > At the same time, change the notify() function in the client to
> > accept the new tree conflict notifications.
>
> Hey, that's actually a very good idea. I did this thinking that it would be
> good to have the usual notification alongside the conflict. But that is
> better solved with two new columns, as we discussed elsewhere.
>
> >
> > Specifically, in notify(), we should add a couple of new cases,
> > and remove the 'C' in the fourth column from all other cases
> > that have it. E.g.:
> >
> > switch (n->action)
> > {
> > case svn_wc_notify_tree_conflict:
> > nb->in_external ? nb->ext_tree_conflicts++
> > : nb->tree_conflicts++;
> > if ((err = svn_cmdline_printf(pool, " C %s\n", path_local)))
> > goto print_error;
> > break;
> >
> > case svn_wc_notify_conflict_skip:
> > nb->in_external ? nb->ext_skipped_paths++
> > : nb->skipped_paths++;
> > if ((err = svn_cmdline_printf
> > (pool, _("Skipped conflicted path '%s'\n"), path_local)))
> > goto print_error;
> > break;
> >
> >
> > The current code (for update, too) mixes tree conflict output
> > with other output.
> >
> > This will affect repos_diff.c. Callbacks like the following
> > should do tree conflict notification earlier, before skipping
> > the rest of the callback (including the notify code below).
> >
> > The notify code at the end of the repos_diff callbacks
> > shouldn't set the tree_conflicted field, because it won't
> > be in svn_wc_notify_t anymore.
> >
> > >> + notify->tree_conflicted = b->tree_conflicted;
>
> Huh? Where is that?
> Oh, you mean when removing that field. Naturally.
>
> >
> > Comments?
>
> About this
> > case svn_wc_notify_conflict_skip:
> that prints
> > (pool, _("Skipped conflicted path '%s'\n"), path_local)))
>
> Where is this going to be used, exactly? For persisting tree-conflicts? Not
> for nodes inside a newly tree-conflicted directory, I presume.

There is already a "skip" notification defined:

  /** The type of action occurring. */
  typedef enum svn_wc_notify_action_t
  {
  [...]
    /** Skipping a path. */
    svn_wc_notify_skip,
  [...]

Shouldn't we be using svn_wc_notify_skip as the "action occurring", and
set the "content_state" to "conflicted" or the "tree_conflicted" flag to
true to indicate that the reason is a conflict?

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-30 13:45:35 CET

This is an archived mail posted to the Subversion Dev mailing list.