Stefan Sperling <stsp_at_elego.de> writes:
> There's a similar hack for resolving tree-conflicts with --mine-conflict,
> which the client API does not expose as a valid option either.
>
> I'd say leave these as-is. We won't accumulate more such hacks. They
> are clearly marked as hacks in comments, and they exist only because
> we have to remain compatible with the old conflict system somehow.
I think that it is wrong to add a hack based on having a similar hack
somewhere, as well as expect this to be the "very last hack" that we
add, because things like this just happen to pile up.
The actual problem here is that we bend the API based on the immediate
needs of the command-line client, and that can cause problems in the
longer run. It's the cl client that should be remapping various --accept
options to svn_client_conflict_option_id_t. And currently we have the
API guessing that, and silently doing something other than what it was
told to do.
Furthermore, I don't see any problems with inverting this (and I think
that the code actually becomes simpler, with all the mapping performed
in one place).
> Then try out your idea and show us your diff ;-)
> Stefan has already gone through several iterations of this patch,
> so I'd rather not ask him to revisit this again.
Here is the patch. Not sure that I fully understand the point about the
number of iterations, though ;)
Log message:
[[[
Remove two hacks from the conflict resolver API that were added to
allow handling --accept=mine-conflict | working for tree conflicts
and --accept=working for binary text conflicts.
This patch makes the command-line client fully responsible for choosing
the appropriate conflict option id, and keeps the various new APIs
such as svn_client_conflict_text_resolve_by_id() simple and doing
just what they were told to do.
* subversion/libsvn_client/conflicts.c
(svn_client_conflict_text_resolve_by_id,
svn_client_conflict_tree_resolve_by_id): Remove hacks from these
functions.
* subversion/svn/conflict-callbacks.c
(resolve_conflict_by_accept_option): Inline parts of this function that
handle svn_cl__accept_edit and svn_cl__accept_launch ...
(svn_cl__resolve_conflict): ...here. Accept svn_cl__accept_t by value,
drop the option id argument. Pick the appropriate option id based on
the passed-in svn_cl__accept_t argument. Prompt the user if there is
no option or if the option did not apply.
* subversion/svn/resolve-cmd.c
(svn_cl__resolve): Handle unsupported --accept [--non-interactive] cases
in this function.
(svn_cl__walk_conflicts): Remove the is_resolve_cmd argument. Don't
map the --accept option to option id here, as we will do it in the
svn_cl__resolve_conflict() function. Adjust calls to walk_conflicts()
and svn_cl__resolve_conflict().
(walk_conflicts): Remove option_id argument, accept svn_cl__accept_t
by value.
(conflict_status_walker): Adjust call to svn_cl__resolve_conflict().
(conflict_status_walker_baton): Remove option_id field, store
accept_which field by value.
* subversion/svn/merge-cmd.c
(svn_cl__merge): Adjust call to svn_cl__walk_conflicts().
* subversion/svn/switch-cmd.c
(svn_cl__switch): Adjust call to svn_cl__walk_conflicts().
* subversion/svn/update-cmd.c
(svn_cl__update): Adjust call to svn_cl__walk_conflicts().
* subversion/svn/cl.h
(svn_cl__resolve_conflict, svn_cl__walk_conflicts): Update declarations.
]]]
Regards,
Evgeny Kotkov
Received on 2016-11-10 17:34:48 CET