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

Re: [PATCH] Implement (l)aunch an external merge tool

From: Ben Collins-Sussman <sussman_at_red-bean.com>
Date: 2007-09-27 22:37:38 CEST

On 9/19/07, Augie Fackler <durin42@gmail.com> wrote:

> 1) I use getwd() - is there some APR/Subversion function I can use
> for this? I couldn't find anything, and this seems to work just fine.

Unsure...?

> 2) It seems that there should be more ways to set the tool than
> $SVNMERGE - probably a config variable. What's the procedure for
> adding one? I'd love to go ahead and add that too.

Imitate what we do for the 'diff3-cmd' runtime config variable in
~/.subversion/config. You can't go wrong. :-)

> +/* Search for a merge tool command in environment variables,
> + and use it to perform the merge of the four given files.
> + Use POOL for all allocations. */
> +svn_error_t *
> +svn_cl__merge_file_externally(const char *base_path,
> + const char *repos_path,
> + const char *user_path,
> + const char *merged_path,
> + apr_pool_t *pool);

Please augment the docstring to explain what all four path args mean.

> Index: subversion/svn/util.c
> ===================================================================
> --- subversion/svn/util.c (revision 26684)
> +++ subversion/svn/util.c (working copy)
> @@ -27,6 +27,7 @@
> #include <string.h>
> #include <ctype.h>
> #include <assert.h>
> +#include <unistd.h>
>
> #include <apr_errno.h>
> #include <apr_strings.h>
> @@ -193,8 +194,49 @@
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_cl__merge_file_externally(const char *base_path,
> + const char *repos_path,
> + const char *user_path,
> + const char *merged_path,
> + apr_pool_t *pool)
> +{
> + const char *merge_tool;
> + merge_tool = getenv("SVNMERGE");
> + /* Error if there is no editor specified */

Comment above is wrong. :-)

> + if (merge_tool)
> + {
> + const char *c;
>
> + for (c = merge_tool; *c; c++)
> + if (!svn_ctype_isspace(*c))
> + break;
>
> + if (! *c)
> + return svn_error_create
> + (SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL, NULL,
> + _("The SVNMERGE environment variable is empty or "
> + "consists solely of whitespace. Expected a shell
> command.\n"));
> + }
> + else
> + return svn_error_create
> + (SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL, NULL,
> + _("The environment variable SVNMERGE is not set.\n"));
> +
> + /* command base left right destination */
> + const char *arguments[] = { merge_tool, base_path, repos_path,
> + user_path, merged_path, NULL};
> + int exitcode;
> + apr_exit_why_e exitwhy;
> + char *sys_path = getwd(NULL);
> + const char *path = svn_path_internal_style(sys_path, pool);

C++ is a lovely thing, but this is C99, I believe, and one isn't
allowed to declare variables right in the center of a code block. :-)

> + if (merge_err &&
> + merge_err->apr_err ==
> SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL)
> + {
> + SVN_ERR(svn_cmdline_printf(subpool, merge_err-
> >message ?
> + merge_err->message :
> + _("No merge tool
> found.\n")));

Extend the error message to be more helpful: tell the user to set
$SVNMERGE, for example.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 27 22:37:51 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.