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

Re: svn commit: r25670 - in trunk/subversion: include libsvn_subr libsvn_wc svn tests/cmdline/svntest

From: David Glasser <glasser_at_mit.edu>
Date: 2007-07-06 22:18:48 CEST

On 7/6/07, sussman@tigris.org <sussman@tigris.org> wrote:
> Author: sussman
> Date: Fri Jul 6 11:48:53 2007
> New Revision: 25670
>
> Log:
> Make text-conflicts prompt the user during updates and merges.
>
> This new feature can be avoided with --non-interactive, or disabled
> permanently by setting '[miscellany] interactive-conflicts = no' in
> your run-time config file.

This rocks!

> +/** The final result returned by the conflict callback. If the
> + * callback wholly resolves the conflict by itself, it would return @c
> + * svn_wc_conflict_result_resolved. If the conflict still persists,
> + * then return @c svn_wc_conflict_result_conflicted. In the case of
> + * file conflicts, the callback may instead signal that the user
> + * wishes to resolve the conflict by "choosing" one of the four
> + * fulltext files.
> + *
> + * @since New in 1.5.
> + */
> +typedef enum svn_wc_conflict_result_t
> +{
> + svn_wc_conflict_result_conflicted, /* user did nothing; conflict remains */
> + svn_wc_conflict_result_resolved, /* user has resolved the conflict */
> +
> + /* The following results only apply to file-conflicts. Note that
> + they're all specific ways of saying that the conflict is
> + resolved, in the sense that the user has chosen one of the four
> + files. The caller of the conflict-callback is responsible for
> + "installing" the chosen file as the final version of the file.*/
> +
> + svn_wc_conflict_result_choose_base, /* user chooses the base file */
> + svn_wc_conflict_result_choose_repos, /* user chooses the repository file */
> + svn_wc_conflict_result_choose_user, /* user chooses own version of file */
> + svn_wc_conflict_result_choose_merged /* user chooses the merged-file
> + (which she may have
> + manually edited) */
> +} svn_wc_conflict_result_t;

I'm really not sure how svn_wc_conflict_result_resolved differs from
svn_wc_conflict_result_choose_merged here.

> Modified: trunk/subversion/libsvn_subr/config_file.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/config_file.c?pathrev=25670&r1=25669&r2=25670
> ==============================================================================
> --- trunk/subversion/libsvn_subr/config_file.c (original)
> +++ trunk/subversion/libsvn_subr/config_file.c Fri Jul 6 11:48:53 2007
> @@ -986,6 +986,9 @@
> "### for 'svn add' and 'svn import', it defaults to 'no'." NL
> "### Automatic properties are defined in the section 'auto-props'." NL
> "# enable-auto-props = yes" NL
> + "### Set interactive-conflicts to 'no' to disable interactive" NL
> + "### confict resolution prompting. It defaults to 'yes'." NL
> + "# interative-conflicts = no" NL

Typo, missing a 'c' in the example.

