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

branch tree-conflicts-notify: how many columns? -- was: merge to trunk?

From: Neels J. Hofmeyr <neels_at_elego.de>
Date: Sun, 26 Oct 2008 02:25:33 +0200

Stephen Butler wrote:
> Quoting "Neels J. Hofmeyr" <neels_at_elego.de>:
>
>> Hey Steve! (sbutler)
>>
>> That's so awesome! Somehow you've magically fixed almost everything on
>> the
>> branch! All I had to do was raise and notify a tree-conflict on file
>> addition in a conflicted directory.
>
> Thanks!
>
> BTW, I think we should (silently) skip a file-add in a tree-conflicted
> tree, rather than flag a new tree conflict. And of course skip all
> other changes, too. Otherwise the user might see a whole treeful of
> t.c. victims, and not be sure where the problem started. A single
> tree conflict, raised at the root of the tree during open_directory(),
> should be enough.

Hey, of course. What was I thinking. If a directory is tree-conflicted,
everything inside is naturally going to be a tree-conflict and doesn't need
to be listed in detail.

Oh, but what about the case when a directory has a previously raised,
persisting tree-conflict, and say update wants to add a file in it.
Shouldn't that then be a new, separate tree-conflict?

>
>> Should we add the M or lose the A and D? I'd say add the M and
>> whatever else
>> would have been shown without a tree-conflict, to be able to see what was
>> tried by the update. (Maybe there are more cases to this, though.)
>
> I believe the first two columns tell the user what changes the update
> has made in their working copy. If a path is skipped due to a tree
> conflict, we should leave these columns blank.
>
> Of course, we plan to skip all tree conflict victims (very soon), so
> the 'x C' will become ' C' in all cases.
>
> If ' C' is too cryptic, we could add the '(M->!)' later.

Yes, I believe it should be visible what was being tried when encountering a
tree-conflict. Either way, 'D C' or ' C(D->M)' but btw, I'm actually -1
on the brackets and even on the arrow. I'd prefer having ' CDM'. Much less
characters to read, and less unnecessary indentation for most of the cases
where there are no tree-conflicts. Here's a sort of scale of possibilities:

notify nr of additional columns
========== ===
   C 1
D C 1
D CM 2
   CDM 3 <-- neels likes this one best.
   CD>M 4
   C D>M 5
   C:D>M 5
   C D->M 6
   C:D->M 6
   C(D>M) 6
   C D->M 6
   C(D->M) 7
   ...
   Tree-Conflict: ((( Delete ---> Modified ))) 25 ;)

So I'd like to stay near the top of this list, because most of the time, the
additional columns will be empty anyway, making it harder to see which
normal notification (on the far left) goes with which path.

Also, the one I like best conforms to the way notification is used until
now: we're just adding two columns for conflict action and reason, no more
and no less. And naturally this should be similar across commands.

>
>>
>>
>> Either way, I think this branch is ready for reintegration into trunk!!
>> What do you think, Steve?
>> (still in need of a complete `make check' though)
>
> Yes, if the switch and merge tests aren't badly affected. Looking
> into that now.

Well, actually, that was a bit quick by me. Let's bugfix some more.

>> The complete local tree was removed, so if you try to add a file a few
>> levels inside a removed dir, all the non-existing path elements are in
>> tree
>> conflict. Makes sense to me.
>
> But the whole tree is already tree-conflicted, so raising new tree
> conflicts insided it is redundant.

Yes. Somehow I forgot to not want redundancy.

>> The check_tree_conflict() vs. svn_wc_conflicted_p() discussion made me
>> think: In the tests, I guess we should also try to run the same operation
>> that causes the conflicts again, to check that the persisting conflicts
>> aren't re-raised or missed altogether. Is this accounted for yet?
>
> No, currently we test only fresh tree conflicts. Good point.

I'll try that now if you haven't already.

>
>>
>>
>> And to squeeze yet another topic into this mail: What's the deal with the
>> run_and_verify_unquiet_status tests -- is status not intended to show
>> tree-conflicts (on missing nodes) by default?
>
> True. The final status check in every run_and_verify_X() runs with the
> -q option, ignoring files and dirs that aren't under version control.

Ah, I see ... Nice catch!

>
> I think I'll change our DeepTreesTestCase class (on trunk) to do the
> status check unquietly, because so many t.c. victims don't exist.

+1

~Neels

Received on 2008-10-26 02:25:59 CEST

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