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

Re: new conflict callback API

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 17 May 2012 11:09:48 +0100 (BST)

Stefan Sperling wrote:
> On Wed, May 16, 2012 at 02:52:06PM +0100, Julian Foad wrote:
>> OK, I appreciate that issue.  I see two ways the client lib can help:
>>
>> 1. providing a set of client-lib APIs to perform the most commonly
>> wanted resolutions;
>>
>> 2. providing a set of (APIs? comments?) that suggest what set of
>> possible resolutions to offer for a given conflict.
>>
>> Does that match what you're aiming for?
>
> Not quite. I am aiming for extensive coverage, not just the "most
> common" cases.
>
> The resolver should be good enough to handle any conflict people might
> run into, and offer all reasonable resolution strategies for a given
> conflict. We won't get there in one day, of course. It will evolve with
> time.

Let's take a closer look at what the user (through the client
application) needs to be able to do.

>  /* The set of all conflict choices for all kinds of conflicts. */
>  enum svn_client_resolution_choice_t {
>    svn_client_resolution_choice_mark_resolved,
>    svn_client_resolution_choice_theirs_full,
>    svn_client_resolution_choice_mine_full,

"theirs-full" and "mine-full" are nice and simple: each choice leads to a
 single, definite result.  If the user chooses one of these, the
Subversion library can simply execute the user's decision with no
further info or interaction needed.

>    svn_client_resolution_choice_theirs_conflict,
>    svn_client_resolution_choice_mine_conflict,

"theirs-conflict" and "mine-conflict" are good options for an
interactive 3-way merge tool to offer the user while showing the user
which sections of the file do have conflicts and which don't.  It's not
an option I'd like to choose outside a 3-way merge tool.  In terms of
tree conflicts, the same applies: I'd only be comfortable with this
option when it's presented in a 3-way directory-merge window that's
showing me which children are conflicting and which are not.  After
pressing this button in the 3-way (file or dir) merge tool, I wouldn't
expect the tool to immediately close and Subversion library to
immediately finalize my choice, instead I'd expect to be able to review
and edit the result and then finally confirm.

So, the resolution that the 3-way merge tool provides to the Subversion
library should not be a simple "he chose 'mine-conflict'", but rather
should be a complete file (or directory tree) that the user finally
chose as the result.  As an implementation alternative, of course it could be in the form of instructions to
generate such a file (or dir).

>    [...]
>    svn_client_resolution_choice_diff_full,

"diff-full" isn't a resolution choice, it's the user asking to see more information before choosing.  We need to get away from the mode
of operation of the existing interactive resolver loop where the user interacts my means of question-and-answer.  That question-and-answer loop should be in the presentation layer (svn), not in the library.

>    svn_client_resolution_choice_edit_file,

"edit-file" isn't a resolution choice that the library can then
"execute" in a black-box manner.  Instead, the user wants the GUI to
help him/her prepare the final result with the aid of an editor, and
then the user may choose to confirm that resulting file as the final
resolved result.  The resolution needs to be communicated to the Svn lib
 in the form of a file, not "he chose 'edit' as the resolution".

>    [...]
>    svn_client_resolution_choice_scan_log_for_moves,

Not a resolution.  Instead, there should be a way for the GUI to get the
 suggestion(s) of what moves might be applicable, and to offer those as
suggestions to the user, and then for the user to select maybe one of
the choices or maybe another resolution.

>    svn_client_resolution_choice_specify_local_target_for_incoming_move,
>    svn_client_resolution_choice_merge_edits_into_local_file,
>    [...]
>  };

A GUI wants the user to be in control.  The first obvious thing is the user should be able to select which file/dir to resolve next.  If the GUI wants to have a button that enters a loop that iterates over all conflicts sequentially, that's it's business and we should make it possible to do that, but we should not implement this loop in the Svn lib.

The overall process the user goes through to resolve a particular conflict needs to include a reviewing phase, an editing phase, and finally a confirmation that the result is as the user wants it.  How does this fit with the idea of there being a point where the user chooses a resolution such as "choose edit-file"?

While in the reviewing phase of resolving any particular conflict, the user should be able to bring up, on demand, a diff
between 'mine' and 'base' or 'mine:theirs' or 'base:theirs' or whatever
he/she chooses, jump to the location in their editor where the next text conflict occurs, review the log messages on the source and/or target
branch,

and so on.  And all of this in parallel, in separate, non-modal windows.

To enable this, the application needs access to the metadata about the conflict.  This metadata will include at least the WC location or repository location (URL, rev, etc.) of each of the three files/dirs involved.  It needs to be sufficient to be able to display any required diffs, displaying friendly file
names (not the names of temp files) and labels (e.g. "Mine", "Theirs").

In short, it needs information like this:

  svn_node_kind_t node_kind;
  svn_wc_conflict_kind_t kind;

  svn_wc_conflict_action_t action;
  svn_wc_conflict_reason_t reason;
  svn_wc_operation_t operation;

  const char *base_abspath;
  const char *their_abspath;
  const char *my_abspath;
  const char *merged_file;

  const svn_wc_conflict_version_t *src_left_version;
  const svn_wc_conflict_version_t *src_right_version;

... which is an extract from svn_wc_conflict_description2_t.  Certainly we don't want the data to be presented in an "svn_wc_" struct, but I can't get away from the fact that that's the kind of metadata we need.

> There should be no reason for client UIs to implement custom resolution
> strategies. Any such strategies belong into the core. If UI developers
> have a special need they should talk to us and ask for a change in the
> resolver if need be. The current situation is too fragmented and confusing.
> I don't think it helps our users if every client developer keeps
> reinventing the wheel. That just leads to inconsistent behaviour and
> indicates a deficiency of the core library.
>
> So, the APIs should present the supported options, and then act upon
> the choice made by the user. I don't see why you would want to split
> these APIs into two separate categories. They belong together.

So, first the user needs to be presented with all the information so he/she can review the situation before making a choice.  Then there are some choices that the Svn lib can simply execute (such as choose 'mine-full').  Then there are other choices that are not a simple matter of stating a one-of-N choice, but rather involve interactive editing in a 3-way (file or dir) merge tool and presenting Subversion with the final result from that.

> As I mentioned, if clients still want to mess with conflicts on their
> own using any of the existing APIs, they can do so. We won't force client
> API users to run the resolver, just strongly encourage them to do so.
>
> Note that all of this will run during an offline resolve, *not* during
> the current interactive conflict resolution during update/merge.

I like and fully support the idea of doing resolution in this way, as a separate pass after the update/merge has finished, if it's possible.

> I intend to remove interactive conflict resolution during update/merge
> for 1.8 and make 'svn resolve' handle all conflict resolution instead
> (at least to the same degree as interactive conflict resolution in 1.7,
> if not better).

That's a subject for a new thread.

- Julian
Received on 2012-05-17 12:10:31 CEST

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