Merge lp:~doxxx/bzr/828803-get_merge_tools into lp:bzr

[Gordon Tyler]

Properly decode command lines with embedded quotes for external merge tools.

Fixes bug 828803 by using get_user_option in get_merge_tools so that config values are correctly unquoted.

[Gordon Tyler (doxxx)]

I'm going to add tests for this.

[Gordon Tyler (doxxx)]

Sorry, I haven't had time to look at this yet. Family visits, etc.

[Gordon Tyler (doxxx)]

I've updated the tests.

[Vincent Ladeuil (vila)]

Sounds good to me !

You lack a news entry though.

Also, I'm fine landing this in trunk but as soon as you get confirmation that it fixes the issue, it would be nice to backport it to 2.4 no ?

Voting 'needsfixing' but I can add the news entry and land it if you prefer.

review: Needs Fixing

[Gordon Tyler (doxxx)]

On Friday, September 09, 2011 3:26:23 AM, Vincent Ladeuil wrote:
> You lack a news entry though.

I always do. ;) Not enough practice.

> Also, I'm fine landing this in trunk but as soon as you get confirmation that it fixes the issue, it would be nice to backport it to 2.4 no ?

True... I'll do one for 2.4 and submit it as well.

> Voting 'needsfixing' but I can add the news entry and land it if you prefer.

No worries, I can do it now.

[Gordon Tyler (doxxx)]

Release notes added.

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2011-09-07 15:21:39 +0000
+++ bzrlib/config.py	2011-09-09 12:30:53 +0000
@@ -622,7 +622,7 @@
         for (oname, value, section, conf_id, parser) in self._get_options():
             if oname.startswith('bzr.mergetool.'):
                 tool_name = oname[len('bzr.mergetool.'):]
-                tools[tool_name] = value
+                tools[tool_name] = self.get_user_option(oname)
         trace.mutter('loaded merge tools: %r' % tools)
         return tools
 

=== modified file 'bzrlib/tests/test_config.py'
--- bzrlib/tests/test_config.py	2011-09-07 15:21:39 +0000
+++ bzrlib/tests/test_config.py	2011-09-09 12:30:53 +0000
@@ -183,6 +183,7 @@
 user_global_option=something
 bzr.mergetool.sometool=sometool {base} {this} {other} -o {result}
 bzr.mergetool.funkytool=funkytool "arg with spaces" {this_temp}
+bzr.mergetool.newtool='"newtool with spaces" {this_temp}'
 bzr.default_mergetool=sometool
 [ALIASES]
 h=help
@@ -1339,7 +1340,8 @@
         self.log(repr(tools))
         self.assertEqual(
             {u'funkytool' : u'funkytool "arg with spaces" {this_temp}',
-            u'sometool' : u'sometool {base} {this} {other} -o {result}'},
+            u'sometool' : u'sometool {base} {this} {other} -o {result}',
+            u'newtool' : u'"newtool with spaces" {this_temp}'},
             tools)
 
     def test_get_merge_tools_empty(self):

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-09-09 07:52:11 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-09-09 12:30:53 +0000
@@ -172,6 +172,9 @@
   operations that use it, like merge, can now create trees without a root.
   (Aaron Bentley)
 
+* Fixed loading of external merge tools from config to properly decode
+  command-lines which contain embedded quotes. (Gordon Tyler, #828803)
+
 Documentation
 *************