Merge lp:~doxxx/qbzr/mergetools into lp:qbzr

[Gordon Tyler]

(doxxx) Use bzrlib.mergetools for managing and using external merge tools in qconfig and qconflicts.

This merge proposal changes qconfig and qconflicts to make use of the bzrlib.mergetools module added by lp:~doxxx/bzr/mergetools. It uses the API to display and edit the list of external merge tools in the Merge tab of qconfig, to display a dropdown selector of an external merge tool in qconflicts and to invoke the selected external merge tool from qconflicts.

[Gordon Tyler (doxxx)]

Almost ready for review, just need to tweak the way error messages are displayed in qconflicts.

[Gordon Tyler (doxxx)]

Could I get some feedback on this? Any suggestions for how to better handle notification of problems with the selected merge tool in qconflicts?

[lp:~doxxx/qbzr/mergetools]

1313. By Gordon Tyler on 2010-10-28

Updated for API change in bzrlib.mergetools.

1314. By Gordon Tyler on 2010-11-06

Use missing gettext call for a user-visible string.

1315. By Gordon Tyler on 2010-11-07

Replaced Merge config tab contents with new UI.

[Gordon Tyler (doxxx)]

I've redesigned the UI for the Merge tab in qconfig to allow editing the name and commandline of merge tools.

[lp:~doxxx/qbzr/mergetools]

1316. By Gary van der Merwe on 2010-11-08

qconfig: Don't crash if no default is set.

[Gary van der Merwe (garyvdm)]

Sorry about the delay in looking at this. Been busy at work.

Thank you for tackling this.

* qconfig and qconflicts break if using using a old bzr. I would like to keep some level of backwards compatibility. So if running an old bzr, qconfig should load, but the merge tab should show a error message. qconflicts should load, show conflicts, but you won't be able to launch a ext merges app.

* I would prefer for the qconfig merge tab to be a simple list, like the diff tab, rather than the list + form. My reasons for this are:
  + It's easier to code. With the list + form code, you have to do a lot of work to synchronize the selection in the list, and the form item displayed. (see bug below.)
  + Less widgets = Cleaner look.

Anyway - this is bike shedding. So if you disagree, I don't mind. What I do want is for the diff and merge tabs to share the same design, and code.

* I would be nice to give help on then merge parameters (%b %T %o, etc..). Maybe a link to qhelp, like the qgetnew dialog.

* First time I ran qconfig, I got the following error, because I did not have a default set.

bzr: ERROR: exceptions.IndexError: list index out of range

Traceback (most recent call last):
  File "/home/garyvdm/qbzr/mergetools/lib/commands.py", line 166, in run
    ret_code = self._qbzr_run(*args, **kwargs)
  File "/home/garyvdm/qbzr/mergetools/lib/commands.py", line 504, in _qbzr_run
    window = QBzrConfigWindow()
  File "/home/garyvdm/bzr/mergetools/bzrlib/lazy_import.py", line 128, in __call__
    return obj(*args, **kwargs)
  File "/home/garyvdm/qbzr/mergetools/lib/config.py", line 247, in __init__
    self.load()
  File "/home/garyvdm/qbzr/mergetools/lib/config.py", line 378, in load
    self.merge_tools_list_model.set_merge_tools(definedMergeTools, defaultMergeTool)
  File "/home/garyvdm/qbzr/mergetools/lib/config.py", line 743, in set_merge_tools
    self._default = [mt for mt in self._merge_tools if mt == default][0]
IndexError: list index out of range

I've fixed this and pushed to lp:~qbzr-dev/qbzr/mergetools. I've added you to ~qbzr-dev, so if you want, you can also push there.

* In qconfig, if you click on then ... button for the command line, and click cancel, it changes the command line. It should not.

* I saw this bug: steps to reproduce:
Click on a merge tool.
Click detect. (There is now no selection in the list box, but the form on the left stays populated.)
Edit the command line.
press tab (or otherwise let the command line text box lose focus.)

Actual result: get this error:
bzr: ERROR: exceptions.AssertionError:

Traceback (most recent call last):
  File "/home/garyvdm/qbzr/mergetools/lib/config.py", line 606, in merge_tool_name_changed
    assert sel_model.hasSelection()
AssertionError

* The qconficts code seems pretty solid.

I've been wanting to change qconficts to use the treewidget for a while. I'll see if I can work on it shortly.

I also want to make a merge tool menu, like then diff tool menu, so you can select the merge tool in qcommit, qbrowse, be wt view, etc.

review: Needs Fixing

[lp:~doxxx/qbzr/mergetools]

1317. By Gary van der Merwe on 2010-11-08

Merge Trunk.

[Gordon Tyler (doxxx)]

> Sorry about the delay in looking at this. Been busy at work.

No worries, thanks for the review.

> * qconfig and qconflicts break if using using a old bzr. I would like to keep
> some level of backwards compatibility. So if running an old bzr, qconfig
> should load, but the merge tab should show a error message. qconflicts should
> load, show conflicts, but you won't be able to launch a ext merges app.

Okay.

> * I would prefer for the qconfig merge tab to be a simple list, like the diff
> tab, rather than the list + form. My reasons for this are:
> + It's easier to code. With the list + form code, you have to do a lot of
> work to synchronize the selection in the list, and the form item displayed.
> (see bug below.)
> + Less widgets = Cleaner look.
>
> Anyway - this is bike shedding. So if you disagree, I don't mind. What I do
> want is for the diff and merge tabs to share the same design, and code.

I went with a separate list and edit form design because I thought it would be easier to use, which I felt was a drawback of the previous design. For instance, there's no obvious clue that the radio button of the previous design indicates that it marks the default tool.

I will investigate an alternate design using a table though, to see if I can come up with something that is easier to use than the old design and can be re-used for the Diff tab.

> * I would be nice to give help on then merge parameters (%b %T %o, etc..).
> Maybe a link to qhelp, like the qgetnew dialog.

Okay.

> * First time I ran qconfig, I got the following error, because I did not have
> a default set.

> I've fixed this and pushed to lp:~qbzr-dev/qbzr/mergetools. I've added you to
> ~qbzr-dev, so if you want, you can also push there.

Cool. Thanks. Can I just pull/push --remember to that with my current branch and continue working like that? And if I do, should I resubmit this merge proposal using the new branch?

