Rev 5425: Simplify MergeTool by making the name mandatory and keeping the command line intact. in file:///home/vila/src/bzr/reviews/mergetools/

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon Dec 6 10:33:58 GMT 2010


At file:///home/vila/src/bzr/reviews/mergetools/

------------------------------------------------------------
revno: 5425
revision-id: v.ladeuil+lp at free.fr-20101206103358-o4rr2spy1ggkhwa8
parent: v.ladeuil+lp at free.fr-20101206101525-4f9xea2yqcak0vlx
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: mergetools
timestamp: Mon 2010-12-06 11:33:58 +0100
message:
  Simplify MergeTool by making the name mandatory and keeping the command line intact.
-------------- next part --------------
=== modified file 'bzrlib/cmdline.py'
--- a/bzrlib/cmdline.py	2010-11-27 05:03:48 +0000
+++ b/bzrlib/cmdline.py	2010-12-06 10:33:58 +0000
@@ -158,23 +158,3 @@
 def split(unsplit, single_quotes_allowed=True):
     splitter = Splitter(unsplit, single_quotes_allowed=single_quotes_allowed)
     return [arg for quoted, arg in splitter]
-
-
-def unsplit(args):
-    return ' '.join([_quote_arg(arg) for arg in args])
-    
-
-def _quote_arg(arg):
-    if u' ' in arg and not _is_arg_quoted(arg):
-        return u'"%s"' % _escape_quotes(arg)
-    else:
-        return arg
-
-
-def _is_arg_quoted(arg):
-    return (arg[0] == u"'" and arg[-1] == u"'") or \
-           (arg[0] == u'"' and arg[-1] == u'"')
-
-
-def _escape_quotes(arg):
-    return arg.replace(u'"', u'\\"')

=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2010-12-06 10:15:25 +0000
+++ b/bzrlib/config.py	2010-12-06 10:33:58 +0000
@@ -370,15 +370,15 @@
     def set_merge_tools(self, tools):
         # remove entries from config for tools which do not appear in
         # merge_tools
-        tool_names = [tool.get_name() for tool in tools]
+        tool_names = [tool.name for tool in tools]
         for (oname, value, section, conf_id, parser) in self._get_options():
             if oname.startswith('bzr.mergetool.'):
                 if oname[len('bzr.mergetool.'):] not in tool_names:
                     self.remove_user_option(oname)
         # set config entries
         for tool in tools:
-            oname = 'bzr.mergetool.%s' % tool.get_name()
-            value = tool.get_commandline()
+            oname = 'bzr.mergetool.%s' % tool.name
+            value = tool.command_line
             if oname == '' or value == '':
                 continue
             self.set_user_option(oname, value)

=== modified file 'bzrlib/mergetools.py'
--- a/bzrlib/mergetools.py	2010-12-06 10:01:50 +0000
+++ b/bzrlib/mergetools.py	2010-12-06 10:33:58 +0000
@@ -60,59 +60,15 @@
 
 class MergeTool(object):
 
-    def __init__(self, name, commandline):
+    def __init__(self, name, command_line):
         """Initializes the merge tool with a name and a command-line (a string
         or sequence of strings).
         """
-        self.set_commandline(commandline)
-        self.set_name(name) # needs commandline set first when name is None
+        self.name = name
+        self.command_line = command_line
 
     def __repr__(self):
-        return '<MergeTool %s: %r>' % (self._name, self._commandline)
-
-    def __eq__(self, other):
-        if type(other) == MergeTool:
-            return cmp(self, other) == 0
-        else:
-            return False
-
-    def __ne__(self, other):
-        if type(other) == MergeTool:
-            return cmp(self, other) != 0
-        else:
-            return True
-
-    def __cmp__(self, other):
-        if type(other == MergeTool):
-            return cmp((self._name, self._commandline),
-                (other._name, other._commandline))
-
-    def __str__(self):
-        return self.get_commandline()
-
-    def get_name(self):
-        return self._name
-
-    def set_name(self, name):
-        if name is None:
-            self._name = tool_name_from_executable(self.get_executable())
-        else:
-            self._name = name
-
-    def get_commandline(self):
-        return cmdline.unsplit(self._commandline)
-
-    def get_commandline_as_list(self):
-        return self._commandline
-
-    def set_commandline(self, commandline):
-        if isinstance(commandline, basestring):
-            self._commandline = cmdline.split(commandline)
-        elif isinstance(commandline, (tuple, list)):
-            self._commandline = list(commandline)
-        else:
-            raise TypeError('%r is not valid for commandline; must be string '
-                            'or sequence of strings' % commandline)
+        return '<%s(%s, %s)>' % (self.__class__, self.name, self.command_line)
 
     def get_executable(self):
         if len(self._commandline) < 1:
@@ -124,8 +80,8 @@
 
     def is_available(self):
         executable = self.get_executable()
-        return os.path.exists(executable) or \
-               osutils.find_executable_on_path(executable) is not None
+        return (os.path.exists(executable)
+                or osutils.find_executable_on_path(executable) is not None)
 
     def invoke(self, filename, invoker=None):
         if invoker is None:

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2010-12-06 10:15:25 +0000
+++ b/bzrlib/tests/test_config.py	2010-12-06 10:33:58 +0000
@@ -963,22 +963,22 @@
         tools = conf.get_merge_tools()
         self.log(repr(tools))
         self.assertEqual(3, len(tools))
-        self.assertEqual('kdiff3', tools[0].get_name())
+        self.assertEqual('kdiff3', tools[0].name)
         self.assertEqual('kdiff3 {base} {this} {other} -o {result}',
-                         tools[0].get_commandline())
-        self.assertEqual('winmergeu', tools[1].get_name())
-        self.assertEqual('winmergeu {result}', tools[1].get_commandline())
-        self.assertEqual('funkytool', tools[2].get_name())
+                         tools[0].command_line)
+        self.assertEqual('winmergeu', tools[1].name)
+        self.assertEqual('winmergeu {result}', tools[1].command_line)
+        self.assertEqual('funkytool', tools[2].name)
         self.assertEqual('funkytool "arg with spaces" {this_temp}',
-                          tools[2].get_commandline())
+                          tools[2].command_line)
 
     def test_get_default_merge_tool(self):
         conf = self._get_sample_config()
         tool = conf.get_default_merge_tool()
         self.log(repr(tool))
-        self.assertEqual('kdiff3', tool.get_name())
+        self.assertEqual('kdiff3', tool.name)
         self.assertEqual('kdiff3 {base} {this} {other} -o {result}',
-                         tool.get_commandline())
+                         tool.command_line)
 
     def test_get_default_merge_tool_empty(self):
         conf = self._get_empty_config()
@@ -1034,9 +1034,9 @@
             ])
         tools = conf.get_merge_tools()
         self.assertEqual(1, len(tools))
-        self.assertEqual('kdiff3', tools[0].get_name())
+        self.assertEqual('kdiff3', tools[0].name)
         self.assertEqual('kdiff3 {base} {this} {other} -o {result}',
-                         tools[0].get_commandline())
+                         tools[0].command_line)
 
     def test_set_merge_tools_empty_tool(self):
         conf = config.GlobalConfig()
@@ -1048,9 +1048,9 @@
             ])
         tools = conf.get_merge_tools()
         self.assertEqual(1, len(tools))
-        self.assertEqual('kdiff3', tools[0].get_name())
+        self.assertEqual('kdiff3', tools[0].name)
         self.assertEqual('kdiff3 {base} {this} {other} -o {result}',
-                         tools[0].get_commandline())
+                         tools[0].command_line)
 
     def test_set_merge_tools_remove_one(self):
         conf = config.GlobalConfig()

=== modified file 'bzrlib/tests/test_mergetools.py'
--- a/bzrlib/tests/test_mergetools.py	2010-12-06 10:01:50 +0000
+++ b/bzrlib/tests/test_mergetools.py	2010-12-06 10:33:58 +0000
@@ -146,20 +146,6 @@
              u'{result}'],
             self.tool.get_commandline_as_list())
 
-    def test_set_executable(self):
-        self.tool.set_executable(u'otherb\u0414r')
-        self.assertEqual(
-            [u'otherb\u0414r', u'--opt', u'{base}', u'-x', u'{this}',
-             u'{other}', u'--stuff', u'{result}'],
-            self.tool.get_commandline_as_list())
-
-    def test_quoted_executable(self):
-        self.requireFeature(backslashdir_feature)
-        self.tool.set_commandline(u'"C:\\Program Files\\KDiff3\\b\u0414r.exe" '
-                                  '{base} {this} {other} -o {result}')
-        self.assertEqual(u'C:\\Program Files\\KDiff3\\b\u0414r.exe',
-                         self.tool.get_executable())
-
     def test_comparison(self):
         other_tool = mergetools.MergeTool(u'someb\u0414r',
             u'/path/to/b\u0414r --opt {base} -x {this} {other} --stuff {result}')



More information about the bazaar-commits mailing list