Rev 5427: Further cleanup. in file:///home/vila/src/bzr/reviews/mergetools/

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon Dec 6 13:50:15 GMT 2010


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

------------------------------------------------------------
revno: 5427
revision-id: v.ladeuil+lp at free.fr-20101206135015-m33q3gm33vrvxkcu
parent: v.ladeuil+lp at free.fr-20101206133347-qjs6o0x3on22741n
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: mergetools
timestamp: Mon 2010-12-06 14:50:15 +0100
message:
  Further cleanup.
-------------- next part --------------
=== modified file 'bzrlib/mergetools.py'
--- a/bzrlib/mergetools.py	2010-12-06 10:33:58 +0000
+++ b/bzrlib/mergetools.py	2010-12-06 13:50:15 +0000
@@ -49,15 +49,6 @@
                    for ext in os.getenv('PATHEXT', '').split(';')]
 
 
-def tool_name_from_executable(executable):
-    name = os.path.basename(executable)
-    if sys.platform == 'win32':
-        root, ext = os.path.splitext(name)
-        if ext.lower() in _WIN32_PATH_EXT:
-            name = root
-    return name
-
-
 class MergeTool(object):
 
     def __init__(self, name, command_line):
@@ -66,27 +57,20 @@
         """
         self.name = name
         self.command_line = command_line
+        self._cmd_list = cmdline.split(self.command_line)
 
     def __repr__(self):
         return '<%s(%s, %s)>' % (self.__class__, self.name, self.command_line)
 
-    def get_executable(self):
-        if len(self._commandline) < 1:
-            return u''
-        return self._commandline[0]
-
-    def set_executable(self, executable):
-        self._commandline[:1] = [executable]
-
     def is_available(self):
-        executable = self.get_executable()
-        return (os.path.exists(executable)
-                or osutils.find_executable_on_path(executable) is not None)
+        exe = self._cmd_list[0]
+        return (os.path.exists(exe)
+                or osutils.find_executable_on_path(exe) is not None)
 
     def invoke(self, filename, invoker=None):
         if invoker is None:
             invoker = subprocess_invoker
-        args, tmp_file = self._subst_filename(self._commandline, filename)
+        args, tmp_file = self._subst_filename(self._cmd_list, filename)
         def cleanup(retcode):
             if tmp_file is not None:
                 if retcode == 0: # on success, replace file with temp file

=== modified file 'bzrlib/tests/test_mergetools.py'
--- a/bzrlib/tests/test_mergetools.py	2010-12-06 10:33:58 +0000
+++ b/bzrlib/tests/test_mergetools.py	2010-12-06 13:50:15 +0000
@@ -36,71 +36,10 @@
     def test_get_commandline(self):
         self.assertEqual(
             '/path/to/tool --opt {base} -x {this} {other} --stuff {result}',
-            self.tool.get_commandline())
-
-    def test_get_commandline_as_list(self):
-        self.assertEqual(
-            ['/path/to/tool', '--opt', '{base}', '-x', '{this}', '{other}',
-             '--stuff', '{result}'],
-            self.tool.get_commandline_as_list())
-
-    def test_get_executable(self):
-        self.assertEqual('/path/to/tool', self.tool.get_executable())
+            self.tool.command_line)
 
     def test_get_name(self):
-        self.assertEqual('sometool', self.tool.get_name())
-
-    def test_set_name(self):
-        self.tool.set_name('bettertool')
-        self.assertEqual('bettertool', self.tool.get_name())
-
-    def test_set_name_none(self):
-        self.tool.set_name(None)
-        self.assertEqual('tool', self.tool.get_name())
-
-    def test_set_commandline(self):
-        self.tool.set_commandline(
-            '/new/path/to/bettertool {base} {this} {other} {result}')
-        self.assertEqual(
-            ['/new/path/to/bettertool', '{base}', '{this}', '{other}',
-             '{result}'],
-            self.tool.get_commandline_as_list())
-
-    def test_set_executable(self):
-        self.tool.set_executable('othertool')
-        self.assertEqual(
-            ['othertool', '--opt', '{base}', '-x', '{this}', '{other}',
-             '--stuff', '{result}'],
-            self.tool.get_commandline_as_list())
-
-    def test_quoted_executable(self):
-        self.requireFeature(backslashdir_feature)
-        self.tool.set_commandline('"C:\\Program Files\\KDiff3\\kdiff3.exe" '
-                                  '{base} {this} {other} -o {result}')
-        self.assertEqual('C:\\Program Files\\KDiff3\\kdiff3.exe',
-                         self.tool.get_executable())
-
-    def test_comparison(self):
-        other_tool = mergetools.MergeTool('sometool',
-            '/path/to/tool --opt {base} -x {this} {other} --stuff {result}')
-        self.assertTrue(self.tool == other_tool)
-        self.assertTrue(self.tool != None)
-
-    def test_comparison_none(self):
-        self.assertFalse(self.tool == None)
-        self.assertTrue(self.tool != None)
-
-    def test_comparison_fail_name(self):
-        other_tool = mergetools.MergeTool('sometoolx',
-            '/path/to/tool --opt {base} -x {this} {other} --stuff {result}')
-        self.assertFalse(self.tool == other_tool)
-        self.assertTrue(self.tool != other_tool)
-
-    def test_comparison_fail_commandline(self):
-        other_tool = mergetools.MergeTool('sometool',
-            '/path/to/tool --opt {base} -x {other} --stuff {result} extra')
-        self.assertFalse(self.tool == other_tool)
-        self.assertTrue(self.tool != other_tool)
+        self.assertEqual('sometool', self.tool.name)
 
 
 class TestUnicodeBasics(tests.TestCase):
@@ -116,57 +55,10 @@
         self.assertEqual(
             u'/path/to/b\u0414r --opt {base} -x {this} {other}'
             ' --stuff {result}',
-            self.tool.get_commandline())
-
-    def test_get_commandline_as_list(self):
-        self.assertEqual(
-            [u'/path/to/b\u0414r', u'--opt', u'{base}', u'-x', u'{this}',
-             u'{other}', u'--stuff', u'{result}'],
-            self.tool.get_commandline_as_list())
-
-    def test_get_executable(self):
-        self.assertEqual(u'/path/to/b\u0414r', self.tool.get_executable())
+            self.tool.command_line)
 
     def test_get_name(self):
-        self.assertEqual(u'someb\u0414r', self.tool.get_name())
-
-    def test_set_name(self):
-        self.tool.set_name(u'betterb\u0414r')
-        self.assertEqual(u'betterb\u0414r', self.tool.get_name())
-
-    def test_set_name_none(self):
-        self.tool.set_name(None)
-        self.assertEqual(u'b\u0414r', self.tool.get_name())
-
-    def test_set_commandline(self):
-        self.tool.set_commandline(
-            u'/new/path/to/betterb\u0414r {base} {this} {other} {result}')
-        self.assertEqual(
-            [u'/new/path/to/betterb\u0414r', u'{base}', u'{this}', u'{other}',
-             u'{result}'],
-            self.tool.get_commandline_as_list())
-
-    def test_comparison(self):
-        other_tool = mergetools.MergeTool(u'someb\u0414r',
-            u'/path/to/b\u0414r --opt {base} -x {this} {other} --stuff {result}')
-        self.assertTrue(self.tool == other_tool)
-        self.assertFalse(self.tool != other_tool)
-
-    def test_comparison_none(self):
-        self.assertFalse(self.tool == None)
-        self.assertTrue(self.tool != None)
-
-    def test_comparison_fail_name(self):
-        other_tool = mergetools.MergeTool(u'someb\u0414rx',
-            u'/path/to/b\u0414r --opt {base} -x {this} {other} --stuff {result}')
-        self.assertFalse(self.tool == other_tool)
-        self.assertTrue(self.tool != other_tool)
-
-    def test_comparison_fail_commandline(self):
-        other_tool = mergetools.MergeTool(u'someb\u0414r',
-            u'/path/to/b\u0414r --opt {base} -x {other} --stuff {result} extra')
-        self.assertFalse(self.tool == other_tool)
-        self.assertTrue(self.tool != other_tool)
+        self.assertEqual(u'someb\u0414r', self.tool.name)
 
 
 class TestMergeToolOperations(tests.TestCaseInTempDir):
@@ -229,24 +121,14 @@
         mt = mergetools.MergeTool(None, "ThisExecutableShouldReallyNotExist")
         self.assertFalse(mt.is_available())
 
-    def test_empty_commandline(self):
-        mt = mergetools.MergeTool('', '')
-        self.assertEqual('', mt.get_commandline())
-
-    def test_no_arguments(self):
-        mt = mergetools.MergeTool('tool', 'tool')
-        self.assertEqual('tool', mt.get_commandline())
-
 
 class TestModuleFunctions(tests.TestCaseInTempDir):
 
     def test_detect(self):
         # only way to reliably test detection is to add a known existing
         # executable to the list used for detection
-        old_kmt = mergetools._KNOWN_MERGE_TOOLS
-        mergetools._KNOWN_MERGE_TOOLS = ['sh', 'cmd']
+        self.overrideAttr(mergetools, '_KNOWN_MERGE_TOOLS', ['sh', 'cmd'])
         tools = mergetools.detect_merge_tools()
-        tools_commandlines = [mt.get_commandline() for mt in tools]
+        tools_commandlines = [mt.command_line for mt in tools]
         self.assertTrue('sh' in tools_commandlines or
                         'cmd' in tools_commandlines)
-        mergetools._KNOWN_MERGE_TOOLS = old_kmt



More information about the bazaar-commits mailing list