> * In qconfig, if you click on then ... button for the command line, and click
> cancel, it changes the command line. It should not.

Indeed it should not.

> * I saw this bug: steps to reproduce:
> Click on a merge tool.
> Click detect. (There is now no selection in the list box, but the form on the
> left stays populated.)
> Edit the command line.
> press tab (or otherwise let the command line text box lose focus.)
>
> Actual result: get this error:
> bzr: ERROR: exceptions.AssertionError:
>
> Traceback (most recent call last):
> File "/home/garyvdm/qbzr/mergetools/lib/config.py", line 606, in
> merge_tool_name_changed
> assert sel_model.hasSelection()
> AssertionError

Yeah, that's to catch code paths like the one you discovered when the list of merge tools has no selection but the edit form is still enabled. I'll investigate.

> I've been wanting to change qconficts to use the treewidget for a while. I'll
> see if I can work on it shortly.

You mean a treewidget to show the conflicts in the context of the working tree structure?

> I also want to make a merge tool menu, like then diff tool menu, so you can
> select the merge tool in qcommit, qbrowse, be wt view, etc.

Nice idea.

[lp:~doxxx/qbzr/mergetools]

1318. By Gordon Tyler on 2010-11-08

Fixed the other code site which modifies self._default.

1319. By Gordon Tyler on 2010-11-09

Improved how detected tools are added to the list so that its selection is not cleared.

