Sorry for the delay.  This looks pretty nice.  And thanks too for a
very clear cover letter.

It should get a NEWS entry, a whatsnew entry and some description in
the user guide.  (whatsnew could be very similar to your cover
letter.)  Experience shows that if we don't document new features when
they are added, it is harder to come back and do them later.

Strictly speaking it seems this is redundant with editing the config
file, or with vila's 'config' command, but perhaps the configuration
here is complex enough that it's worth having a special option.

New Pythons have a "hello {name}" syntax
<http://docs.python.org/library/string.html#format-string-syntax> that
would be ideal for this: less error-prone than "%(name)s" but more
explicit and extensible than "%n".  Unfortunately that is only in 2.6
and later, but perhaps we could simulate it on old Pythons...?

On 17 October 2010 13:48, Gordon Tyler <email address hidden> wrote:
> Gordon Tyler has proposed merging lp:~doxxx/bzr/mergetools into lp:bzr.
>
> Requested reviews:
>  bzr-core (bzr-core)
> Related bugs:
>  #489915 [master] external diff/merge configuration needs serious rework
>  https://bugs.launchpad.net/bugs/489915
>
>
> External merge tool support in bzr and related GUI tools is not very well defined and there are currently two incompatible mechanisms for setting an external merge tool.
>
> This merge proposal seeks to improve this by defining a bzrlib.mergetools module which provides an API for managing a list of external merge tools, adding a 'bzr mergetools' command which allows the user to manage the list of mergetools, and adding a '--resolve-using' option to 'bzr merge' and 'bzr remerge' to invoke an external merge tool for text conflicts.
>
> There will be a corresponding merge proposal for qbzr to make use of 'bzrlib.mergetools' in qconfig and qconflicts.
>
> Merge tools are defined by their command-line. The name of a merge tool, for the purposes of display and reference in the 'bzr merge' and 'bzr remerge' commands, is derived from the basename of the executable, stripping '.exe' if on win32. In the future, this could be separated out into a distinct field to allow multiple definitions of a merge tool using the same executable but different arguments, which may be necessary for jbowtie's binary merge idea.
>
> The mergetools module also tracks a default merge tool that will be used by qconfig and qconflicts. The 'bzr mergetools' command doesn't provide a method for setting it since it is not necessary for any of the bzr commands that make use of external merge tools.
>
> The 'bzr mergetools' command provides the following operations:
>
> - add: Adds a new external merge tool
> - update: Updates an existing merge tool by name
> - remove: Removes an existing merge tool by name
> - detect: Automatically detects known merge tools on the PATH
> - list: Lists the external merge tools that have been defined
>
> --
> https://code.launchpad.net/~doxxx/bzr/mergetools/+merge/38663
> Your team bzr-core is requested to review the proposed merge of lp:~doxxx/bzr/mergetools into lp:bzr.
>
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py  2010-10-15 11:30:54 +0000
> +++ bzrlib/builtins.py  2010-10-17 17:48:50 +0000
> @@ -41,6 +41,7 @@
>     log,
>     merge as _mod_merge,
>     merge_directive,
> +    mergetools,
>     osutils,
>     reconfigure,
>     rename_map,
> @@ -198,6 +199,19 @@
>     return bzrdir.BzrDir.open_containing_tree_or_branch(filename)
>
>
> +def _resolve_using_merge_tool(tool_name, conflicts):
> +    merge_tool = mergetools.find_merge_tool(tool_name)
> +    if merge_tool is None:
> +        available = '\n  '.join([mt.get_name() for mt in
> +                                 mergetools.get_merge_tools()
> +                                 if mt.is_available()])
> +        raise errors.BzrCommandError('Unrecognized merge tool: %s\n\n'
> +                                     'Available merge tools:\n'
> +                                     '  %s' % (tool_name, available))
> +    for conflict in conflicts:
> +        merge_tool.invoke(conflict.path)
> +
> +
>  # TODO: Make sure no commands unconditionally use the working directory as a
>  # branch.  If a filename argument is used, the first of them should be used to
>  # specify the branch.  (Perhaps this can be factored out into some kind of
> @@ -3816,7 +3830,9 @@
>         Option('preview', help='Instead of merging, show a diff of the'
>                ' merge.'),
>         Option('interactive', help='Select changes interactively.',
> -            short_name='i')
> +            short_name='i'),
> +        Option('resolve-using', help='Invokes the external merge tool named '
> +               'ARG for each merged file with conflicts.', type=str),
>     ]
>
>     def run(self, location=None, revision=None, force=False,
> @@ -3825,6 +3841,7 @@
>             directory=None,
>             preview=False,
>             interactive=False,
> +            resolve_using=None
>             ):
>         if merge_type is None:
>             merge_type = _mod_merge.Merge3Merger
> @@ -3901,12 +3918,15 @@
>                 "This branch has no commits."
>                 " (perhaps you would prefer 'bzr pull')")
>         if preview:
> -            return self._do_preview(merger)
> +            retval = self._do_preview(merger)
>         elif interactive:
> -            return self._do_interactive(merger)
> +            retval = self._do_interactive(merger)
>         else:
> -            return self._do_merge(merger, change_reporter, allow_pending,
> -                                  verified)
> +            retval = self._do_merge(merger, change_reporter, allow_pending,
> +                                    verified)
> +        if retval != 0 and resolve_using is not None:
> +                _resolve_using_merge_tool(resolve_using, tree.conflicts())
> +        return retval
>
>     def _get_preview(self, merger):
>         tree_merger = merger.make_merger()
> @@ -4103,14 +4123,16 @@
>     """
>     takes_args = ['file*']
>     takes_options = [
> -            'merge-type',
> -            'reprocess',
> -            Option('show-base',
> -                   help="Show base revision text in conflicts."),
> -            ]
> +        'merge-type',
> +        'reprocess',
> +        Option('show-base',
> +               help="Show base revision text in conflicts."),
> +        Option('resolve-using', help='Invokes the external merge tool named '
> +               'ARG for each merged file with conflicts.', type=str),
> +    ]
>
>     def run(self, file_list=None, merge_type=None, show_base=False,
> -            reprocess=False):
> +            reprocess=False, resolve_using=None):
>         from bzrlib.conflicts import restore
>         if merge_type is None:
>             merge_type = _mod_merge.Merge3Merger
> @@ -4168,6 +4190,8 @@
>         finally:
>             tree.set_parent_ids(parents)
>         if conflicts > 0:
> +            if resolve_using is not None:
> +                _resolve_using_merge_tool(resolve_using, tree.conflicts())
>             return 1
>         else:
>             return 0
> @@ -6077,5 +6101,6 @@
>         ('cmd_conflicts', [], 'bzrlib.conflicts'),
>         ('cmd_sign_my_commits', [], 'bzrlib.sign_my_commits'),
>         ('cmd_test_script', [], 'bzrlib.tests.script'),
> +        ('cmd_mergetools', [], 'bzrlib.mergetools'),
>         ]:
>         builtin_command_registry.register_lazy(name, aliases, module_name)
>
> === modified file 'bzrlib/conflicts.py'
> --- bzrlib/conflicts.py 2010-07-16 15:35:43 +0000
> +++ bzrlib/conflicts.py 2010-10-17 17:48:50 +0000
> @@ -27,10 +27,12 @@
>     cleanup,
>     commands,
>     errors,
> +    mergetools,
>     osutils,
>     rio,
>     trace,
>     transform,
> +    ui,
>     workingtree,
>     )
>  """)
> @@ -115,9 +117,15 @@
>             'directory',
>             option.Option('all', help='Resolve all conflicts in this tree.'),
>             ResolveActionOption(),
> +            option.Option('using', help='Resolve conflicts using an '
> +                          'external merge tool.', type=str),
> +            'verbose',
>             ]
>     _see_also = ['conflicts']
> -    def run(self, file_list=None, all=False, action=None, directory=u'.'):
> +    def run(self, file_list=None, all=False, action=None, directory=u'.',
> +            using=None, verbose=False):
> +        if using is not None:
> +            action = 'tool'
>         if all:
>             if file_list:
>                 raise errors.BzrCommandError("If --all is specified,"
> @@ -156,6 +164,44 @@
>                 # refactoring to transfer tree.auto_resolve() to
>                 # conflict.auto(tree) --vila 091242
>                 pass
> +        elif action == 'tool':
> +            merge_tool = mergetools.find_merge_tool(using)
> +            if merge_tool is None:
> +                raise errors.BzrCommandError(
> +                    'Unrecognized merge tool: %s\n\n'
> +                    'Available merge tools:\n'
> +                    '  %s' % (using,
> +                            '\n  '.join([mt.get_name() for mt in
> +                                       mergetools.get_merge_tools()
> +                                       if mt.is_available()])))
> +            if merge_tool is not None and not merge_tool.is_available():
> +                raise errors.BzrCommandError(
> +                    'Merge tool is not available: %s' % merge_tool.get_name())
> +            if all:
> +                file_list = []
> +                for conflict in tree.conflicts():
> +                    file_list.append(conflict.path)
> +            if file_list is None:
> +                raise errors.BzrCommandError(
> +                    'Either FILE(s) or --all must be provided')
> +            resolved = 0
> +            for file in file_list:
> +                # to avoid unnecessary './' prefix on file names
> +                if directory != u'.':
> +                    file = os.path.join(directory, file)
> +                if verbose:
> +                    ui.ui_factory.show_message('Invoking %s on %s...' %
> +                                     (merge_tool.get_name(), file))
> +                retcode = merge_tool.invoke(file)
> +                if retcode == 0:
> +                    resolve(tree, [file])
> +                    resolved += 1
> +            ui.ui_factory.show_message('%d conflict(s) resolved.' % resolved)
> +            unresolved = tree.conflicts()
> +            if len(unresolved) > 0:
> +                ui.ui_factory.show_message('Remaining conflicts:')
> +                for conflict in unresolved:
> +                    ui.ui_factory.show_message(str(conflict))
>         else:
>             resolve(tree, file_list, action=action)
>
>
> === added file 'bzrlib/mergetools.py'
> --- bzrlib/mergetools.py        1970-01-01 00:00:00 +0000
> +++ bzrlib/mergetools.py        2010-10-17 17:48:50 +0000
> @@ -0,0 +1,404 @@
> +# Copyright (C) 2010 Canonical Ltd.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +
> +"""Registry for external merge tools, e.g. kdiff3, meld, etc."""
> +
> +import os
> +import shutil
> +import subprocess
> +import sys
> +import tempfile
> +
> +from bzrlib.lazy_import import lazy_import
> +lazy_import(globals(), """
> +from bzrlib import (
> +    cmdline,
> +    config,
> +    errors,
> +    trace,
> +    ui,
> +    workingtree,
> +)
> +""")
> +
> +from bzrlib.commands import Command
> +from bzrlib.option import Option
> +
> +
> +substitution_help = {
> +    '%b' : 'file.BASE',
> +    '%t' : 'file.THIS',
> +    '%o' : 'file.OTHER',
> +    '%r' : 'file (output)',
> +    '%T' : 'file.THIS (temp copy, used to overwrite "file" if merge succeeds)'
> +}
> +
> +
> +def subprocess_invoker(executable, args, cleanup):
> +    retcode = subprocess.call([executable] + args, shell=True)
> +    cleanup(retcode)
> +    return retcode
> +
> +
> +class MergeTool(object):
> +    @classmethod
> +    def from_executable_and_args(cls, executable, args):
> +        executable = _optional_quote_arg(executable)
> +        if not isinstance(args, str) and not isinstance(args, unicode):
> +            args = ' '.join([_optional_quote_arg(arg) for arg in args])
> +        return MergeTool(executable + ' ' + args)
> +
> +    def __init__(self, commandline):
> +        """commandline: Command line of merge tool, including executable and
> +                        args with filename substitution markers.
> +        """
> +        self._commandline = commandline
> +
> +    def __repr__(self):
> +        return '<MergeTool %r>' % self._commandline
> +
> +    def __str__(self):
> +        return self._commandline
> +
> +    def get_name(self):
> +        name = os.path.basename(self.get_executable())
> +        if sys.platform == 'win32' and name.lower().endswith('.exe'):
> +            name = name[:-4]
> +        return name
> +
> +    def get_commandline(self):
> +        return self._commandline
> +
> +    def get_executable(self):
> +        parts = cmdline.split(self._commandline)
> +        if len(parts) < 1:
> +            return ''
> +        return parts[0]
> +
> +    def get_arguments(self):
> +        parts = cmdline.split(self._commandline)
> +        if len(parts) < 2:
> +            return ''
> +        return ' '.join(parts[1:])
> +
> +    def set_executable(self, executable):
> +        split_cmdline = cmdline.split(self._commandline)
> +        split_cmdline[0] = _optional_quote_arg(executable)
> +        self._commandline = ' '.join(split_cmdline)
> +
> +    def set_arguments(self, args):
> +        if not isinstance(args, str) and not isinstance(args, unicode):
> +            args = ' '.join([_optional_quote_arg(arg) for arg in args])
> +        self._commandline = self.get_executable() + ' ' + args
> +
> +    def set_commandline(self, commandline):
> +        self._commandline = commandline
> +
> +    def is_available(self):
> +        executable = self.get_executable()
> +        return os.path.exists(executable) or _find_executable(executable)
> +
> +    def invoke(self, filename, invoker=None):
> +        if invoker is None:
> +            invoker = subprocess_invoker
> +        # TODO: find a cleaner way to expand into args
> +        commandline, tmp_file = self._expand_commandline(filename)
> +        args = cmdline.split(commandline)
> +        def cleanup(retcode):
> +            if tmp_file is not None:
> +                if retcode == 0: # on success, replace file with temp file
> +                    shutil.move(tmp_file, filename)
> +                else: # otherwise, delete temp file
> +                    os.remove(tmp_file)
> +        return invoker(args[0], args[1:], cleanup)
> +
> +    def _expand_commandline(self, filename):
> +        commandline = self._commandline
> +        tmp_file = None
> +        commandline = commandline.replace('%b', _optional_quote_arg(filename +
> +                                                                    '.BASE'))
> +        commandline = commandline.replace('%t', _optional_quote_arg(filename +
> +                                                                    '.THIS'))
> +        commandline = commandline.replace('%o', _optional_quote_arg(filename +
> +                                                                    '.OTHER'))
> +        commandline = commandline.replace('%r', _optional_quote_arg(filename))
> +        if '%T' in commandline:
> +            tmp_file = tempfile.mktemp("_bzr_mergetools_%s.THIS" %
> +                                       os.path.basename(filename))
> +            shutil.copy(filename + ".THIS", tmp_file)
> +            commandline = commandline.replace('%T',
> +                                              _optional_quote_arg(tmp_file))
> +        return commandline, tmp_file
> +
> +
> +_KNOWN_MERGE_TOOLS = (
> +    'bcompare %t %o %b %r',
> +    'kdiff3 %b %t %o -o %r',
> +    'xxdiff -m -O -M %r %t %b %o',
> +    'meld %b %T %o',
> +    'opendiff %t %o -ancestor %b -merge %r',
> +    'winmergeu %r',
> +)
> +
> +
> +def detect_merge_tools():
> +    tools = [MergeTool(commandline) for commandline in _KNOWN_MERGE_TOOLS]
> +    return [tool for tool in tools if tool.is_available()]
> +
> +
> +def get_merge_tools(conf=None):
> +    """Returns list of MergeTool objects."""
> +    if conf is None:
> +        conf = config.GlobalConfig()
> +    commandlines = conf.get_user_option_as_list('mergetools')
> +    if commandlines is None:
> +        return []
> +    return [MergeTool(commandline) for commandline in commandlines]
> +
> +
> +def set_merge_tools(merge_tools, conf=None):
> +    if conf is None:
> +        conf = config.GlobalConfig()
> +    conf_value = sorted(set(merge_tool.get_commandline()
> +                           for merge_tool in merge_tools
> +                           if len(merge_tool.get_commandline()) > 0))
> +    conf.set_user_option("mergetools", conf_value)
> +
> +
> +def find_merge_tool(name, conf=None):
> +    if conf is None:
> +        conf = config.GlobalConfig()
> +    merge_tools = get_merge_tools(conf)
> +    for merge_tool in merge_tools:
> +        if merge_tool.get_name() == name:
> +            return merge_tool
> +    return None
> +
> +
> +def find_first_available_merge_tool(conf=None):
> +    if conf is None:
> +        conf = config.GlobalConfig()
> +    merge_tools = get_merge_tools(conf)
> +    for merge_tool in merge_tools:
> +        if merge_tool.is_available():
> +            return merge_tool
> +    return None
> +
> +
> +def get_default_merge_tool(conf=None):
> +    if conf is None:
> +        conf = config.GlobalConfig()
> +    name = conf.get_user_option('default_mergetool')
> +    if name is None:
> +        trace.mutter('no default merge tool defined')
> +        return None
> +    merge_tool = find_merge_tool(name, conf)
> +    trace.mutter('found default merge tool: %r', merge_tool)
> +    return merge_tool
> +
> +
> +def set_default_merge_tool(name, conf=None):
> +    if conf is None:
> +        conf = config.GlobalConfig()
> +    if isinstance(name, MergeTool):
> +        name = name.get_name()
> +    if find_merge_tool(name, conf) is None:
> +        raise errors.BzrError('invalid merge tool name: %r' % name)
> +    trace.mutter('setting default merge tool: %s', name)
> +    conf.set_user_option('default_mergetool', name)
> +
> +
> +def _optional_quote_arg(arg):
> +    if ' ' in arg and not _is_arg_quoted(arg):
> +        return '"%s"' % _escape_quotes(arg)
> +    else:
> +        return arg
> +
> +
> +def _is_arg_quoted(arg):
> +    return (arg[0] == "'" and arg[-1] == "'") or \
> +           (arg[0] == '"' and arg[-1] == '"')
> +
> +
> +def _escape_quotes(arg):
> +    return arg.replace('"', '\\"')
> +
> +
> +# courtesy of 'techtonik' at http://snippets.dzone.com/posts/show/6313
> +def _find_executable(executable, path=None):
> +    """Try to find 'executable' in the directories listed in 'path' (a
> +    string listing directories separated by 'os.pathsep'; defaults to
> +    os.environ['PATH']).  Returns the complete filename or None if not
> +    found
> +    """
> +    if path is None:
> +        path = os.environ['PATH']
> +    paths = path.split(os.pathsep)
> +    extlist = ['']
> +    if sys.platform == 'win32':
> +        pathext = os.environ['PATHEXT'].lower().split(os.pathsep)
> +        (base, ext) = os.path.splitext(executable)
> +        if ext.lower() not in pathext:
> +            extlist = pathext
> +    for ext in extlist:
> +        execname = executable + ext
> +        if os.path.isfile(execname):
> +            return execname
> +        else:
> +            for p in paths:
> +                f = os.path.join(p, execname)
> +                if os.path.isfile(f):
> +                    return f
> +    else:
> +        return None
> +
> +
> +class cmd_mergetools(Command):
> +    __doc__ = """Manages external merge tools.
> +
> +    External merge tools are defined by their command line. The executable may
> +    omit its path if it can be found on the PATH.
> +
> +    When updating or removing an external merge tool, use the simple name of
> +    the executable, e.g. 'C:\Tools\kdiff3.exe' -> 'kdiff3'.
> +
> +    If you see the warning 'External merge tool is not available' when adding
> +    or updating, the executable in the command-line you have supplied cannot
> +    be found.
> +
> +    Examples:
> +        To add an external merge tool:
> +
> +            bzr mergetools --add mydifftool %b %t %o %r
> +
> +        If you need to include options in your external merge tool's
> +        command-line, insert '--' before the command-line to prevent bzr from
> +        processing them as options to the mergetools command:
> +
> +            bzr mergetools --add -- kdiff3 %b %t %o -o %r
> +
> +        To update an existing external merge tool:
> +
> +            bzr mergetools --update=kdiff3 -- /opt/kde/bin/kdiff3 %b %t %o -o %r
> +
> +        To remove an existing external merge tool:
> +
> +            bzr mergetools --remove=kdiff3
> +    """
> +    takes_args = ['args*']
> +    takes_options = [
> +        Option('add', help='Adds an external merge tool using ARGS as the '
> +               'command-line.', short_name='a'),
> +        Option('detect', help='Automatically detect known external merge tools '
> +               'on the PATH'),
> +        Option('list', help='Lists the currently defined external merge tools.',
> +               short_name='l'),
> +        Option('remove', help='Removes the external merge tool called ARG.',
> +               type=str, short_name='r'),
> +        Option('update', help='Updates the external merge tool called ARG '
> +               'using ARGS as the command-line.', type=str, short_name='u'),
> +    ]
> +
> +    def run(self, args_list=None, add=False, detect=False, list=False,
> +            remove=None, update=None):
> +        if (not add and not detect and not list and remove is None and
> +            update is None):
> +            raise errors.BzrCommandError(
> +                'You must supply one of --add, --detect, --list, --remove or '
> +                '--update')
> +        if add:
> +            self.add_tool(args_list)
> +        elif detect:
> +            self.detect_tools()
> +        elif list:
> +            self.list_tools()
> +        elif remove is not None:
> +            self.remove_tool(remove)
> +        elif update is not None:
> +            self.update_tool(update, args_list)
> +
> +    def add_tool(self, args):
> +        if args is None or len(args) == 0:
> +            raise errors.BzrCommandError(
> +                'You must supply the command-line for the external merge tool')
> +        new_mt = MergeTool.from_executable_and_args(args[0], args[1:])
> +        if find_merge_tool(new_mt.get_name()) is not None:
> +            raise errors.BzrCommandError(
> +                'External merge tool already exists: %s' % new_mt.get_name())
> +        merge_tools = get_merge_tools()
> +        merge_tools.append(new_mt)
> +        set_merge_tools(merge_tools)
> +        uif = ui.ui_factory
> +        uif.note('Added external merge tool: %s' % new_mt.get_name())
> +        if not new_mt.is_available():
> +            uif.show_warning(
> +                'External merge tool is not available: %s' % new_mt.get_name())
> +
> +    def detect_tools(self):
> +        new_merge_tools = detect_merge_tools()
> +        merge_tools = get_merge_tools()
> +        for mt in merge_tools:
> +            for new_mt in new_merge_tools:
> +                if mt.get_name() == new_mt.get_name():
> +                    new_merge_tools.remove(new_mt)
> +                    break
> +        uif = ui.ui_factory
> +        if len(new_merge_tools) > 0:
> +            for mt in new_merge_tools:
> +                uif.note('Detected external merge tool: %s' % mt.get_name())
> +        else:
> +            uif.note('No external merge tools detected')
> +            return
> +        new_merge_tools.extend(merge_tools)
> +        set_merge_tools(new_merge_tools)
> +
> +    def list_tools(self):
> +        s = ui.ui_factory.make_output_stream()
> +        merge_tools = get_merge_tools()
> +        for mt in merge_tools:
> +            s.write('%s: %s\n' % (mt.get_name(), mt.get_commandline()))
> +
> +    def remove_tool(self, name):
> +        merge_tools = get_merge_tools()
> +        for mt in merge_tools:
> +            if mt.get_name() == name:
> +                merge_tools.remove(mt)
> +                break
> +        else:
> +            raise errors.BzrCommandError(
> +                'Unknown external merge tool: %s' % name)
> +        set_merge_tools(merge_tools)
> +
> +    def update_tool(self, name, args):
> +        if args is None or len(args) == 0:
> +            raise errors.BzrCommandError(
> +                'You must supply the command-line for the external merge tool')
> +        merge_tools = get_merge_tools()
> +        for mt in merge_tools:
> +            if mt.get_name() == name:
> +                mt.set_executable(args[0])
> +                mt.set_arguments(args[1:])
> +                uif = ui.ui_factory
> +                uif.note('Updated external merge tool: %s' % name)
> +                if not mt.is_available():
> +                    uif.show_warning(
> +                        'External merge tool is not available: %s' %
> +                        mt.get_name())
> +                break
> +        else:
> +            raise errors.BzrCommandError(
> +                'Unknown external merge tool: %s' % name)
> +        set_merge_tools(merge_tools)
>
> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py    2010-10-15 11:20:45 +0000
> +++ bzrlib/tests/__init__.py    2010-10-17 17:48:50 +0000
> @@ -3748,6 +3748,7 @@
>         'bzrlib.tests.test_merge3',
>         'bzrlib.tests.test_merge_core',
>         'bzrlib.tests.test_merge_directive',
> +        'bzrlib.tests.test_mergetools',
>         'bzrlib.tests.test_missing',
>         'bzrlib.tests.test_msgeditor',
>         'bzrlib.tests.test_multiparent',
>
> === added file 'bzrlib/tests/test_mergetools.py'
> --- bzrlib/tests/test_mergetools.py     1970-01-01 00:00:00 +0000
> +++ bzrlib/tests/test_mergetools.py     2010-10-17 17:48:50 +0000
> @@ -0,0 +1,152 @@
> +# Copyright (C) 2005-2010 Canonical Ltd
> +#   Authors: Robert Collins <email address hidden>
> +#            and others
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +
> +import os
> +import re
> +import sys
> +
> +from bzrlib import (
> +    config,
> +    mergetools,
> +    tests
> +)
> +
> +
> +class TestMergeTool(tests.TestCaseInTempDir):
> +    def test_basics(self):
> +        mt = mergetools.MergeTool('/path/to/tool --opt %b -x %t %o --stuff %r')
> +        self.assertEquals('/path/to/tool --opt %b -x %t %o --stuff %r', mt.get_commandline())
> +        self.assertEquals('/path/to/tool', mt.get_executable())
> +        self.assertEquals('--opt %b -x %t %o --stuff %r', mt.get_arguments())
> +        self.assertEquals('tool', mt.get_name())
> +        mt.set_commandline('/new/path/to/bettertool %b %t %o %r')
> +        self.assertEquals('/new/path/to/bettertool %b %t %o %r', mt.get_commandline())
> +        self.assertEquals('/new/path/to/bettertool', mt.get_executable())
> +        self.assertEquals('%b %t %o %r', mt.get_arguments())
> +        mt.set_executable('othertool')
> +        self.assertEquals('othertool', mt.get_executable())
> +        self.assertEquals('othertool %b %t %o %r', mt.get_commandline())
> +        mt.set_arguments('%r %b %t %o')
> +        self.assertEquals('%r %b %t %o', mt.get_arguments())
> +        self.assertEquals('othertool %r %b %t %o', mt.get_commandline())
> +
> +    def test_name_win32(self):
> +        if sys.platform != 'win32':
> +            raise tests.TestSkipped('only required on win32')
> +        mt = mergetools.MergeTool('/path/to/tool.exe foo bar')
> +        self.assertEquals('tool', mt.get_name())
> +        mt = mergetools.MergeTool('/path/to/TOOL.EXE foo bar')
> +        self.assertEquals('TOOL', mt.get_name())
> +
> +    def test_quoted_executable(self):
> +        mt = mergetools.MergeTool('"C:\\Program Files\\KDiff3\\kdiff3.exe" %b %t %o -o %r')
> +        self.assertEquals('kdiff3', mt.get_name())
> +
> +    def test_expand_commandline(self):
> +        mt = mergetools.MergeTool('kdiff3 %b %t %o -o %r')
> +        commandline, _ = mt._expand_commandline('test.txt')
> +        self.assertEquals(
> +            'kdiff3 test.txt.BASE test.txt.THIS test.txt.OTHER -o test.txt',
> +            commandline)
> +        commandline, _ = mt._expand_commandline('file with space.txt')
> +        self.assertEquals(
> +            'kdiff3 "file with space.txt.BASE" "file with space.txt.THIS" "file with space.txt.OTHER" -o "file with space.txt"',
> +            commandline)
> +        commandline, _ = mt._expand_commandline('file with "space and quotes".txt')
> +        self.assertEquals(
> +            'kdiff3 "file with \\"space and quotes\\".txt.BASE" "file with \\"space and quotes\\".txt.THIS" "file with \\"space and quotes\\".txt.OTHER" -o "file with \\"space and quotes\\".txt"',
> +            commandline)
> +
> +    def test_expand_commandline_tempfile(self):
> +        self.build_tree(('test.txt', 'test.txt.BASE', 'test.txt.THIS',
> +                         'test.txt.OTHER'))
> +        mt = mergetools.MergeTool('some_tool %T')
> +        commandline, tmpfile = mt._expand_commandline('test.txt')
> +        self.assertStartsWith(commandline, 'some_tool ')
> +        m = re.match('some_tool (.*)', commandline)
> +        self.assertEquals(tmpfile, m.group(1))
> +        self.failUnlessExists(m.group(1))
> +        os.remove(tmpfile)
> +
> +    def test_is_available(self):
> +        mt = mergetools.MergeTool('%s' % sys.executable)
> +        self.assertTrue(mt.is_available())
> +        mt.set_executable(os.path.basename(sys.executable))
> +        self.assertTrue(mt.is_available())
> +        mt.set_executable("ThisExecutableShouldReallyNotExist")
> +        self.assertFalse(mt.is_available())
> +
> +    def test_empty_commandline(self):
> +        mt = mergetools.MergeTool('')
> +        self.assertEquals('', mt.get_executable())
> +        self.assertEquals('', mt.get_arguments())
> +
> +    def test_no_arguments(self):
> +        mt = mergetools.MergeTool('tool')
> +        self.assertEquals('tool', mt.get_executable())
> +        self.assertEquals('', mt.get_arguments())
> +
> +    def test_get_merge_tools(self):
> +        config = FakeConfig()
> +        config.set_mergetools(['kdiff3 %b %t %o -o %r', 'winmergeu %r', ''])
> +        tools = mergetools.get_merge_tools(config)
> +        self.assertEquals(3, len(tools))
> +        self.assertEquals('kdiff3 %b %t %o -o %r', tools[0].get_commandline())
> +        self.assertEquals('winmergeu %r', tools[1].get_commandline())
> +        self.assertEquals('', tools[2].get_commandline())
> +
> +    def test_set_merge_tools_duplicates(self):
> +        config = FakeConfig()
> +        mergetools.set_merge_tools(
> +            [mergetools.MergeTool('kdiff3 %b %t %o -o %r'),
> +             mergetools.MergeTool('kdiff3 %b %t %o -o %r')],
> +            config)
> +        tools = mergetools.get_merge_tools(config)
> +        self.assertEquals(1, len(tools))
> +        self.assertEquals('kdiff3 %b %t %o -o %r', tools[0].get_commandline())
> +
> +    def test_set_merge_tools_empty_tool(self):
> +        config = FakeConfig()
> +        mergetools.set_merge_tools(
> +            [mergetools.MergeTool('kdiff3 %b %t %o -o %r'),
> +             mergetools.MergeTool('')],
> +            config)
> +        tools = mergetools.get_merge_tools(config)
> +        self.assertEquals(1, len(tools))
> +        self.assertEquals('kdiff3 %b %t %o -o %r', tools[0].get_commandline())
> +
> +
> +class FakeConfig(object):
> +    """
> +    Just enough of the Config interface to fool the mergetools module.
> +    """
> +    def set_mergetools(self, value):
> +        self.mergetools = value
> +
> +    def get_user_option_as_list(self, option):
> +        if option == 'mergetools':
> +            return self.mergetools
> +        else:
> +            raise AssertionError('unknown option "%s"' % option)
> +
> +    def set_user_option(self, option, value):
> +        if option == 'mergetools':
> +            self.mergetools = value
> +        else:
> +            raise AssertionError('unknown option "%s"' % option)
> +
> \ No newline at end of file
>
> === modified file 'doc/en/release-notes/bzr-2.3.txt'
> --- doc/en/release-notes/bzr-2.3.txt    2010-10-15 15:05:09 +0000
> +++ doc/en/release-notes/bzr-2.3.txt    2010-10-17 17:48:50 +0000
> @@ -38,6 +38,10 @@
>   default format for new branches created with ``bzr init``.
>   (Neil Martinsen-Burrell, #484101)
>
> +* ``bzr mergetools`` is a new command that allows you to define external merge
> +  tools. These external merge tools can be used with ``bzr merge`` and
> +  ``bzr remerge`` to resolve text conflicts. (Gordon Tyler, #489915)
> +
>  Improvements
>  ************
>
> @@ -72,6 +76,9 @@
>   deprecated in favour of ``known_hooks.key_to_parent_and_attribute`` in
>   the same module.  (Andrew Bennetts)
>
> +* Added ``bzrlib.mergetools`` module with helper functions for working with
> +  the list of external merge tools.
> +
>  Internals
>  *********
>
>
>
>



-- 
Martin