> Modified: trunk/subversion/svn/conflict-callbacks.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/conflict-callbacks.c?pathrev=25670&r1=25669&r2=25670
> ==============================================================================
> --- trunk/subversion/svn/conflict-callbacks.c (original)
> +++ trunk/subversion/svn/conflict-callbacks.c Fri Jul 6 11:48:53 2007
> @@ -29,6 +29,7 @@
>
> #include "svn_cmdline.h"
> #include "svn_client.h"
> +#include "svn_pools.h"
> #include "cl.h"
>
> #include "svn_private_config.h"
> @@ -105,33 +106,168 @@
> if (desc->repos_file)
> SVN_ERR(svn_cmdline_printf(pool, _(" Repository's file: %s\n"),
> desc->repos_file));
> - if (desc->edited_file)
> + if (desc->user_file)
> SVN_ERR(svn_cmdline_printf(pool, _(" User's file: %s\n"),
> - desc->edited_file));
> - if (desc->conflict_file)
> + desc->user_file));
> + if (desc->merged_file)
> SVN_ERR(svn_cmdline_printf(pool, _(" File with conflict markers: %s\n"),
> - desc->conflict_file));
> + desc->merged_file));
>
> return SVN_NO_ERROR;
> }
>
>
> -
> +/* A conflict callback which does nothing; useful for debugging and/or
> + printing a description of the conflict. */
> svn_error_t *
> -svn_cl__ignore_conflicts(const svn_wc_conflict_description_t *description,
> +svn_cl__ignore_conflicts(svn_wc_conflict_result_t *result,
> + const svn_wc_conflict_description_t *description,
> void *baton,
> apr_pool_t *pool)
> {
> - /* This routine is still useful for debugging purposes; it makes for
> - a nice breakpoint where one can examine the conflict
> - description. Or, just uncomment this bit: */
> -
> - /*
> SVN_ERR(svn_cmdline_printf(pool, _("Discovered a conflict.\n\n")));
> SVN_ERR(print_conflict_description(description, pool));
> SVN_ERR(svn_cmdline_printf(pool, _("\n\n")));
> - */
>
> - return svn_error_create(SVN_ERR_CLIENT_CONFLICT_REMAINS, NULL,
> - _("Conflict was not resolved."));
> + *result = svn_wc_conflict_result_conflicted; /* conflict remains. */
> + return SVN_NO_ERROR;
> +}
> +
> +
> +/* A conflict callback which does real user prompting. */
> +svn_error_t *
> +svn_cl__interactive_conflict_handler(svn_wc_conflict_result_t *result,
> + const svn_wc_conflict_description_t *desc,
> + void *baton,
> + apr_pool_t *pool)
> +{
> + apr_pool_t *subpool = svn_pool_create(pool);
> +
> + /* For now, we only handle conflicting file contents. We can deal
> + with other sorts of conflicts someday. */
> + if ((desc->node_kind == svn_node_file)
> + && (desc->action == svn_wc_conflict_action_edit)
> + && (desc->reason == svn_wc_conflict_reason_edited))
> + {
> + const char *answer;
> + char *prompt;
> + svn_boolean_t performed_edit = FALSE;
> +
> + SVN_ERR(svn_cmdline_printf(subpool,
> + _("Conflict discovered in '%s'.\n"),
> + desc->path));
> + while (1)
> + {
> + char *colonprompt;
> +
> + svn_pool_clear(subpool);
> +
> + prompt = apr_pstrdup(subpool, _("Select: (p)ostpone"));
> + if (desc->merged_file)
> + prompt = apr_pstrcat(subpool, prompt, _(", (d)iff, (e)dit"),
> + NULL);
> + if (performed_edit)
> + prompt = apr_pstrcat(subpool, prompt, _(", (a)ccept"), NULL);
> + prompt = apr_pstrcat(subpool, prompt, _(", (h)elp : "), NULL);
> +
> + SVN_ERR(svn_cmdline_prompt_user(&answer, prompt, subpool));
> +
> + if (strcmp(answer, "h") == 0)
> + {
> + SVN_ERR(svn_cmdline_printf(subpool,
> + _(" (p)ostpone - mark the conflict to be resolved later\n"
> + " (d)iff - show all changes made to merged file\n"
> + " (e)dit - use an editor to resolve conflict\n"
> + " (a)ccept - use merged verison of file\n"
> + " (m)ine - use my version of file\n"
> + " (t)heirs - use repository's version of file\n"
> + " (l)aunch - use third-party tool to resolve conflict\n"
> + " (h)elp - show this list\n\n")));
> + }

* Allow '?' as well as 'h', maybe?

* At the risk of bikeshedding, (r)esolved instead of (a)ccept for
  consistency with 'svn resolved'

* Do the descriptions for 'mine' and 'theirs' make sense in the 'svn
  merge' context as well as the update/switch context? I'm not sure.

> + if (strcmp(answer, "p") == 0)
> + {
> + /* Do nothing, let file be marked conflicted. */
> + *result = svn_wc_conflict_result_conflicted;
> + break;
> + }
> + if (strcmp(answer, "m") == 0)
> + {
> + *result = svn_wc_conflict_result_choose_user;
> + break;
> + }
> + if (strcmp(answer, "t") == 0)
> + {
> + *result = svn_wc_conflict_result_choose_repos;
> + break;
> + }
> + if (strcmp(answer, "d") == 0)
> + {
> + if (desc->merged_file && desc->base_file)
> + {
> + svn_diff_t *diff;
> + svn_stream_t *output;
> + svn_diff_file_options_t *options =
> + svn_diff_file_options_create(subpool);
> + options->ignore_eol_style = TRUE;
> + SVN_ERR(svn_stream_for_stdout(&output, subpool));
> + SVN_ERR(svn_diff_file_diff_2(&diff, desc->base_file,
> + desc->merged_file,
> + options, subpool));
> + SVN_ERR(svn_diff_file_output_unified2(output, diff,
> + desc->base_file,
> + desc->merged_file,
> + NULL, NULL,
> + APR_LOCALE_CHARSET,
> + subpool));
> + performed_edit = TRUE;
> + }
> + else
> + SVN_ERR(svn_cmdline_printf(subpool, _("Invalid option.\n\n")));
> + }
> + if (strcmp(answer, "e") == 0)
> + {
> + if (desc->merged_file)
> + {
> + /* ### TODO: launch $EDITOR or $SVN_EDITOR here. */
> + SVN_ERR(svn_cmdline_printf(
> + subpool, _("Feature not yet implemented.\n\n")));
> + performed_edit = TRUE;
> + }
> + else
> + SVN_ERR(svn_cmdline_printf(subpool, _("Invalid option.\n\n")));
> + }
> + if (strcmp(answer, "l") == 0)
> + {
> + if (desc->base_file && desc->repos_file && desc->user_file)
> + {
> + /* ### TODO: launch $SVNMERGE tool here with 3 fulltexts. */
> + SVN_ERR(svn_cmdline_printf(
> + subpool, _("Feature not yet implemented.\n\n")));
> + performed_edit = TRUE;
> + }
> + else
> + SVN_ERR(svn_cmdline_printf(subpool, _("Invalid option.\n\n")));
> + }
> + if (strcmp(answer, "a") == 0)
> + {
> + /* We only allow the user accept the merged version of
> + the file if they've edited it, or at least looked at
> + the diff. */
> + if (performed_edit)
> + {
> + *result = svn_wc_conflict_result_choose_merged;
> + break;
> + }
> + else
> + SVN_ERR(svn_cmdline_printf(subpool, _("Invalid option.\n\n")));
> + }
> + }
> + }
> + else /* other types of conflicts */
> + {
> + *result = svn_wc_conflict_result_conflicted; /* conflict remains. */
> + }
> +
> + svn_pool_destroy(subpool);
> + return SVN_NO_ERROR;
> }
>
> Modified: trunk/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/main.c?pathrev=25670&r1=25669&r2=25670
> ==============================================================================
> --- trunk/subversion/svn/main.c (original)
> +++ trunk/subversion/svn/main.c Fri Jul 6 11:48:53 2007
> @@ -973,6 +973,7 @@
> svn_config_t *cfg;
> svn_boolean_t used_change_arg = FALSE;
> svn_boolean_t descend = TRUE;
> + svn_boolean_t interactive_conflicts = FALSE;
>
> /* Initialize the app. */
> if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
> @@ -1712,8 +1713,24 @@
> ctx->auth_baton = ab;
>
> /* Set up conflict resolution callback. */
> - ctx->conflict_func = svn_cl__ignore_conflicts;
> - ctx->conflict_baton = NULL;
> + if ((err = svn_config_get_bool(cfg, &interactive_conflicts,
> + SVN_CONFIG_SECTION_MISCELLANY,
> + SVN_CONFIG_OPTION_INTERACTIVE_CONFLICTS,
> + TRUE))) /* ### interactivity on by default.
> + we can change this. */
> + svn_handle_error2(err, stderr, TRUE, "svn: ");
> +
> + if (interactive_conflicts
> + && (! opt_state.non_interactive ))
> + {
> + ctx->conflict_func = svn_cl__interactive_conflict_handler;
> + ctx->conflict_baton = NULL;
> + }
> + else
> + {
> + ctx->conflict_func = NULL;
> + ctx->conflict_baton = NULL;
> + }
>
> /* And now we finally run the subcommand. */
> err = (*subcommand->cmd_func)(os, &command_baton, pool);
>
> Modified: trunk/subversion/tests/cmdline/svntest/main.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/svntest/main.py?pathrev=25670&r1=25669&r2=25670
> ==============================================================================
> --- trunk/subversion/tests/cmdline/svntest/main.py (original)
> +++ trunk/subversion/tests/cmdline/svntest/main.py Fri Jul 6 11:48:53 2007
> @@ -378,7 +378,11 @@
>
> # define default config file contents if none provided
> if config_contents is None:
> - config_contents = "#\n"
> + config_contents = """
> +#
> +[miscellany]
> +interactive-conflicts = false
> +"""
>
> # define default server file contents if none provided
> if server_contents is None:

You've got to watch out here: there are three random tests that call
create_config_dir with a config_contents file of their own, so this'll
get lost there (hopefully it's irrelevant). Perhaps we should just
append the default text to the provided one (I believe it's fine to
have duplicate sections in our format). (The same should probably be
done for the servers file, although no test actually customizes that.)

--dave

-- 
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jul 7 00:45:44 2007

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