[Gary van der Merwe (garyvdm)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/11/2010 01:46, Gordon Tyler wrote:
>> I've fixed this and pushed to lp:~qbzr-dev/qbzr/mergetools. I've added you to
>> ~qbzr-dev, so if you want, you can also push there.
>
> Cool. Thanks. Can I just pull/push --remember to that with my current
> branch and continue working like that? And if I do, should I
> resubmit this merge proposal using the new branch?

Yes. Then it makes it easier for me to provide fixes.

>> I've been wanting to change qconficts to use the treewidget for a while. I'll
>> see if I can work on it shortly.
>
> You mean a treewidget to show the conflicts in the context of the working tree structure?

Yes - like qcommit/qrevet/qadd.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkzY+Q0ACgkQd/3EdwGKOh2qiQCgg+HP/oPNqu0UC/AkX1ULyqaq
9QYAniJ50ZS/Ak9Ie/iI+qig6zzWmcQN
=svmx
-----END PGP SIGNATURE-----

[Gordon Tyler (doxxx)]

Superseded by https://code.launchpad.net/~qbzr-dev/qbzr/mergetools/+merge/40426.

Unmerged revisions

=== modified file 'NEWS.txt'
--- NEWS.txt	2010-11-08 14:03:03 +0000
+++ NEWS.txt	2010-11-09 00:10:03 +0000
@@ -24,6 +24,8 @@
      (Bug #621934)
  * qcommit: added option to load commit message from a file.
    (Bug #640071, Philip Peitsch)
+ * Use bzrlib.mergetools for managing and using external merge tools in qconfig
+   and qconflicts. (Bug #489915, Gordon Tyler)
 
 
 0.19.3 - Under Development
 
=== modified file 'lib/config.py'
--- lib/config.py	2010-09-06 06:56:31 +0000
+++ lib/config.py	2010-11-09 00:10:03 +0000
@@ -20,12 +20,14 @@
 import re
 import os.path
 from PyQt4 import QtCore, QtGui
+from PyQt4.Qt import Qt
 from bzrlib.config import (
     ensure_config_dir_exists,
     extract_email_address,
     )
-from bzrlib import errors, trace
+from bzrlib import errors, mergetools, trace
 
+from bzrlib.plugins.qbzr.lib import ui_merge_config
 from bzrlib.plugins.qbzr.lib.i18n import gettext, N_
 from bzrlib.plugins.qbzr.lib.spellcheck import SpellChecker
 from bzrlib.plugins.qbzr.lib.util import (
@@ -188,34 +190,48 @@
         diffLayout.addWidget(self.extDiffList)
         diffLayout.addLayout(extDiffButtonsLayout)
         
+        self.merge_ui = ui_merge_config.Ui_MergeConfig()
         mergeWidget = QtGui.QWidget()
-
-        label = QtGui.QLabel(gettext("External Merge Apps:"))
-        self.extMergeList = QtGui.QTreeWidget(mergeWidget)
-        self.extMergeList.setRootIsDecorated(False)
-        self.extMergeList.setHeaderLabels([gettext("Definition")])
-        self.extMergeList.setItemDelegateForColumn(0,
-            QRadioCheckItemDelegate(self.extMergeList))
-        self.connect(self.extMergeList, QtCore.SIGNAL("itemChanged (QTreeWidgetItem *,int)"),
-                     self.extMergeListItemChanged)        
-
-        addExtMergeButton = QtGui.QPushButton(gettext("Add"), mergeWidget)
-        self.connect(addExtMergeButton, QtCore.SIGNAL("clicked()"),
-                     self.addExtMerge)
-        removeExtMergeButton = QtGui.QPushButton(gettext("Remove"), mergeWidget)
-        self.connect(removeExtMergeButton, QtCore.SIGNAL("clicked()"),
-                     self.removeExtMerge)
-
-        extMergeButtonsLayout = QtGui.QHBoxLayout()
-        extMergeButtonsLayout.addWidget(addExtMergeButton)
-        extMergeButtonsLayout.addWidget(removeExtMergeButton)
-        extMergeButtonsLayout.addStretch()
-
-        mergeLayout = QtGui.QVBoxLayout(mergeWidget)
-        mergeLayout.addWidget(label)
-        mergeLayout.addWidget(self.extMergeList)
-        mergeLayout.addLayout(extMergeButtonsLayout)
-        
+        self.merge_ui.setupUi(mergeWidget)
+        self.merge_ui.merge_tool_details.setEnabled(False)
+        
+        self.merge_tools_list_model = MergeToolsListModel()
+        self.merge_ui.merge_tools_list.setModel(
+            self.merge_tools_list_model)
+
+        self.connect(self.merge_ui.merge_tools_list.selectionModel(),
+                     QtCore.SIGNAL("currentChanged(QModelIndex,QModelIndex)"),
+                     self.merge_tools_list_currentChanged)
+        self.connect(self.merge_tools_list_model,
+                     QtCore.SIGNAL("dataChanged(QModelIndex,QModelIndex)"),
+                     self.merge_tools_list_dataChanged)
+        
+        self.connect(self.merge_ui.merge_tools_add,
+                     QtCore.SIGNAL("clicked()"),
+                     self.merge_tools_add_clicked)
+        self.connect(self.merge_ui.merge_tools_remove,
+                     QtCore.SIGNAL("clicked()"),
+                     self.merge_tools_remove_clicked)
+        self.connect(self.merge_ui.merge_tools_detect,
+                     QtCore.SIGNAL("clicked()"),
+                     self.merge_tools_detect_clicked)
+        
+        self.connect(self.merge_ui.merge_tool_name,
+                     QtCore.SIGNAL("editingFinished()"),
+                     self.merge_tool_name_changed)
+        
+        self.connect(self.merge_ui.merge_tool_commandline,
+                     QtCore.SIGNAL("editingFinished()"),
+                     self.merge_tool_commandline_changed)
+        
+        self.connect(self.merge_ui.merge_tool_default,
+                     QtCore.SIGNAL("toggled(bool)"),
+                     self.merge_tool_default_toggled)
+        
+        self.connect(self.merge_ui.merge_tool_browse,
+                     QtCore.SIGNAL("clicked()"),
+                     self.merge_tool_browse_clicked)
+
         self.tabwidget.addTab(generalWidget, gettext("General"))
         self.tabwidget.addTab(aliasesWidget, gettext("Aliases"))
         self.tabwidget.addTab(bugTrackersWidget, gettext("Bug Trackers"))
@@ -357,30 +373,10 @@
         self.extDiffListIgnore = False
 
         # Merge
-        bzr_config = get_global_config()
-        defaultMerge = bzr_config.get_user_option("external_merge")
-        if defaultMerge is None:
-            defaultMerge = ""
-        
-        self.extMergeListIgnore = True
-        def create_ext_merge_item(definition):
-            item = QtGui.QTreeWidgetItem(self.extMergeList)
-            item.setFlags(QtCore.Qt.ItemIsSelectable |
-                          QtCore.Qt.ItemIsEditable |
-                          QtCore.Qt.ItemIsEnabled |
-                          QtCore.Qt.ItemIsUserCheckable)
-            if definition == defaultMerge:
-                item.setCheckState(0, QtCore.Qt.Checked)
-            else:
-                item.setCheckState(0, QtCore.Qt.Unchecked)
-            
-            item.setText(0, definition)
-            return item
-
-        for name, value in parser.get('DEFAULT', {}).items():
-            if name == "external_merge":
-                create_ext_merge_item(value)
-        self.extMergeListIgnore = False
+        definedMergeTools = mergetools.get_merge_tools(config)
+        defaultMergeTool = mergetools.get_default_merge_tool(config)
+        self.merge_tools_list_model.set_merge_tools(definedMergeTools, defaultMergeTool)
+        self.merge_tools_list_model.sort(0, Qt.AscendingOrder)
 
     def save(self):
         """Save the configuration."""
@@ -465,26 +461,20 @@
         qconfig.set_option('default_diff',
                            defaultDiff)
         
-        # Merge        
-        defaultMerge = None
-        for index in range(self.extMergeList.topLevelItemCount()):
-            item = self.extMergeList.topLevelItem(index)
-            definition = unicode(item.text(0))
-            if item.checkState(0) == QtCore.Qt.Checked:
-                defaultMerge = definition
-            
-        set_or_delete_option(parser, 'external_merge',
-                             defaultMerge)
         
         def save_config(config, parser):
-            ensure_config_dir_exists(os.path.dirname(config._get_filename()))
-            f = open(config._get_filename(), 'wb')
+            ensure_config_dir_exists(os.path.dirname(config.file_name))
+            f = open(config.file_name, 'wb')
             parser.write(f)
             f.close()
         
         save_config(config, parser)
         qconfig.save()
 
+        # Merge
+        mergetools.set_merge_tools(self.merge_tools_list_model.get_merge_tools())
+        mergetools.set_default_merge_tool(self.merge_tools_list_model.get_default())
+
     def do_accept(self):
         """Save changes and close the window."""
         if not self.validate():
@@ -578,42 +568,80 @@
                 changed_item.setCheckState(0, QtCore.Qt.Checked)
                 self.extDiffListIgnore = False
 
-    def addExtMerge(self):
-        item = QtGui.QTreeWidgetItem(self.extMergeList)
-        item.setFlags(QtCore.Qt.ItemIsSelectable |
-                      QtCore.Qt.ItemIsEditable |
-                      QtCore.Qt.ItemIsEnabled |
-                      QtCore.Qt.ItemIsUserCheckable)
-        item.setCheckState(0, QtCore.Qt.Unchecked)
-        self.extMergeList.setCurrentItem(item)
-        self.extMergeList.editItem(item, 0)
-
-    def removeExtMerge(self):
-        for item in self.extMergeList.selectedItems():
-            index = self.extMergeList.indexOfTopLevelItem(item)
-            self.extMergeList.takeTopLevelItem(index)
-
-    def extMergeListItemChanged(self, changed_item, col):
-        if col == 0 and not self.extMergeListIgnore:
-            checked_count = 0
-            for index in range(self.extMergeList.topLevelItemCount()):
-                item = self.extMergeList.topLevelItem(index)
-                if item.checkState(0) == QtCore.Qt.Checked:
-                    checked_count += 1
+    def merge_tools_list_currentChanged(self, curr, prev):
+        mt = self.merge_tools_list_model.get_merge_tool(curr)
+        self.merge_ui.merge_tool_name.setText(mt.get_name())
+        self.merge_ui.merge_tool_commandline.setText(
+            mt.get_commandline(quote=True))
+        default = self.merge_tools_list_model.get_default()
+        self.merge_ui.merge_tool_default.blockSignals(True)
+        self.merge_ui.merge_tool_default.setChecked(default is mt)
+        self.merge_ui.merge_tool_default.blockSignals(False)
+        self.merge_ui.merge_tool_details.setEnabled(True)
+        
+    def merge_tools_list_dataChanged(self, first, last):
+        sel_model = self.merge_ui.merge_tools_list.selectionModel()
+        assert sel_model.hasSelection()
+        curr = sel_model.currentIndex()
+        if curr >= first and curr <= last:
+            self.merge_tools_list_currentChanged(curr, None)
+        
+    def merge_tools_add_clicked(self):
+        index = self.merge_tools_list_model.new_merge_tool()
+        sel_model = self.merge_ui.merge_tools_list.selectionModel()
+        sel_model.setCurrentIndex(index, QtGui.QItemSelectionModel.ClearAndSelect)
+        self.merge_ui.merge_tool_name.setFocus(Qt.OtherFocusReason)
+        self.merge_ui.merge_tool_name.selectAll()
+        
+    def merge_tools_remove_clicked(self):
+        sel_model = self.merge_ui.merge_tools_list.selectionModel()
+        assert sel_model.hasSelection()
+        self.merge_tools_list_model.remove_merge_tool(sel_model.currentIndex())
+    
+    def merge_tools_detect_clicked(self):
+        self.merge_tools_list_model.detect_merge_tools()
+    
+    def merge_tool_name_changed(self):
+        sel_model = self.merge_ui.merge_tools_list.selectionModel()
+        assert sel_model.hasSelection()
+        text = unicode(self.merge_ui.merge_tool_name.text())
+        self.merge_tools_list_model.set_merge_tool_name(
+            sel_model.currentIndex(), text)
+        self.merge_tools_list_model.sort(0, Qt.AscendingOrder)
+        
+    def merge_tool_commandline_changed(self):
+        sel_model = self.merge_ui.merge_tools_list.selectionModel()
+        assert sel_model.hasSelection()
+        curr = sel_model.currentIndex()
+        mt = self.merge_tools_list_model.get_merge_tool(curr)
+        text = unicode(self.merge_ui.merge_tool_commandline.text())
+        mt.set_commandline(text)
+        
+    def merge_tool_default_toggled(self, checked):
+        sel_model = self.merge_ui.merge_tools_list.selectionModel()
+        assert sel_model.hasSelection()
+        if checked:
+            new_default = self.merge_tools_list_model.get_merge_tool(
+                sel_model.currentIndex())
+            if self.merge_tools_list_model.get_default() is not new_default:
+                self.merge_tools_list_model.set_default(new_default)
+        else:
+            self.merge_tools_list_model.set_default(None)
             
-            if checked_count == 0:
-                self.extMergeListIgnore = True
-                changed_item.setCheckState(0, QtCore.Qt.Checked)
-                self.extMergeListIgnore = False
-            elif checked_count > 1:
-                self.extMergeListIgnore = True
-                for index in range(self.extMergeList.topLevelItemCount()):
-                    item = self.extMergeList.topLevelItem(index)
-                    if item.checkState(0) == QtCore.Qt.Checked:
-                        item.setCheckState(0, QtCore.Qt.Unchecked)
-                changed_item.setCheckState(0, QtCore.Qt.Checked)
-                self.extMergeListIgnore = False
-
+    def merge_tool_browse_clicked(self):
+        sel_model = self.merge_ui.merge_tools_list.selectionModel()
+        assert sel_model.hasSelection()
+        self.merge_tool_commandline_changed() # force update of model
+        filename = QtGui.QFileDialog.getOpenFileName(self,
+            gettext('Select merge tool executable'),
+            '/')
+        if filename is not None:
+            curr = sel_model.currentIndex()
+            mt = self.merge_tools_list_model.get_merge_tool(curr)
+            mt.set_executable(unicode(filename))
+            self.merge_ui.merge_tool_commandline.setText(
+                mt.get_commandline(quote=True))
+        
     def browseEditor(self):
         filename = QtGui.QFileDialog.getOpenFileName(self,
             gettext('Select editor executable'),
@@ -698,3 +726,130 @@
 
     import socket
     return realname, (username + '@' + socket.gethostname())
+
+class MergeToolsListModel(QtCore.QAbstractListModel):
+    def __init__(self):
+        super(MergeToolsListModel, self).__init__()
+        self._merge_tools = []
+        self._default = None
+        
+    def get_merge_tools(self):
+        return self._merge_tools
+    
+    def set_merge_tools(self, merge_tools, default):
+        self.beginResetModel()
+        self._merge_tools = merge_tools
+        # see set_default for explanation
+        for mt in self._merge_tools:
+            if mt == default:
+                self._default = mt
+                break
+        self.endResetModel()
+        
+    def get_default(self):
+        return self._default
+        
+    def set_default(self, new_default):
+        old_index = None
+        if self._default is not None:
+            i = self._merge_tools.index(self._default)
+            old_index = self.createIndex(i, i)
+        new_index = None
+        if new_default is not None:
+            i = self._merge_tools.index(new_default)
+            new_index = self.createIndex(i, i)
+            # new_default may be == to an instance in self._merge_tools but not
+            # the same instance. To preserve editing semantics in this model,
+            # self._default must refer to an instance in self._merge_tools, so
+            # we find the == instance in self._merge_tools and set self._default
+            # to that instead.
+            for mt in self._merge_tools:
+                if mt == default:
+                    self._default = mt
+                    break
+        else:
+            self._default = None
+        if old_index:
+            self.emit(QtCore.SIGNAL("dataChanged(QModelIndex,QModelIndex)"),
+                      old_index, old_index)
+        if new_index:
+            self.emit(QtCore.SIGNAL("dataChanged(QModelIndex,QModelIndex)"),
+                      new_index, new_index)
+        
+    def get_merge_tool(self, index):
+        return self._merge_tools[index.row()]
+        
+    def index_of(self, tool):
+        return self.createIndex(self._merge_tools.index(tool), 0)
+        
+    def new_merge_tool(self):
+        index = self.createIndex(len(self._merge_tools), 0)
+        self.beginInsertRows(QtCore.QModelIndex(), index.row(), index.row())
+        self._merge_tools.append(mergetools.MergeTool(
+            gettext('New Merge Tool'), ''))
+        self.endInsertRows()
+        return index
+        
+    def remove_merge_tool(self, index):
+        self.beginRemoveRows(QtCore.QModelIndex(), index.row(), index.row())
+        if self._merge_tools[index.row()] == self._default:
+            self._default = None
+        del self._merge_tools[index.row()]
+        self.endRemoveRows()
+        
+    def detect_merge_tools(self):
+        detected_tools = mergetools.detect_merge_tools()
+        for mt in detected_tools:
+            if mt not in self._merge_tools:
+                row = len(self._merge_tools)
+                self.beginInsertRows(QtCore.QModelIndex(), row, row)
+                self._merge_tools.append(mt)
+                self.endInsertRows()
+        self.sort(0, Qt.AscendingOrder)
+        
+    def set_merge_tool_name(self, index, name):
+        self._merge_tools[index.row()].set_name(name)
+        self.emit(QtCore.SIGNAL("dataChanged(QModelIndex,QModelIndex)"), index,
+                  index)
+        
+    def set_merge_tool_commandline(self, index, commandline):
+        self._merge_tools[index.row()].set_commandline(commandline)
+        
+    def set_merge_tool_executable(self, index, executable):
+        self._merge_tools[index.row()].set_executable(executable)
+        
+    def rowCount(self, parent):
+        return len(self._merge_tools)
+    
+    def data(self, index, role):
+        if role == Qt.DisplayRole:
+            name = self._merge_tools[index.row()].get_name()
+            if self._default is not None and self._default.get_name() == name:
+                name = '%s (%s)' % (name, gettext('default'))
+            return QtCore.QVariant(name)
+        elif role == Qt.EditRole:
+            return QtCore.QVariant(self._merge_tools[index.row()].get_name())
+        return QtCore.QVariant()
+        
+    def setData(self, index, value, role):
+        if role == Qt.EditRole:
+            self._merge_tools[index.row()].set_name(unicode(value.toString()))
+            self.emit(QtCore.SIGNAL("dataChanged(QModelIndex,QModelIndex)"),
+                      index,index)
+            self.sort(0, Qt.AscendingOrder)
+            return True
+        return False
+        
+    def flags(self, index):
+        return super(MergeToolsListModel, self).flags(index) | Qt.ItemIsEditable
+
+    def sort(self, column, sortOrder):
+        self.emit(QtCore.SIGNAL("layoutAboutToBeChanged()"))
+        index_map = self._merge_tools[:] # copy
+        self._merge_tools.sort()
+        for i in range(0, len(index_map)):
+            index_map[i] = self._merge_tools.index(index_map[i])
+        from_list = [self.createIndex(i, 0) for i in index_map]
+        to_list = [self.createIndex(i, 0) for i in range(0, len(index_map))]
+        self.changePersistentIndexList(from_list, to_list)
+        self.emit(QtCore.SIGNAL("layoutChanged()"))
 
=== modified file 'lib/conflicts.py'
--- lib/conflicts.py	2010-09-01 07:30:29 +0000
+++ lib/conflicts.py	2010-11-09 00:10:03 +0000
@@ -20,6 +20,7 @@
 from PyQt4 import QtCore, QtGui
 from bzrlib.config import GlobalConfig
 from bzrlib.conflicts import resolve
+from bzrlib import mergetools
 from bzrlib.workingtree import WorkingTree
 from bzrlib.plugins.qbzr.lib.i18n import gettext, N_, ngettext
 from bzrlib.plugins.qbzr.lib.util import (
@@ -58,7 +59,7 @@
         self.connect(
             self.conflicts_list.selectionModel(),
             QtCore.SIGNAL("selectionChanged(QItemSelection, QItemSelection)"),
-            self.update_selection)
+            self.update_merge_tool_ui)
         self.connect(
             self.conflicts_list,
             QtCore.SIGNAL("customContextMenuRequested(QPoint)"),
@@ -77,22 +78,14 @@
         vbox.addWidget(self.conflicts_list)
 
         hbox = QtGui.QHBoxLayout()
-        self.program_edit = QtGui.QLineEdit(self)
-        self.program_edit.setEnabled(False)
-        self.connect(
-            self.program_edit,
-            QtCore.SIGNAL("textChanged(QString)"),
-            self.check_merge_tool_edit)
-        self.program_extmerge_default_button = QtGui.QCheckBox(gettext("Use Configured Default"))
-        self.program_extmerge_default_button.setToolTip(gettext(
-                "The merge tool configured in qconfig under Merge' file.\n"
-                "It follows the convention used in the bzr plugin: extmerge\n"
-                "external_merge = kdiff3 --output %r %b %t %o\n"
-                "%r is output, %b is .BASE, %t is .THIS and %o is .OTHER file."))
-        self.connect(
-            self.program_extmerge_default_button,
-            QtCore.SIGNAL("clicked()"),
-            self.program_extmerge_default_clicked)
+        self.merge_tools_combo = QtGui.QComboBox(self)
+        self.merge_tools_combo.setEditable(False)
+        self.connect(self.merge_tools_combo,
+                     QtCore.SIGNAL("currentIndexChanged(int)"),
+                     self.update_merge_tool_ui)
+        
+        self.merge_tool_error = QtGui.QLabel('', self)
+        
         self.program_launch_button = QtGui.QPushButton(gettext("&Launch..."), self)
         self.program_launch_button.setEnabled(False)
         self.connect(
@@ -100,10 +93,11 @@
             QtCore.SIGNAL("clicked()"),
             self.launch_merge_tool)
         self.program_label = QtGui.QLabel(gettext("M&erge tool:"), self)
-        self.program_label.setBuddy(self.program_edit)
+        self.program_label.setBuddy(self.merge_tools_combo)
         hbox.addWidget(self.program_label)
-        hbox.addWidget(self.program_edit)
-        hbox.addWidget(self.program_extmerge_default_button)
+        hbox.addWidget(self.merge_tools_combo)
+        hbox.addWidget(self.merge_tool_error)
+        hbox.addStretch(1)
         hbox.addWidget(self.program_launch_button)
         vbox.addLayout(hbox)
 
@@ -125,21 +119,13 @@
         self.initialize_ui()        
 
     def initialize_ui(self):
-        merge_tool_extmerge = get_qbzr_config().get_option("merge_tool_extmerge")
-        
-        self.program_extmerge_default_button.setCheckState(QtCore.Qt.Unchecked)
-        if merge_tool_extmerge in ("True", "1"):
-            self.program_extmerge_default_button.setCheckState(QtCore.Qt.Checked)
-        self.program_extmerge_default_clicked()
-        enabled, error_msg = self.is_merge_tool_launchable()
-        self.update_program_edit_text(enabled, error_msg)
-        # if extmerge not configured then resort to using default
-        if not enabled and self.program_extmerge_default_button.isChecked():
-            bzr_config = GlobalConfig()
-            extmerge_tool = bzr_config.get_user_option("external_merge")
-            if not extmerge_tool:
-                self.program_extmerge_default_button.setCheckState(QtCore.Qt.Unchecked)
-                self.update_program_edit_text(False, "")
+        defined_tools = mergetools.get_merge_tools()
+        default_tool = mergetools.get_default_merge_tool()
+        for merge_tool in defined_tools:
+            self.merge_tools_combo.insertItem(self.merge_tools_combo.count(), merge_tool.get_name())
+        if default_tool is not None:
+            self.merge_tools_combo.setCurrentIndex(self.merge_tools_combo.findText(default_tool.get_name()))
+        self.update_merge_tool_ui()
 
     def create_context_menu(self):
         self.context_menu = QtGui.QMenu(self.conflicts_list)
@@ -190,56 +176,31 @@
                 gettext('&OK'))
             self.close()
 
-    def update_selection(self, selected, deselected):
+    def update_merge_tool_ui(self):
         enabled, error_msg = self.is_merge_tool_launchable()
-        self.program_edit.setEnabled(enabled and not self.program_extmerge_default_button.isChecked())
+        self.merge_tool_error.setText(error_msg)
         self.program_launch_button.setEnabled(enabled)
-        self.update_program_edit_text(enabled, error_msg)
-        if enabled and self.program_extmerge_default_button.isChecked():
-            self.program_edit.setEnabled(False)
         self.merge_action.setEnabled(enabled)
 
-    def check_merge_tool_edit(self, text):
-        enabled, error_msg = self.is_merge_tool_launchable()
-        self.program_launch_button.setEnabled(enabled)
-
     def launch_merge_tool(self):
         items = self.conflicts_list.selectedItems()
         enabled, error_msg = self.is_merge_tool_launchable()
         if not enabled:
             return
-        merge_tool = unicode(self.program_edit.text()).strip()
-        if not merge_tool:
-            return
+        merge_tool = mergetools.find_merge_tool(self.merge_tools_combo.currentText())
         file_id = str(items[0].data(0, QtCore.Qt.UserRole).toString())
         file_name = self.wt.abspath(self.wt.id2path(file_id))
-        base_file_name = file_name + ".BASE"
-        this_file_name = file_name + ".THIS"
-        other_file_name = file_name + ".OTHER"
-        new_args = [base_file_name, this_file_name, other_file_name]
-        config = get_qbzr_config()
-        config.set_option("merge_tool_extmerge", False)
-
-        if self.program_extmerge_default_button.isChecked():
-            bzr_config = GlobalConfig()
-            extmerge_tool = bzr_config.get_user_option("external_merge")
-            args = cmdline_split(extmerge_tool)
-            new_args = args[1:len(args)]
-            i = 0
-            while i < len(new_args):
-                new_args[i] = new_args[i].replace('%r', file_name)
-                new_args[i] = new_args[i].replace('%o', other_file_name)
-                new_args[i] = new_args[i].replace('%b', base_file_name)
-                new_args[i] = new_args[i].replace('%t', this_file_name)
-                i = i + 1
-            merge_tool = args[0]
-            config.set_option("merge_tool_extmerge", True)
-        else:
-            config.set_option("merge_tool", merge_tool)
-
         process = QtCore.QProcess(self)
-        self.connect(process, QtCore.SIGNAL("error(QProcess::ProcessError)"), self.show_merge_tool_error)
-        process.start(merge_tool, new_args)
+        def qprocess_invoker(executable, args, cleanup):
+            def qprocess_error(error):
+                self.show_merge_tool_error(error)
+                cleanup(process.exitCode())
+            def qprocess_finished(exit_code, exit_status):
+                cleanup(exit_code)
+            self.connect(process, QtCore.SIGNAL("error(QProcess::ProcessError)"), qprocess_error)
+            self.connect(process, QtCore.SIGNAL("finished(int,QProcess::ExitStatus)"), qprocess_finished)
+            process.start(executable, args)
+        merge_tool.invoke(file_name, qprocess_invoker)
 
     def show_merge_tool_error(self, error):
         msg = gettext("Error while running merge tool (code %d)") % error
@@ -261,34 +222,20 @@
     def show_context_menu(self, pos):
         self.context_menu.popup(self.conflicts_list.viewport().mapToGlobal(pos))
 
-    def program_extmerge_default_clicked(self):	
-        enabled, error_msg = self.is_merge_tool_launchable()
-        self.program_edit.setEnabled(enabled and not self.program_extmerge_default_button.isChecked())
-        self.program_launch_button.setEnabled(enabled)
-        self.update_program_edit_text(enabled, error_msg)
-        config = get_qbzr_config()
-        config.set_option("merge_tool_extmerge", 
-                          self.program_extmerge_default_button.isChecked()) 
-
     def is_merge_tool_launchable(self):
         items = self.conflicts_list.selectedItems()
         error_msg = ""
         enabled = True
         if len(items) != 1 or items[0].data(1, QtCore.Qt.UserRole).toString() != "text conflict":
             enabled = False
-
-        # check to see if the extmerge config is correct   
-        if self.program_extmerge_default_button.isChecked():
-            bzr_config = GlobalConfig()
-            extmerge_tool = bzr_config.get_user_option("external_merge")
-            if not extmerge_tool:
-                error_msg = gettext("Set up external_merge app in qconfig under the Merge tab")
-                enabled = False
-                return enabled, error_msg
-            error = self.is_extmerge_definition_valid(False)
-            if len(error) > 0:
-                enabled = False
-                error_msg = error
+        merge_tool = mergetools.find_merge_tool(self.merge_tools_combo.currentText())
+        if merge_tool is None:
+            error_msg = gettext("Set up external_merge app in qconfig under the Merge tab")
+            enabled = False
+        elif not merge_tool.is_available():
+            enabled = False
+            error_msg = gettext("External merge tool %(tool)s is not available") % \
+                    { 'tool': merge_tool.get_name() }
         return enabled, error_msg
 
     def is_extmerge_definition_valid(self, showErrorDialog):
@@ -316,17 +263,6 @@
             return gettext("Missing the flag: %s. Configure in qconfig under the merge tab.") % flags
         return ""
 
-    def update_program_edit_text(self, enabled, error_msg):
-        if self.program_extmerge_default_button.isChecked():
-            if enabled or (len(error_msg) <= 0):
-                config = GlobalConfig()
-                extmerge = config.get_user_option("external_merge")
-                self.program_edit.setText(gettext("%s (Configured external merge definition in qconfig)") % extmerge)
-            else:
-                self.program_edit.setText(error_msg)
-        else:
-            config = get_qbzr_config()
-            self.program_edit.setText((config.get_user_option("merge_tool") or "").strip() or "meld")
 
 if 0:
     N_("path conflict")
 
=== added file 'lib/ui_merge_config.py'
--- lib/ui_merge_config.py	1970-01-01 00:00:00 +0000
+++ lib/ui_merge_config.py	2010-11-09 00:10:03 +0000
@@ -0,0 +1,97 @@
+# -*- coding: utf-8 -*-
+
+# Form implementation generated from reading ui file 'ui/merge_config.ui'
+#
+# Created: Sun Nov 07 17:00:49 2010
+#      by: PyQt4 UI code generator 4.7.2
+#
+# WARNING! All changes made in this file will be lost!
+
+from PyQt4 import QtCore, QtGui
+from bzrlib.plugins.qbzr.lib.i18n import gettext
+
+
+class Ui_MergeConfig(object):
+    def setupUi(self, MergeConfig):
+        MergeConfig.setObjectName("MergeConfig")
+        MergeConfig.resize(626, 297)
+        self.verticalLayout_2 = QtGui.QVBoxLayout(MergeConfig)
+        self.verticalLayout_2.setObjectName("verticalLayout_2")
+        self.groupBox = QtGui.QGroupBox(MergeConfig)
+        self.groupBox.setObjectName("groupBox")
+        self.verticalLayout_3 = QtGui.QVBoxLayout(self.groupBox)
+        self.verticalLayout_3.setObjectName("verticalLayout_3")
+        self.splitter = QtGui.QSplitter(self.groupBox)
+        self.splitter.setOrientation(QtCore.Qt.Horizontal)
+        self.splitter.setChildrenCollapsible(False)
+        self.splitter.setObjectName("splitter")
+        self.layoutWidget = QtGui.QWidget(self.splitter)
+        self.layoutWidget.setObjectName("layoutWidget")
+        self.verticalLayout = QtGui.QVBoxLayout(self.layoutWidget)
+        self.verticalLayout.setObjectName("verticalLayout")
+        self.merge_tools_list = QtGui.QListView(self.layoutWidget)
+        self.merge_tools_list.setObjectName("merge_tools_list")
+        self.verticalLayout.addWidget(self.merge_tools_list)
+        self.merge_tools_buttons = QtGui.QWidget(self.layoutWidget)
+        self.merge_tools_buttons.setObjectName("merge_tools_buttons")
+        self.horizontalLayout = QtGui.QHBoxLayout(self.merge_tools_buttons)
+        self.horizontalLayout.setObjectName("horizontalLayout")
+        spacerItem = QtGui.QSpacerItem(40, 20, QtGui.QSizePolicy.Expanding, QtGui.QSizePolicy.Minimum)
+        self.horizontalLayout.addItem(spacerItem)
+        self.merge_tools_add = QtGui.QPushButton(self.merge_tools_buttons)
+        self.merge_tools_add.setObjectName("merge_tools_add")
+        self.horizontalLayout.addWidget(self.merge_tools_add)
+        self.merge_tools_remove = QtGui.QPushButton(self.merge_tools_buttons)
+        self.merge_tools_remove.setObjectName("merge_tools_remove")
+        self.horizontalLayout.addWidget(self.merge_tools_remove)
+        self.merge_tools_detect = QtGui.QPushButton(self.merge_tools_buttons)
+        self.merge_tools_detect.setObjectName("merge_tools_detect")
+        self.horizontalLayout.addWidget(self.merge_tools_detect)
+        spacerItem1 = QtGui.QSpacerItem(40, 20, QtGui.QSizePolicy.Expanding, QtGui.QSizePolicy.Minimum)
+        self.horizontalLayout.addItem(spacerItem1)
+        self.verticalLayout.addWidget(self.merge_tools_buttons)
+        self.merge_tool_details = QtGui.QWidget(self.splitter)
+        self.merge_tool_details.setMinimumSize(QtCore.QSize(300, 0))
+        self.merge_tool_details.setObjectName("merge_tool_details")
+        self.gridLayout = QtGui.QGridLayout(self.merge_tool_details)
+        self.gridLayout.setContentsMargins(-1, 0, -1, 0)
+        self.gridLayout.setObjectName("gridLayout")
+        self.label = QtGui.QLabel(self.merge_tool_details)
+        self.label.setObjectName("label")
+        self.gridLayout.addWidget(self.label, 0, 0, 1, 1)
+        self.merge_tool_name = QtGui.QLineEdit(self.merge_tool_details)
+        self.merge_tool_name.setObjectName("merge_tool_name")
+        self.gridLayout.addWidget(self.merge_tool_name, 1, 0, 1, 1)
+        self.label_2 = QtGui.QLabel(self.merge_tool_details)
+        self.label_2.setObjectName("label_2")
+        self.gridLayout.addWidget(self.label_2, 2, 0, 1, 1)
+        self.merge_tool_commandline = QtGui.QLineEdit(self.merge_tool_details)
+        self.merge_tool_commandline.setObjectName("merge_tool_commandline")
+        self.gridLayout.addWidget(self.merge_tool_commandline, 3, 0, 1, 1)
+        self.merge_tool_browse = QtGui.QToolButton(self.merge_tool_details)
+        self.merge_tool_browse.setObjectName("merge_tool_browse")
+        self.gridLayout.addWidget(self.merge_tool_browse, 3, 1, 1, 1)
+        spacerItem2 = QtGui.QSpacerItem(248, 130, QtGui.QSizePolicy.Minimum, QtGui.QSizePolicy.Expanding)
+        self.gridLayout.addItem(spacerItem2, 6, 0, 1, 1)
+        self.merge_tool_default = QtGui.QCheckBox(self.merge_tool_details)
+        self.merge_tool_default.setObjectName("merge_tool_default")
+        self.gridLayout.addWidget(self.merge_tool_default, 4, 0, 1, 1)
+        self.verticalLayout_3.addWidget(self.splitter)
+        self.verticalLayout_2.addWidget(self.groupBox)
+        self.label.setBuddy(self.merge_tool_name)
+        self.label_2.setBuddy(self.merge_tool_commandline)
+
+        self.retranslateUi(MergeConfig)
+        QtCore.QMetaObject.connectSlotsByName(MergeConfig)
+
+    def retranslateUi(self, MergeConfig):
+        MergeConfig.setWindowTitle(gettext("External Merge Tools"))
+        self.groupBox.setTitle(gettext("External Merge Tools"))
+        self.merge_tools_add.setText(gettext("Add"))
+        self.merge_tools_remove.setText(gettext("Remove"))
+        self.merge_tools_detect.setText(gettext("Detect"))
+        self.label.setText(gettext("Name:"))
+        self.label_2.setText(gettext("Command line:"))
+        self.merge_tool_browse.setText(gettext("..."))
+        self.merge_tool_default.setText(gettext("Default"))
+
 
=== added file 'ui/merge_config.ui'
--- ui/merge_config.ui	1970-01-01 00:00:00 +0000
+++ ui/merge_config.ui	2010-11-09 00:10:03 +0000
@@ -0,0 +1,169 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
+ <class>MergeConfig</class>
+ <widget class="QWidget" name="MergeConfig">
+  <property name="geometry">
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>626</width>
+    <height>297</height>
+   </rect>
+  </property>
+  <property name="windowTitle">
+   <string>External Merge Tools</string>
+  </property>
+  <layout class="QVBoxLayout" name="verticalLayout_2">
+   <item>
+    <widget class="QGroupBox" name="groupBox">
+     <property name="title">
+      <string>External Merge Tools</string>
+     </property>
+     <layout class="QVBoxLayout" name="verticalLayout_3">
+      <item>
+       <widget class="QSplitter" name="splitter">
+        <property name="orientation">
+         <enum>Qt::Horizontal</enum>
+        </property>
+        <property name="childrenCollapsible">
+         <bool>false</bool>
+        </property>
+        <widget class="QWidget" name="layoutWidget">
+         <layout class="QVBoxLayout" name="verticalLayout">
+          <item>
+           <widget class="QListView" name="merge_tools_list"/>
+          </item>
+          <item>
+           <widget class="QWidget" name="merge_tools_buttons" native="true">
+            <layout class="QHBoxLayout" name="horizontalLayout">
+             <item>
+              <spacer name="spacer">
+               <property name="orientation">
+                <enum>Qt::Horizontal</enum>
+               </property>
+               <property name="sizeHint" stdset="0">
+                <size>
+                 <width>40</width>
+                 <height>20</height>
+                </size>
+               </property>
+              </spacer>
+             </item>
+             <item>
+              <widget class="QPushButton" name="merge_tools_add">
+               <property name="text">
+                <string>Add</string>
+               </property>
+              </widget>
+             </item>
+             <item>
+              <widget class="QPushButton" name="merge_tools_remove">
+               <property name="text">
+                <string>Remove</string>
+               </property>
+              </widget>
+             </item>
+             <item>
+              <widget class="QPushButton" name="merge_tools_detect">
+               <property name="text">
+                <string>Detect</string>
+               </property>
+              </widget>
+             </item>
+             <item>
+              <spacer name="spacer_2">
+               <property name="orientation">
+                <enum>Qt::Horizontal</enum>
+               </property>
+               <property name="sizeHint" stdset="0">
+                <size>
+                 <width>40</width>
+                 <height>20</height>
+                </size>
+               </property>
+              </spacer>
+             </item>
+            </layout>
+           </widget>
+          </item>
+         </layout>
+        </widget>
+        <widget class="QWidget" name="merge_tool_details" native="true">
+         <property name="minimumSize">
+          <size>
+           <width>300</width>
+           <height>0</height>
+          </size>
+         </property>
+         <layout class="QGridLayout" name="gridLayout">
+          <property name="topMargin">
+           <number>0</number>
+          </property>
+          <property name="bottomMargin">
+           <number>0</number>
+          </property>
+          <item row="0" column="0">
+           <widget class="QLabel" name="label">
+            <property name="text">
+             <string>Name:</string>
+            </property>
+            <property name="buddy">
+             <cstring>merge_tool_name</cstring>
+            </property>
+           </widget>
+          </item>
+          <item row="1" column="0">
+           <widget class="QLineEdit" name="merge_tool_name"/>
+          </item>
+          <item row="2" column="0">
+           <widget class="QLabel" name="label_2">
+            <property name="text">
+             <string>Command line:</string>
+            </property>
+            <property name="buddy">
+             <cstring>merge_tool_commandline</cstring>
+            </property>
+           </widget>
+          </item>
+          <item row="3" column="0">
+           <widget class="QLineEdit" name="merge_tool_commandline"/>
+          </item>
+          <item row="3" column="1">
+           <widget class="QToolButton" name="merge_tool_browse">
+            <property name="text">
+             <string>...</string>
+            </property>
+           </widget>
+          </item>
+          <item row="6" column="0">
+           <spacer name="spacer_3">
+            <property name="orientation">
+             <enum>Qt::Vertical</enum>
+            </property>
+            <property name="sizeHint" stdset="0">
+             <size>
+              <width>248</width>
+              <height>130</height>
+             </size>
+            </property>
+           </spacer>
+          </item>
+          <item row="4" column="0">
+           <widget class="QCheckBox" name="merge_tool_default">
+            <property name="text">
+             <string>Default</string>
+            </property>
+           </widget>
+          </item>
+         </layout>
+        </widget>
+       </widget>
+      </item>
+     </layout>
+    </widget>
+   </item>
+  </layout>
+ </widget>
+ <resources/>
+ <connections/>
+</ui>