Stefan Sperling <s...@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
Index: subversion/libsvn_client/conflicts.c
===================================================================
--- subversion/libsvn_client/conflicts.c        (revision 1768984)
+++ subversion/libsvn_client/conflicts.c        (working copy)
@@ -8933,7 +8933,6 @@ svn_client_conflict_text_resolve_by_id(
 {
   apr_array_header_t *resolution_options;
   svn_client_conflict_option_t *option;
-  const char *mime_type;
 
   SVN_ERR(svn_client_conflict_text_get_resolution_options(
             &resolution_options, conflict, ctx,
@@ -8940,17 +8939,6 @@ svn_client_conflict_text_resolve_by_id(
             scratch_pool, scratch_pool));
   option = svn_client_conflict_option_find_by_id(resolution_options,
                                                  option_id);
-
-  /* Support svn_client_conflict_option_merged_text for binary conflicts by
-   * mapping this option to svn_client_conflict_option_working_text. */
-  if (option == NULL && option_id == svn_client_conflict_option_merged_text)
-    {
-      mime_type = svn_client_conflict_text_get_mime_type(conflict);
-      if (mime_type && svn_mime_type_is_binary(mime_type))
-        option = svn_client_conflict_option_find_by_id(resolution_options,
-                   svn_client_conflict_option_working_text);
-    }
-
   if (option == NULL)
       return svn_error_createf(SVN_ERR_CLIENT_CONFLICT_OPTION_NOT_APPLICABLE,
                                NULL,
@@ -9048,51 +9036,6 @@ svn_client_conflict_tree_resolve_by_id(
   apr_array_header_t *resolution_options;
   svn_client_conflict_option_t *option;
 
-  /* Backwards compatibility hack: Upper layers may still try to resolve
-   * these two tree conflicts as 'mine-conflict' as Subversion 1.9 did.
-   * Fix up if necessary... */
-  if (option_id == svn_client_conflict_option_working_text_where_conflicted)
-    {
-      svn_wc_operation_t operation;
-
-      operation = svn_client_conflict_get_operation(conflict);
-      if (operation == svn_wc_operation_update ||
-          operation == svn_wc_operation_switch)
-        {
-          svn_wc_conflict_reason_t reason;
-
-          reason = svn_client_conflict_get_local_change(conflict);
-          if (reason == svn_wc_conflict_reason_moved_away)
-            {
-              /* Map 'mine-conflict' to 'update move destination'. */
-              option_id = svn_client_conflict_option_update_move_destination;
-            }
-          else if (reason == svn_wc_conflict_reason_deleted ||
-                   reason == svn_wc_conflict_reason_replaced)
-            {
-              svn_wc_conflict_action_t action;
-              svn_node_kind_t node_kind;
-
-              action = svn_client_conflict_get_incoming_change(conflict);
-              node_kind =
-                svn_client_conflict_tree_get_victim_node_kind(conflict);
-
-              if (action == svn_wc_conflict_action_edit &&
-                  node_kind == svn_node_dir)
-                {
-                  /* Map 'mine-conflict' to 'update any moved away children'. 
*/
-                  option_id =
-                    svn_client_conflict_option_update_any_moved_away_children;
-                }
-            }
-        }
-    }
-  else if (option_id == svn_client_conflict_option_merged_text)
-    {
-      /* Another backwards compatibility hack for 'choose merged'. */
-      option_id = svn_client_conflict_option_accept_current_wc_state;
-    }
-  
   SVN_ERR(svn_client_conflict_tree_get_resolution_options(
             &resolution_options, conflict, ctx,
             scratch_pool, scratch_pool));
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1768984)
+++ subversion/svn/cl.h (working copy)
@@ -385,17 +385,16 @@ svn_cl__print_conflict_stats(svn_cl__conflict_stat
  */
 svn_error_t *
 svn_cl__resolve_conflict(svn_boolean_t *resolved,
-                         svn_cl__accept_t *accept_which,
                          svn_boolean_t *quit,
                          svn_boolean_t *external_failed,
                          svn_boolean_t *printed_summary,
                          svn_client_conflict_t *conflict,
+                         svn_cl__accept_t accept_which,
                          const char *editor_cmd,
                          apr_hash_t *config,
                          const char *path_prefix,
                          svn_cmdline_prompt_baton_t *pb,
                          svn_cl__conflict_stats_t *conflict_stats,
-                         svn_client_conflict_option_id_t option_id,
                          svn_client_ctx_t *ctx,
                          apr_pool_t *scratch_pool);
 
@@ -406,7 +405,6 @@ svn_cl__resolve_conflict(svn_boolean_t *resolved,
 svn_error_t *
 svn_cl__walk_conflicts(apr_array_header_t *targets,
                        svn_cl__conflict_stats_t *conflict_stats,
-                       svn_boolean_t is_resolve_cmd,
                        svn_cl__opt_state_t *opt_state,
                        svn_client_ctx_t *ctx,
                        apr_pool_t *scratch_pool);
Index: subversion/svn/conflict-callbacks.c
===================================================================
--- subversion/svn/conflict-callbacks.c (revision 1768984)
+++ subversion/svn/conflict-callbacks.c (working copy)
@@ -1798,163 +1798,6 @@ handle_tree_conflict(svn_boolean_t *resolved,
 }
 
 static svn_error_t *
-resolve_conflict_by_accept_option(svn_client_conflict_option_id_t *option_id,
-                                  svn_cl__accept_t accept_which, 
-                                  svn_boolean_t *external_failed,
-                                  svn_client_conflict_t *conflict,
-                                  const char *editor_cmd,
-                                  apr_hash_t *config,
-                                  const char *path_prefix,
-                                  svn_cmdline_prompt_baton_t *pb,
-                                  svn_cl__conflict_stats_t *conflict_stats,
-                                  svn_client_ctx_t *ctx,
-                                  apr_pool_t *result_pool,
-                                  apr_pool_t *scratch_pool)
-{
-  svn_error_t *err;
-  const char *base_abspath = NULL;
-  const char *my_abspath = NULL;
-  const char *their_abspath = NULL;
-  const char *merged_abspath = svn_client_conflict_get_local_abspath(conflict);
-  svn_boolean_t text_conflicted;
-  apr_array_header_t *props_conflicted;
-  svn_boolean_t tree_conflicted;
-
-  SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
-                                             &props_conflicted,
-                                             &tree_conflicted,
-                                             conflict,
-                                             scratch_pool,
-                                             scratch_pool));
-
-  if (text_conflicted)
-    SVN_ERR(svn_client_conflict_text_get_contents(NULL, &my_abspath,
-                                                  &base_abspath,
-                                                  &their_abspath,
-                                                  conflict, scratch_pool,
-                                                  scratch_pool));
-
-  *option_id = svn_client_conflict_option_unspecified;
-  /* Handle the --accept option. */ 
-  switch (accept_which)
-    {
-    case svn_cl__accept_invalid:
-    case svn_cl__accept_unspecified:
-      /* No (or no valid) --accept option, fall through to prompting. */
-      break;
-    case svn_cl__accept_postpone:
-      *option_id = svn_client_conflict_option_postpone;
-      break;
-    case svn_cl__accept_base:
-      *option_id = svn_client_conflict_option_base_text;
-      break;
-    case svn_cl__accept_working:
-      *option_id = svn_client_conflict_option_merged_text;
-      break;
-    case svn_cl__accept_mine_conflict:
-      *option_id = svn_client_conflict_option_working_text_where_conflicted;
-      break;
-    case svn_cl__accept_theirs_conflict:
-      *option_id = svn_client_conflict_option_incoming_text_where_conflicted;
-      break;
-    case svn_cl__accept_mine_full:
-      *option_id = svn_client_conflict_option_working_text;
-      break;
-    case svn_cl__accept_theirs_full:
-      *option_id = svn_client_conflict_option_incoming_text;
-      break;
-    case svn_cl__accept_edit:
-      if (merged_abspath)
-        {
-          if (*external_failed)
-            {
-              *option_id = svn_client_conflict_option_postpone;
-              break;
-            }
-
-          err = svn_cmdline__edit_file_externally(merged_abspath,
-                                                  editor_cmd, config,
-                                                  scratch_pool);
-          if (err && (err->apr_err == SVN_ERR_CL_NO_EXTERNAL_EDITOR ||
-                      err->apr_err == SVN_ERR_EXTERNAL_PROGRAM))
-            {
-              char buf[1024];
-              const char *message;
-
-              message = svn_err_best_message(err, buf, sizeof(buf));
-              SVN_ERR(svn_cmdline_fprintf(stderr, scratch_pool, "%s\n",
-                                          message));
-              svn_error_clear(err);
-              *external_failed = TRUE;
-            }
-          else if (err)
-            return svn_error_trace(err);
-          *option_id = svn_client_conflict_option_merged_text;
-          break;
-        }
-      /* else, fall through to prompting. */
-      break;
-    case svn_cl__accept_launch:
-      if (base_abspath && their_abspath && my_abspath && merged_abspath)
-        {
-          svn_boolean_t remains_in_conflict;
-          const char *local_abspath;
-
-          if (*external_failed)
-            {
-              *option_id = svn_client_conflict_option_postpone;
-              break;
-            }
-
-          local_abspath = svn_client_conflict_get_local_abspath(conflict);
-          err = svn_cl__merge_file_externally(base_abspath,
-                                              their_abspath,
-                                              my_abspath,
-                                              merged_abspath,
-                                              local_abspath,
-                                              config,
-                                              &remains_in_conflict,
-                                              scratch_pool);
-          if (err && (err->apr_err == SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL ||
-                      err->apr_err == SVN_ERR_EXTERNAL_PROGRAM))
-            {
-              char buf[1024];
-              const char *message;
-
-              message = svn_err_best_message(err, buf, sizeof(buf));
-              SVN_ERR(svn_cmdline_fprintf(stderr, scratch_pool, "%s\n",
-                                          message));
-              *external_failed = TRUE;
-              return svn_error_trace(err);
-            }
-          else if (err)
-            return svn_error_trace(err);
-
-          if (remains_in_conflict)
-            *option_id = svn_client_conflict_option_postpone;
-          else
-            *option_id = svn_client_conflict_option_merged_text;
-          break;
-        }
-      /* else, fall through to prompting. */
-      break;
-    }
-
-  if (*option_id != svn_client_conflict_option_unspecified &&
-      *option_id != svn_client_conflict_option_postpone)
-    {
-      SVN_ERR(mark_conflict_resolved(conflict, *option_id,
-                                     text_conflicted,
-                                     props_conflicted->nelts >  0 ? "" : NULL,
-                                     tree_conflicted,
-                                     path_prefix, conflict_stats,
-                                     ctx, scratch_pool));
-    }
-
-  return SVN_NO_ERROR;
-}
-
-static svn_error_t *
 resolve_conflict_interactively(svn_boolean_t *resolved,
                                svn_boolean_t *postponed,
                                svn_boolean_t *quit,
@@ -2013,17 +1856,16 @@ resolve_conflict_interactively(svn_boolean_t *reso
 
 svn_error_t *
 svn_cl__resolve_conflict(svn_boolean_t *resolved,
-                         svn_cl__accept_t *accept_which,
                          svn_boolean_t *quit,
                          svn_boolean_t *external_failed,
                          svn_boolean_t *printed_summary,
                          svn_client_conflict_t *conflict,
+                         svn_cl__accept_t accept_which,
                          const char *editor_cmd,
                          apr_hash_t *config,
                          const char *path_prefix,
                          svn_cmdline_prompt_baton_t *pb,
                          svn_cl__conflict_stats_t *conflict_stats,
-                         svn_client_conflict_option_id_t option_id,
                          svn_client_ctx_t *ctx,
                          apr_pool_t *scratch_pool)
 {
@@ -2030,6 +1872,8 @@ svn_cl__resolve_conflict(svn_boolean_t *resolved,
   svn_boolean_t text_conflicted;
   apr_array_header_t *props_conflicted;
   svn_boolean_t tree_conflicted;
+  const char *local_abspath;
+  svn_client_conflict_option_id_t option_id;
 
   SVN_ERR(svn_client_conflict_get_conflicted(&text_conflicted,
                                              &props_conflicted,
@@ -2037,60 +1881,226 @@ svn_cl__resolve_conflict(svn_boolean_t *resolved,
                                              conflict,
                                              scratch_pool,
                                              scratch_pool));
+  local_abspath = svn_client_conflict_get_local_abspath(conflict);
 
-  /* Resolve the conflict by --accept option or interactively if no
-   * resolution option was passed. */
-  if (option_id == svn_client_conflict_option_unspecified)
+  if (accept_which == svn_cl__accept_unspecified)
     {
-      SVN_ERR(resolve_conflict_by_accept_option(&option_id, *accept_which,
-                                                external_failed, conflict,
-                                                editor_cmd, config,
-                                                path_prefix, pb,
-                                                conflict_stats, ctx,
-                                                scratch_pool, scratch_pool));
+      option_id = svn_client_conflict_option_unspecified;
+    }
+  else if (accept_which == svn_cl__accept_postpone)
+    {
+      option_id = svn_client_conflict_option_postpone;
+    }
+  else if (accept_which == svn_cl__accept_base)
+    {
+      option_id = svn_client_conflict_option_base_text;
+    }
+  else if (accept_which == svn_cl__accept_working)
+    {
+      option_id = svn_client_conflict_option_merged_text;
 
-      if (option_id == svn_client_conflict_option_unspecified)
+      if (text_conflicted)
         {
-          svn_boolean_t postponed = FALSE;
-          svn_boolean_t printed_description = FALSE;
-          svn_error_t *err;
+          const char *mime_type =
+            svn_client_conflict_text_get_mime_type(conflict);
 
-          *quit = FALSE;
+          /* There is no merged text for binary conflicts, behave as
+           * if 'mine-full' was chosen. */
+          if (mime_type && svn_mime_type_is_binary(mime_type))
+            option_id = svn_client_conflict_option_working_text;
+        }
+      else if (tree_conflicted)
+        {
+          /* For tree conflicts, map 'working' to 'accept current working
+           * copy state'. */
+          option_id = svn_client_conflict_option_accept_current_wc_state;
+        }
+    }
+  else if (accept_which == svn_cl__accept_theirs_conflict)
+    {
+      option_id = svn_client_conflict_option_incoming_text_where_conflicted;
+    }
+  else if (accept_which == svn_cl__accept_mine_conflict)
+    {
+      option_id = svn_client_conflict_option_working_text_where_conflicted;
 
-          /* We're in interactive mode and either the user gave no --accept
-             option or the option did not apply; let's prompt. */
-          while (!*resolved && !postponed && !*quit)
+      if (tree_conflicted)
+        {
+          svn_wc_operation_t operation;
+
+          operation = svn_client_conflict_get_operation(conflict);
+          if (operation == svn_wc_operation_update ||
+              operation == svn_wc_operation_switch)
             {
-              err = resolve_conflict_interactively(resolved, &postponed, quit,
-                                                   external_failed,
-                                                   printed_summary,
-                                                   &printed_description,
-                                                   conflict,
-                                                   editor_cmd, config,
-                                                   path_prefix, pb,
-                                                   conflict_stats, ctx,
-                                                   scratch_pool, scratch_pool);
-              if (err && err->apr_err == SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE)
+              svn_wc_conflict_reason_t reason;
+
+              reason = svn_client_conflict_get_local_change(conflict);
+              if (reason == svn_wc_conflict_reason_moved_away)
                 {
-                  /* Conflict resolution has failed. Let the user try again.
-                   * It is always possible to break out of this loop with
-                   * the 'quit' or 'postpone' options. */
-                  svn_handle_warning2(stderr, err, "svn: ");
+                  /* Map 'mine-conflict' to 'update move destination'. */
+                  option_id =
+                    svn_client_conflict_option_update_move_destination;
+                }
+              else if (reason == svn_wc_conflict_reason_deleted ||
+                       reason == svn_wc_conflict_reason_replaced)
+                {
+                  svn_wc_conflict_action_t action;
+                  svn_node_kind_t node_kind;
+
+                  action = svn_client_conflict_get_incoming_change(conflict);
+                  node_kind =
+                    svn_client_conflict_tree_get_victim_node_kind(conflict);
+
+                  if (action == svn_wc_conflict_action_edit &&
+                      node_kind == svn_node_dir)
+                    {
+                      /* Map 'mine-conflict' to 'update any moved away
+                       * children'. */
+                      option_id =
+                        
svn_client_conflict_option_update_any_moved_away_children;
+                    }
+                }
+            }
+        }
+    }
+  else if (accept_which == svn_cl__accept_theirs_full)
+    {
+      option_id = svn_client_conflict_option_incoming_text;
+    }
+  else if (accept_which == svn_cl__accept_mine_full)
+    {
+      option_id = svn_client_conflict_option_working_text;
+    }
+  else if (accept_which == svn_cl__accept_edit)
+    {
+      option_id = svn_client_conflict_option_unspecified;
+
+      if (local_abspath)
+        {
+          if (*external_failed)
+            {
+              option_id = svn_client_conflict_option_postpone;
+            }
+          else
+            {
+              svn_error_t *err;
+
+              err = svn_cmdline__edit_file_externally(local_abspath,
+                                                      editor_cmd, config,
+                                                      scratch_pool);
+              if (err && (err->apr_err == SVN_ERR_CL_NO_EXTERNAL_EDITOR ||
+                          err->apr_err == SVN_ERR_EXTERNAL_PROGRAM))
+                {
+                  char buf[1024];
+                  const char *message;
+
+                  message = svn_err_best_message(err, buf, sizeof(buf));
+                  SVN_ERR(svn_cmdline_fprintf(stderr, scratch_pool, "%s\n",
+                                              message));
                   svn_error_clear(err);
-                  err = SVN_NO_ERROR;
+                  *external_failed = TRUE;
                 }
-              SVN_ERR(err);
+              else if (err)
+                return svn_error_trace(err);
+              option_id = svn_client_conflict_option_merged_text;
             }
         }
+    }
+  else if (accept_which == svn_cl__accept_launch)
+    {
+      const char *base_abspath = NULL;
+      const char *my_abspath = NULL;
+      const char *their_abspath = NULL;
 
-      return SVN_NO_ERROR;
+      option_id = svn_client_conflict_option_unspecified;
+
+      if (text_conflicted)
+        SVN_ERR(svn_client_conflict_text_get_contents(NULL, &my_abspath,
+                                                      &base_abspath,
+                                                      &their_abspath,
+                                                      conflict, scratch_pool,
+                                                      scratch_pool));
+
+      if (base_abspath && their_abspath && my_abspath && local_abspath)
+        {
+          if (*external_failed)
+            {
+              option_id = svn_client_conflict_option_postpone;
+            }
+          else
+            {
+              svn_boolean_t remains_in_conflict;
+              svn_error_t *err;
+
+              err = svn_cl__merge_file_externally(base_abspath, their_abspath,
+                                                  my_abspath, local_abspath,
+                                                  local_abspath, config,
+                                                  &remains_in_conflict,
+                                                  scratch_pool);
+              if (err && (err->apr_err == SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL ||
+                          err->apr_err == SVN_ERR_EXTERNAL_PROGRAM))
+                {
+                  char buf[1024];
+                  const char *message;
+
+                  message = svn_err_best_message(err, buf, sizeof(buf));
+                  SVN_ERR(svn_cmdline_fprintf(stderr, scratch_pool, "%s\n",
+                                              message));
+                  *external_failed = TRUE;
+                  return svn_error_trace(err);
+                }
+              else if (err)
+                return svn_error_trace(err);
+
+              if (remains_in_conflict)
+                option_id = svn_client_conflict_option_postpone;
+              else
+                option_id = svn_client_conflict_option_merged_text;
+            }
+        }
     }
+  else
+    SVN_ERR_MALFUNCTION();
 
-  /* Non-interactive resolution. */
-  SVN_ERR_ASSERT(option_id != svn_client_conflict_option_unspecified);
+  /* If we are in interactive mode and either the user gave no --accept
+   * option or the option did not apply, then prompt. */
+  if (option_id == svn_client_conflict_option_unspecified)
+    {
+      svn_boolean_t postponed = FALSE;
+      svn_boolean_t printed_description = FALSE;
+      svn_error_t *err;
 
-  if (option_id != svn_client_conflict_option_postpone)
+      *quit = FALSE;
+
+      while (!*resolved && !postponed && !*quit)
+        {
+          err = resolve_conflict_interactively(resolved, &postponed, quit,
+                                               external_failed,
+                                               printed_summary,
+                                               &printed_description,
+                                               conflict,
+                                               editor_cmd, config,
+                                               path_prefix, pb,
+                                               conflict_stats, ctx,
+                                               scratch_pool, scratch_pool);
+          if (err && err->apr_err == SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE)
+            {
+              /* Conflict resolution has failed. Let the user try again.
+               * It is always possible to break out of this loop with
+               * the 'quit' or 'postpone' options. */
+              svn_handle_warning2(stderr, err, "svn: ");
+              svn_error_clear(err);
+              err = SVN_NO_ERROR;
+            }
+          SVN_ERR(err);
+        }
+    }
+  else if (option_id == svn_client_conflict_option_postpone)
     {
+      *resolved = FALSE;
+    }
+  else
+    {
       SVN_ERR(mark_conflict_resolved(conflict, option_id,
                                      text_conflicted,
                                      props_conflicted->nelts > 0 ? "" : NULL,
@@ -2099,10 +2109,6 @@ svn_cl__resolve_conflict(svn_boolean_t *resolved,
                                      ctx, scratch_pool));
       *resolved = TRUE;
     }
-  else
-    {
-      *resolved = FALSE;
-    }
 
   return SVN_NO_ERROR;
 }
Index: subversion/svn/merge-cmd.c
===================================================================
--- subversion/svn/merge-cmd.c  (revision 1768984)
+++ subversion/svn/merge-cmd.c  (working copy)
@@ -547,7 +547,7 @@ svn_cl__merge(apr_getopt_t *os,
   SVN_ERR(svn_cl__conflict_stats_get_paths(&conflicted_paths, conflict_stats,
                                            pool, pool));
   if (conflicted_paths)
-    SVN_ERR(svn_cl__walk_conflicts(conflicted_paths, conflict_stats, FALSE,
+    SVN_ERR(svn_cl__walk_conflicts(conflicted_paths, conflict_stats,
                                    opt_state, ctx, pool));
 
   if (!opt_state->quiet)
Index: subversion/svn/resolve-cmd.c
===================================================================
--- subversion/svn/resolve-cmd.c        (revision 1768984)
+++ subversion/svn/resolve-cmd.c        (working copy)
@@ -43,12 +43,11 @@
 struct conflict_status_walker_baton
 {
   svn_client_ctx_t *ctx;
-  svn_client_conflict_option_id_t option_id;
   svn_wc_notify_func2_t notify_func;
   void *notify_baton;
   svn_boolean_t resolved_one;
   apr_hash_t *resolve_later;
-  svn_cl__accept_t *accept_which;
+  svn_cl__accept_t accept_which;
   svn_boolean_t *quit;
   svn_boolean_t *external_failed;
   svn_boolean_t *printed_summary;
@@ -141,14 +140,12 @@ conflict_status_walker(void *baton,
                                   iterpool, iterpool));
   SVN_ERR(svn_client_conflict_get_conflicted(NULL, NULL, &tree_conflicted,
                                              conflict, iterpool, iterpool));
-  err = svn_cl__resolve_conflict(&resolved, cswb->accept_which,
-                                 cswb->quit, cswb->external_failed,
-                                 cswb->printed_summary,
-                                 conflict, cswb->editor_cmd,
+  err = svn_cl__resolve_conflict(&resolved, cswb->quit, cswb->external_failed,
+                                 cswb->printed_summary, conflict,
+                                 cswb->accept_which, cswb->editor_cmd,
                                  cswb->config, cswb->path_prefix,
                                  cswb->pb, cswb->conflict_stats,
-                                 cswb->option_id, cswb->ctx,
-                                 scratch_pool);
+                                 cswb->ctx, scratch_pool);
   if (err)
     {
       if (tree_conflicted)
@@ -175,8 +172,7 @@ static svn_error_t *
 walk_conflicts(svn_client_ctx_t *ctx,
                const char *local_abspath,
                svn_depth_t depth,
-               svn_client_conflict_option_id_t option_id,
-               svn_cl__accept_t *accept_which,
+               svn_cl__accept_t accept_which,
                svn_boolean_t *quit,
                svn_boolean_t *external_failed,
                svn_boolean_t *printed_summary,
@@ -195,7 +191,6 @@ walk_conflicts(svn_client_ctx_t *ctx,
     depth = svn_depth_infinity;
 
   cswb.ctx = ctx;
-  cswb.option_id = option_id;
 
   cswb.resolved_one = FALSE;
   cswb.resolve_later = (depth != svn_depth_empty)
@@ -339,7 +334,6 @@ walk_conflicts(svn_client_ctx_t *ctx,
 svn_error_t *
 svn_cl__walk_conflicts(apr_array_header_t *targets,
                        svn_cl__conflict_stats_t *conflict_stats,
-                       svn_boolean_t is_resolve_cmd,
                        svn_cl__opt_state_t *opt_state,
                        svn_client_ctx_t *ctx,
                        apr_pool_t *scratch_pool)
@@ -348,7 +342,6 @@ svn_cl__walk_conflicts(apr_array_header_t *targets
   svn_boolean_t quit = FALSE;
   svn_boolean_t external_failed = FALSE;
   svn_boolean_t printed_summary = FALSE;
-  svn_client_conflict_option_id_t option_id;
   svn_cmdline_prompt_baton_t *pb = apr_palloc(scratch_pool, sizeof(*pb));
   const char *path_prefix;
   svn_error_t *err;
@@ -360,51 +353,6 @@ svn_cl__walk_conflicts(apr_array_header_t *targets
   pb->cancel_func = ctx->cancel_func;
   pb->cancel_baton = ctx->cancel_baton;
 
-  switch (opt_state->accept_which)
-    {
-    case svn_cl__accept_working:
-      option_id = svn_client_conflict_option_merged_text;
-      break;
-    case svn_cl__accept_base:
-      option_id = svn_client_conflict_option_base_text;
-      break;
-    case svn_cl__accept_theirs_conflict:
-      option_id = svn_client_conflict_option_incoming_text_where_conflicted;
-      break;
-    case svn_cl__accept_mine_conflict:
-      option_id = svn_client_conflict_option_working_text_where_conflicted;
-      break;
-    case svn_cl__accept_theirs_full:
-      option_id = svn_client_conflict_option_incoming_text;
-      break;
-    case svn_cl__accept_mine_full:
-      option_id = svn_client_conflict_option_working_text;
-      break;
-    case svn_cl__accept_unspecified:
-      if (is_resolve_cmd && opt_state->non_interactive)
-        return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
-                                _("missing --accept option"));
-      option_id = svn_client_conflict_option_unspecified;
-      break;
-    case svn_cl__accept_postpone:
-      if (is_resolve_cmd)
-        return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
-                                _("invalid 'accept' ARG"));
-      option_id = svn_client_conflict_option_postpone;
-      break;
-    case svn_cl__accept_edit:
-    case svn_cl__accept_launch:
-      if (is_resolve_cmd)
-        return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
-                                _("invalid 'accept' ARG"));
-      option_id = svn_client_conflict_option_unspecified;
-      break;
-    default:
-      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
-                              _("invalid 'accept' ARG"));
-    }
-
-
   iterpool = svn_pool_create(scratch_pool);
   for (i = 0; i < targets->nelts; i++)
     {
@@ -425,19 +373,18 @@ svn_cl__walk_conflicts(apr_array_header_t *targets
           SVN_ERR(svn_client_conflict_get(&conflict, local_abspath, ctx,
                                           iterpool, iterpool));
           err = svn_cl__resolve_conflict(&resolved,
-                                         &opt_state->accept_which,
                                          &quit, &external_failed,
                                          &printed_summary,
-                                         conflict, opt_state->editor_cmd,
+                                         conflict, opt_state->accept_which,
+                                         opt_state->editor_cmd,
                                          ctx->config, path_prefix,
                                          pb, conflict_stats,
-                                         option_id, ctx,
-                                         iterpool);
+                                         ctx, iterpool);
         }
       else
         {
           err = walk_conflicts(ctx, local_abspath, opt_state->depth,
-                               option_id, &opt_state->accept_which,
+                               opt_state->accept_which,
                                &quit, &external_failed, &printed_summary,
                                opt_state->editor_cmd, ctx->config,
                                path_prefix, pb, conflict_stats, iterpool);
@@ -490,7 +437,21 @@ svn_cl__resolve(apr_getopt_t *os,
 
   SVN_ERR(svn_cl__check_targets_are_local_paths(targets));
 
-  SVN_ERR(svn_cl__walk_conflicts(targets, conflict_stats, TRUE,
+  if (opt_state->accept_which == svn_cl__accept_unspecified &&
+      opt_state->non_interactive)
+    {
+      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                              _("missing --accept option"));
+    }
+  else if (opt_state->accept_which == svn_cl__accept_postpone ||
+           opt_state->accept_which == svn_cl__accept_edit ||
+           opt_state->accept_which == svn_cl__accept_launch)
+    {
+      return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                              _("invalid 'accept' ARG"));
+    }
+
+  SVN_ERR(svn_cl__walk_conflicts(targets, conflict_stats,
                                  opt_state, ctx, scratch_pool));
 
   return SVN_NO_ERROR;
Index: subversion/svn/switch-cmd.c
===================================================================
--- subversion/svn/switch-cmd.c (revision 1768984)
+++ subversion/svn/switch-cmd.c (working copy)
@@ -195,7 +195,7 @@ svn_cl__switch(apr_getopt_t *os,
   SVN_ERR(svn_cl__conflict_stats_get_paths(&conflicted_paths, conflict_stats,
                                            scratch_pool, scratch_pool));
   if (conflicted_paths)
-    SVN_ERR(svn_cl__walk_conflicts(conflicted_paths, conflict_stats, FALSE,
+    SVN_ERR(svn_cl__walk_conflicts(conflicted_paths, conflict_stats,
                                    opt_state, ctx, scratch_pool));
 
   if (! opt_state->quiet)
Index: subversion/svn/update-cmd.c
===================================================================
--- subversion/svn/update-cmd.c (revision 1768984)
+++ subversion/svn/update-cmd.c (working copy)
@@ -185,7 +185,7 @@ svn_cl__update(apr_getopt_t *os,
   SVN_ERR(svn_cl__conflict_stats_get_paths(&conflicted_paths, conflict_stats,
                                            scratch_pool, scratch_pool));
   if (conflicted_paths)
-    SVN_ERR(svn_cl__walk_conflicts(conflicted_paths, conflict_stats, FALSE,
+    SVN_ERR(svn_cl__walk_conflicts(conflicted_paths, conflict_stats,
                                    opt_state, ctx, scratch_pool));
 
   if (! opt_state->quiet)

Reply via email to