Julian Foad wrote:
> Neels J. Hofmeyr wrote:
>>>>>> I suggest we un-deprecate the original svn_wc_conflicted_p() and use
>>>>>> it for reading text and prop conflict state only. And use the
>>>>> existing
>>>>>> svn_wc_get_tree_conflict() to read tree conflict state.
>>> Any arguments against this?
>> Yes, semi. I agree with taking away the has_tree_conflicted_children_p
>> parameter, but I would endorse keeping the tree_conflicted_p. For the simple
>> reason that anyone outside of the callbacks functions calls
>> svn_wc_conflicted_p.*() to see whether the node has a conflict that should
>> prevent it from changing something.
>
> OK, you're saying keep svn_wc_conflicted_p2() but change its
> tree_conflict output to report on the path as a victim:
>
> svn_error_t *
> svn_wc_conflicted_p2(svn_boolean_t *path_is_text_conflicted,
> svn_boolean_t *path_is_prop_conflicted,
> svn_boolean_t *path_is_tree_conflicted,
> path, entry, pool);
>
>> And I suddenly grasp this: Even in the
>> callbacks, tree_conflicted_p is useful to report any tree-conflicts that
>> have been around before the current operation was launched.
>
> Yes. Each part of the up/sw/merge operation is trying to say:
>
> if target was already in conflict:
> skip this action
> else
> if this action will cause a tree conflict:
> record the tree conflict
> skip this action
> else:
> do this action
Hmm, that'd make a nice comment :)
>
>> So we need both
>> the return value of check_tree_conflict() *and* tree_conflicted_p from
>> svn_wc_conflicted_p2(), right? (Unless check_tree_conflict() already covers
>> persistence of older conflicts, and then this point still holds true for use
>> outside the callbacks.)
>
> check_tree_conflict() doesn't detect pre-existing conflict, it only
> detects whether the current action is causing a conflict, so yes we need
> it and also (logically before it) a check for pre-existing conflicts.
>
> But Steve's point was that to check for pre-existing conflicts we have
> the API svn_wc_get_tree_conflict() to ask if a path is the victim of a
> tree conflict. We can use svn_wc_get_tree_conflict() together with
> svn_wc_conflicted_p() to answer the same question as your proposed
> svn_wc_conflicted_p2() will do.
>
> When svn_wc_conflicted_p2() was ansering "does this path have any
> tree-conflicted children", I thought it was better to separate out that
> question into a separate API, but now that we can answer all three of
> * is there a text conflict on this path?
> * is there a property conflict on this path?
> * is there a tree conflict on this path?
>
> with one svn_wc_conflicted_p2() API, it seems neat and useful to do so.
> I'm not entirely convinced it's the most useful API in the world: most
> callers of it simply want to know whether there are any conflicts on the
> path, and don't care what they are. But it's OK. Go for it. (First step:
> stop using the current svn_wc_conflicted_p2()'s
> contains-tree-conflicted-children parameter.)
Ah great, I see we're on the same page here. That was exactly my plan.
~Neels
--
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Received on 2008-10-24 03:11:41 CEST