Merge lp:~vila/bzr/conflict-manager into lp:bzr

[Vincent Ladeuil]

[Vincent Ladeuil (vila)]

This patch implements some simple actions to resolve tree shape conflicts (that is, all conflicts that are not text conflicts) by providing --keep-mine and --take-their
to the resolve command.
Just marking the conflict as resolved is still accessible via the --done default action.

It also documents (mostly via hidden comments in documentation, TODOs and FIXMEs in the code)
the next steps I plan to work on.

But since this is already a significant patch, I'd like feedback before going further.

[Aaron Bentley (abentley)]

This looks a bit weird to me. Haven't done a full review, but for two-way conflicts, bzr takes the changes from OTHER, so keep_mine should never be a no-op, and take_their should often be a no-op.

take_their should actually be take_theirs. ("my" is to "their" as "mine" is to "theirs".)

Also, the safest way to rename multiple files at once is using a TreeTransform.

There are some grammar issues in the proposed new text:
+may conflict with the current state of your working tree.
+
+When conflicts are present in your working tree (as shown by ``bzr
+conflicts``), you should resolve them and then inform bzr that the conflicts
+has been resolved.

"have been resolved"

+
+Resolving conflicts is sometimes not obvious. Either because the user that
+should resolve them is not the one responsible for their occurrence, as is the
+case when merging other people work or because some conflicts are presented in

"other people's work"

+a way that is not easy to understand.
+
+Bazaar try to avoid conflicts, its aim is to ask you to resolve the conflict

"Bazaar tries to avoid conflicts; its aim is to ask you to resolve the conflict"

+if and only if there's an actual conceptual conflict in the source tree.
+Because Bazaar doesn't understand the real meaning of the files being
+versioned it can, when faced with ambiguities, fall short in either direction

"versioned,"

(But it may be better to restructure the sentence instead of adding commas.)

+trying to resolve the conflict itself. Many kinds of changes can be combined
+programmatically, but sometimes only a human can determine the right thing to
+do.
+
+When Bazaar generates a conflict, it adds information into the working tree to
+present the conflicting versions and it's up to you to find the correct

"versions,"

+resolution.
+
+Whatever the conflict is, resolving it is roughly done in two steps:
+
+- modify the working tree content so that the conflicted item is now in the
+ state you want to keep,
+
+- inform Bazaar that the conflict is now solved and ask to cleanup any
+ remaining generated information (``bzr resolve <item>``).
+
+For most conflict types, there are some obvious ways to modify the working
+tree and put it into the desired state. For some types of conflicts, Bazaar
+itself already made a choice when possible.

I think "for some types...when possible" is too cautious, but at any rate, "Bazaar itself already made a choice when possible" is ungrammatical. It should be "Bazaar itself will have already made a choice, when possible"

+Yet, whether Bazaar made a choice or not, there are some other simple but
+alternative ways to resolve the conflict.

"Bazaar makes a choice"

("alternative" tastes funny here. Maybe "different"?)

+Various actions are available depending on the kind of conflict, for some of
+these actions, Bazaar can provide some help. In the end you should at least
+inform Bazaar that you're done with the conflict with::
+
+ ``bzr resolve FILE --action=done'
+
+Note that this is the default action when a single file is involved so you can
+simply use::
+
+ ``bzr resolve FILE``

Why tell them to use --action=done and then immediately contradict yourself? And w...

[John A Meinel (jameinel)]

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

Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/conflict-manager into lp:bzr with lp:~vila/bzr/cleanup-conflicts as a prerequisite.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> This patch implements some simple actions to resolve tree shape conflicts (that is, all conflicts that are not text conflicts) by providing --keep-mine and --take-their
> to the resolve command.
> Just marking the conflict as resolved is still accessible via the --done default action.

Meta-comment

We seem a bit confused as to whether 'bzr resolve' is a command that has
actions, or whether it is a command that records actions you've already
done. I'm worried that this takes it a step further. (resolve
- --interactive is quite different from what 'resolve' does today.)

I'm wondering if we should be considering how to split out the current
functionality into separate commands...

John
=:->

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

iEYEARECAAYFAktCTvwACgkQJdeBCYSNAAN5rgCggVpL7OvYmZSUwPFJ0/UHfjLq
N14An0NCsQrKF6+Q6+7xR++1F2y2CB/2
=1KQw
-----END PGP SIGNATURE-----

[Vincent Ladeuil (vila)]

>>>>> "Aaron" == Aaron Bentley <email address hidden> writes:

    Aaron> This looks a bit weird to me. Haven't done a full
    Aaron> review, but for two-way conflicts, bzr takes the
    Aaron> changes from OTHER, so keep_mine should never be a
    Aaron> no-op, and take_their should often be a no-op.

I didn't touch the code that generate the conflicts, so any
incoherency there was there. The overall code is already hard to
modify so I don't intend to change that sort of thing, yet.

But, yes, I was surprised to find that we don't always act the
same way when faced with conflicts and I'd like to provide a more
"regular" behavior (like using .THIS .OTHER instead of .new or
.diverted for example).

Also, if there is a conflict, it sounds acceptable that *both*
keep_mine and take_theirs should not be no-ops... with the
down-side that this will left the working tree in a worse state
than choosing between THIS or OTHER.

I'd welcome any feedback about why we do things the way they are
done today !

    Aaron> take_their should actually be take_theirs. ("my" is
    Aaron> to "their" as "mine" is to "theirs".)

Damn, I wrote it --take-theirs first :)

    Aaron> Also, the safest way to rename multiple files at once
    Aaron> is using a TreeTransform.

Thanks. I think there is a single place that deserves that, I'll check.

    Aaron> There are some grammar issues in the proposed new
    Aaron> text:

Thanks, I addressed them in my branch and only answer other
points below.

<snip/>

    Aaron> +Various actions are available depending on the kind of conflict, for some of
    Aaron> +these actions, Bazaar can provide some help. In the end you should at least
    Aaron> +inform Bazaar that you're done with the conflict with::
    Aaron> +
    Aaron> + ``bzr resolve FILE --action=done'
    Aaron> +
    Aaron> +Note that this is the default action when a single file is involved so you can
    Aaron> +simply use::
    Aaron> +
    Aaron> + ``bzr resolve FILE``

    Aaron> Why tell them to use --action=done and then
    Aaron> immediately contradict yourself?

Because the former is explicit why the later is implicit (and
don't contradict each other). I try to always use the explicit
form in docs.

    Aaron> And why --action=done instead of --done ?

Same.

    Aaron> +When you have resolved text conflicts, just run ``bzr
    Aaron> +resolve --auto``, and Bazaar will auto-detect which
    Aaron> +conflicts you have resolved.

    Aaron> Requiring --auto is a regression IMO.

Hehe, this text doesn't imply that --auto is *required* it just
mention it explicitly (same rule as above).

Well, I didn't change the behavior of --auto (finally), but I
intend to. The code that still implements the magic behavior of
--auto. But as said, I find that behavior pretty surprising and
I think we should get rid of it.

141 + # FIXME: There is a special case here related to the option
142 + # handling that could be clearer and easier to discover by
143 + # providing an --auto action (bug #344013 and #383396) and
144 + # make it mandatory instead of implicit and active only
145 + # when no file_list is provided -- vila 091229

If this patch is accepted then --aut...

[Vincent Ladeuil (vila)]

>>>>> "jam" == John A Meinel <email address hidden> writes:

<snip/>

    jam> We seem a bit confused as to whether 'bzr resolve' is a
    jam> command that has actions, or whether it is a command
    jam> that records actions you've already done.

Yes. It's surprising. But 'resolved', which reflects more closely
the actual behavior is already declared as an alias.

    jam> I'm worried that this takes it a step further. (resolve
    jam> --interactive is quite different from what 'resolve'
    jam> does today.)

Well, it literally resolves the conflict instead of just deleting
it (and I abandon the --interactive idea in favor of handling a
list of files (as resolve was already), GUIs may choose
differently but at least the command line UI provides the basis).

So it makes sense to use 'resolve' for the actions and 'resolved'
for just marking the conflicts as resolved.

But then, in 99% of the actions you also want to mark the
conflict as resolved !

In the end, I considered that I was providing more ways to
resolve the conflicts than the actual --done action.

    jam> I'm wondering if we should be considering how to split
    jam> out the current functionality into separate commands...

Well, we currently have: bzr resolve [--auto] {--all|FILE+}.

There is not much that will be lost except that the magic
behavior associated with --auto (disabled when you use --all)
will become explicit.

Do you have better names to propose ?

   Vincent

[Ian Clatworthy (ian-clatworthy)]

I started reviewing this the other day and didn't get a long way. I'm not sure I'm best qualified to comment on the bigger picture here: I'd like Aaron to sign off on that before this lands.

I'm more than happy to review the code for logic bugs and the doc for readability if that helps.

[Vincent Ladeuil (vila)]

Revno 4668 takes review comments into account:
- settle on using --take-this/--take-other instead of --keep-mine/--take-theirs,
- stop using rest in conflict-types.txt since it's displayed by
  ``bzr help conflict-types`` without processing.

Overall, it doesn't modify any existing behaviour but adds new features
on ``bzr resolve``.

The next steps are described at
 http://wiki.bazaar.canonical.com/VincentLadeuil/ConflictResolution

Discussions may still be needed before the next submissions but this patch
itself shouldn't be controversial.

[John A Meinel (jameinel)]

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

Vincent Ladeuil wrote:
> Revno 4668 takes review comments into account:
> - settle on using --take-this/--take-other instead of --keep-mine/--take-theirs,
> - stop using rest in conflict-types.txt since it's displayed by
> ``bzr help conflict-types`` without processing.

I would probably have left in the ReST syntax, since it is how other
bits are formatted, and leave to a separate action creating a plain-text
formatter for the rest documentation. (help strings, etc are all already
ReST or meant to be ReST, so we need to handle it anyway.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAktplFsACgkQJdeBCYSNAAO8BgCfeC/fFAMUm0XGyrDKJRwdnlLy
sUwAnjX6ZYNufZNRIVZ9veuWZSXwHmt5
=h1WN
-----END PGP SIGNATURE-----

[Vincent Ladeuil (vila)]

>>>>> "jam" == John A Meinel <email address hidden> writes:

    jam> Vincent Ladeuil wrote:
    >> Revno 4668 takes review comments into account:
    >> - settle on using --take-this/--take-other instead of --keep-mine/--take-theirs,
    >> - stop using rest in conflict-types.txt since it's displayed by
    >> ``bzr help conflict-types`` without processing.

    jam> I would probably have left in the ReST syntax, since it is how other
    jam> bits are formatted, and leave to a separate action creating a plain-text
    jam> formatter for the rest documentation. (help strings, etc are all already
    jam> ReST or meant to be ReST, so we need to handle it anyway.)

And spit rest bits to the user when doing `bzr help conflict-types` ???

[John A Meinel (jameinel)]

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

Vincent Ladeuil wrote:
>>>>>> "jam" == John A Meinel <email address hidden> writes:
>
> jam> Vincent Ladeuil wrote:
> >> Revno 4668 takes review comments into account:
> >> - settle on using --take-this/--take-other instead of --keep-mine/--take-theirs,
> >> - stop using rest in conflict-types.txt since it's displayed by
> >> ``bzr help conflict-types`` without processing.
>
> jam> I would probably have left in the ReST syntax, since it is how other
> jam> bits are formatted, and leave to a separate action creating a plain-text
> jam> formatter for the rest documentation. (help strings, etc are all already
> jam> ReST or meant to be ReST, so we need to handle it anyway.)
>
> And spit rest bits to the user when doing `bzr help conflict-types` ???

Until we fix 'bzr help' we already do that for lots of other ones.

'bzr help init' shows a '::'
'bzr help init-repo' as well
'bzr help diff'
'bzr help log'

etc, etc, etc.

There is an open bug that we should use a plain-text formatter when
writing out ReST code, I'd rather fix that, then not have use use ReST
for help which gets translated into our HTML documentation.

John
=:->

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

iEYEARECAAYFAktpxSkACgkQJdeBCYSNAAMfCQCeJOtTHmoD7iy+nmhXH80AXD7I
8mgAoNV98mCOlfe2/62SdBnXFvGN47vF
=WUOX
-----END PGP SIGNATURE-----

[Ian Clatworthy (ian-clatworthy)]

John A Meinel wrote:
> Vincent Ladeuil wrote:

>> And spit rest bits to the user when doing `bzr help conflict-types` ???
>
> Until we fix 'bzr help' we already do that for lots of other ones.
>
> 'bzr help init' shows a '::'
> 'bzr help init-repo' as well
> 'bzr help diff'
> 'bzr help log'
>
> etc, etc, etc.
>
> There is an open bug that we should use a plain-text formatter when
> writing out ReST code, I'd rather fix that, then not have use use ReST
> for help which gets translated into our HTML documentation.

Some limited ReST translations gets done when outputting text help
currently, e.g.

:: at the ends of a line gets mapped to :
:doc:`xxx-help` gets mapped to `bzr help xxx`

We don't bundle DocTools or Sphinx but it is possible to write text
that is reasonable in both text output and html output given the rules
already in place. I'm sure we could tweak them further though.

Ian C.

[Vincent Ladeuil (vila)]

>>>>> "Ian" == Ian Clatworthy <email address hidden> writes:

    Ian> John A Meinel wrote:
    >> Vincent Ladeuil wrote:

    >>> And spit rest bits to the user when doing `bzr help conflict-types` ???
    >>
    >> Until we fix 'bzr help' we already do that for lots of other ones.
    >>
    >> 'bzr help init' shows a '::'
    >> 'bzr help init-repo' as well
    >> 'bzr help diff'
    >> 'bzr help log'
    >>
    >> etc, etc, etc.
    >>
    >> There is an open bug that we should use a plain-text formatter when
    >> writing out ReST code, I'd rather fix that, then not have use use ReST
    >> for help which gets translated into our HTML documentation.

    Ian> Some limited ReST translations gets done when outputting text help
    Ian> currently, e.g.

    Ian> :: at the ends of a line gets mapped to :
    Ian> :doc:`xxx-help` gets mapped to `bzr help xxx`

    Ian> We don't bundle DocTools or Sphinx but it is possible to write text
    Ian> that is reasonable in both text output and html output given the rules
    Ian> already in place. I'm sure we could tweak them further though.

Ok, so I've tweaked back these changes, I originally revert the
rest changes because, thinking it *was* a rest document, I
embedded some TODO and FIXMEs in the comment which are *not* to
be displayed in a help output.

Now, can I have some *review* on the core of this patch or,
failing that, an agreement that since nobody cares enough and the
changes are unlikely to break anything I can just land it ?

Thanks in advance.

[John A Meinel (jameinel)]

101 +class ResolveActionOption(option.RegistryOption):
102 +
103 + def __init__(self):
104 + super(ResolveActionOption, self).__init__(
105 + 'action', 'How to resolve the conflict.',
106 + value_switches=True,
107 + registry=resolve_action_registry)
108 +
109 +

^- This only seems useful if you were actually going to re-use the
ResolveActionOption.
I realize you have some tests for it, but it doesn't do anything beyond a
standard RegistryOption.

Were you thinking to have the '--auto' logic as part of the option object?

168 +def resolve(tree, paths=None, ignore_misses=False, recursive=False,
169 + action='done'):
^- Having a top-level function in builtins.py to do command specific actions
isn't particularly nice, though I realize this wasn't from you. Perhaps just a
comment that it should be moved into a bzrlib.resolve* module?

236 + def _do(self, action, tree):
237 + """Apply the specified action to the conflict.
238 +
239 + :param action: The method name to call.
240 +
241 + :param tree: The tree passed as a parameter to the method.
242 + """
243 + meth = getattr(self, action, None)
244 + if meth is None:
245 + raise NotImplementedError(self.__class__.__name__ + '.' + action)
246 + meth(tree)

^- To avoid namespace collisions, this would probably be better as:

meth = getattr(self, 'action_' + action, None)
...

=== modified file 'bzrlib/tests/test_conflicts.py'
971 --- bzrlib/tests/test_conflicts.py 2010-01-04 14:56:59 +0000
972 +++ bzrlib/tests/test_conflicts.py 2010-02-04 13:06:32 +0000
973 @@ -21,8 +21,10 @@
974 bzrdir,
975 conflicts,
976 errors,
977 + option,
978 tests,
979 )
980 +from bzrlib.tests import script

^- script tests don't seem to be a great match for whitebox testing. And
looking at the scripts themselves they certainly look like blackbox tests.
(using commit to assert that things are ok.) Versus even an "assert*" function.

The tests seem fine, but they look like they should belong in a
blackbox/test_resolve.py module.

review: Needs Fixing

[Vincent Ladeuil (vila)]

> 101 +class ResolveActionOption(option.RegistryOption):
> 102 +
> 103 + def __init__(self):
> 104 + super(ResolveActionOption, self).__init__(
> 105 + 'action', 'How to resolve the conflict.',
> 106 + value_switches=True,
> 107 + registry=resolve_action_registry)
> 108 +
> 109 +
>
>
> ^- This only seems useful if you were actually going to re-use the
> ResolveActionOption.
> I realize you have some tests for it, but it doesn't do anything beyond a
> standard RegistryOption.

You're right, I introduced it for tests only.
The alternative was to define it in tests only too which was more likely to break in the future.

I can reconsider later if you feel strongly about it but overall I think it makes things simpler.

>
> Were you thinking to have the '--auto' logic as part of the option object?

Hopefully not :)

>
>
> 168 +def resolve(tree, paths=None, ignore_misses=False, recursive=False,
> 169 + action='done'):
> ^- Having a top-level function in builtins.py to do command specific actions
> isn't particularly nice, though I realize this wasn't from you. Perhaps just a
> comment that it should be moved into a bzrlib.resolve* module?

Meh, that's in conflicts.py and I don't think creating a resolve.py module is right. resolve is an action on conflict objects.

It *could* arguably become part of the MutableTree API and get a @needs_tree_write_lock
decorator or become a method of ConflictList.

>
>
> 236 + def _do(self, action, tree):
> 237 + """Apply the specified action to the conflict.
> 238 +
> 239 + :param action: The method name to call.
> 240 +
> 241 + :param tree: The tree passed as a parameter to the method.
> 242 + """
> 243 + meth = getattr(self, action, None)
> 244 + if meth is None:
> 245 + raise NotImplementedError(self.__class__.__name__ + '.' + action)
> 246 + meth(tree)
>
> ^- To avoid namespace collisions, this would probably be better as:
>
> meth = getattr(self, 'action_' + action, None)
> ...

Good point. Fixed.

>
>
> === modified file 'bzrlib/tests/test_conflicts.py'
> 971 --- bzrlib/tests/test_conflicts.py 2010-01-04 14:56:59 +0000
> 972 +++ bzrlib/tests/test_conflicts.py 2010-02-04 13:06:32 +0000
> 973 @@ -21,8 +21,10 @@
> 974 bzrdir,
> 975 conflicts,
> 976 errors,
> 977 + option,
> 978 tests,
> 979 )
> 980 +from bzrlib.tests import script
>
> ^- script tests don't seem to be a great match for whitebox testing. And
> looking at the scripts themselves they certainly look like blackbox tests.
> (using commit to assert that things are ok.) Versus even an "assert*"
> function.

>
> The tests seem fine, but they look like they should belong in a
> blackbox/test_resolve.py module.

Right, they served as a test bed for shell-like tests and like most shell-like tests they should be rewritten :) I'll add a TODO for that and will address it shortly.

=== modified file 'NEWS'
--- NEWS	2010-02-04 11:02:01 +0000
+++ NEWS	2010-02-04 13:06:32 +0000
@@ -19,6 +19,7 @@
 New Features
 ************
 
+<<<<<<< TREE
 * If the Apport crash-reporting tool is available, bzr crashes are now
   stored into the ``/var/crash`` apport spool directory, and the user is
   invited to report them to the developers from there, either
@@ -31,6 +32,13 @@
 * Stop sending apport crash files to ``.cache`` in the directory from
   which ``bzr selftest`` was run.  (Martin Pool, #422350)
 
+=======
+* Tree-shape conflicts can be resolved by providing ``--take-this`` and
+  ``--take-other`` to the ``bzr resolve`` command. Just marking the conflict
+  as resolved is still accessible via the ``--done`` default action.
+  (Vincent Ladeuil)
+
+>>>>>>> MERGE-SOURCE
 bzr 2.1.0 (not released yet)
 ############################
 

=== modified file 'bzrlib/conflicts.py'
--- bzrlib/conflicts.py	2010-01-08 07:37:25 +0000
+++ bzrlib/conflicts.py	2010-02-04 13:06:32 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2007 Canonical Ltd
+# Copyright (C) 2005, 2007, 2009, 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
@@ -14,12 +14,11 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
-# TODO: Move this into builtins
-
 # TODO: 'bzr resolve' should accept a directory name and work from that
 # point down
 
 import os
+import re
 
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
@@ -32,9 +31,14 @@
     osutils,
     rio,
     trace,
+    transform,
+    workingtree,
     )
 """)
-from bzrlib.option import Option
+from bzrlib import (
+    option,
+    registry,
+    )
 
 
 CONFLICT_SUFFIXES = ('.THIS', '.BASE', '.OTHER')
@@ -55,14 +59,13 @@
     Use bzr resolve when you have fixed a problem.
     """
     takes_options = [
-            Option('text',
-                   help='List paths of files with text conflicts.'),
+            option.Option('text',
+                          help='List paths of files with text conflicts.'),
         ]
     _see_also = ['resolve', 'conflict-types']
 
     def run(self, text=False):
-        from bzrlib.workingtree import WorkingTree
-        wt = WorkingTree.open_containing(u'.')[0]
+        wt = workingtree.WorkingTree.open_containing(u'.')[0]
         for conflict in wt.conflicts():
             if text:
                 if conflict.typestring != 'text conflict':
@@ -72,6 +75,28 @@
                 self.outf.write(str(conflict) + '\n')
 
 
+resolve_action_registry = registry.Registry()
+
+
+resolve_action_registry.register(
+    'done', 'done', 'Marks the conflict as resolved' )
+resolve_action_registry.register(
+    'take-this', 'take_this',
+    'Resolve the conflict preserving the version in the working tree' )
+resolve_action_registry.register(
+    'take-other', 'take_other',
+    'Resolve the conflict taking the merged version into account' )
+resolve_action_registry.default_key = 'done'
+
+class ResolveActionOption(option.RegistryOption):
+
+    def __init__(self):
+        super(ResolveActionOption, self).__init__(
+            'action', 'How to resolve the conflict.',
+            value_switches=True,
+            registry=resolve_action_registry)
+
+
 class cmd_resolve(commands.Command):
     """Mark a conflict as resolved.
 
@@ -87,20 +112,33 @@
     aliases = ['resolved']
     takes_args = ['file*']
     takes_options = [
-            Option('all', help='Resolve all conflicts in this tree.'),
+            option.Option('all', help='Resolve all conflicts in this tree.'),
+            ResolveActionOption(),
             ]
     _see_also = ['conflicts']
-    def run(self, file_list=None, all=False):
-        from bzrlib.workingtree import WorkingTree
+    def run(self, file_list=None, all=False, action=None):
         if all:
             if file_list:
                 raise errors.BzrCommandError("If --all is specified,"
                                              " no FILE may be provided")
-            tree = WorkingTree.open_containing('.')[0]
-            resolve(tree)
+            tree = workingtree.WorkingTree.open_containing('.')[0]
+            if action is None:
+                action = 'done'
         else:
             tree, file_list = builtins.tree_files(file_list)
             if file_list is None:
+                if action is None:
+                    # FIXME: There is a special case here related to the option
+                    # handling that could be clearer and easier to discover by
+                    # providing an --auto action (bug #344013 and #383396) and
+                    # make it mandatory instead of implicit and active only
+                    # when no file_list is provided -- vila 091229
+                    action = 'auto'
+            else:
+                if action is None:
+                    action = 'done'
+        if action == 'auto':
+            if file_list is None:
                 un_resolved, resolved = tree.auto_resolve()
                 if len(un_resolved) > 0:
                     trace.note('%d conflict(s) auto-resolved.', len(resolved))
@@ -112,10 +150,16 @@
                     trace.note('All conflicts resolved.')
                     return 0
             else:
-                resolve(tree, file_list)
-
-
-def resolve(tree, paths=None, ignore_misses=False, recursive=False):
+                # FIXME: This can never occur but the block above needs some
+                # refactoring to transfer tree.auto_resolve() to
+                # conflict.auto(tree) --vila 091242
+                pass
+        else:
+            resolve(tree, file_list, action=action)
+
+
+def resolve(tree, paths=None, ignore_misses=False, recursive=False,
+            action='done'):
     """Resolve some or all of the conflicts in a working tree.
 
     :param paths: If None, resolve all conflicts.  Otherwise, select only
@@ -125,24 +169,29 @@
         recursive commands like revert, this should be True.  For commands
         or applications wishing finer-grained control, like the resolve
         command, this should be False.
-    :ignore_misses: If False, warnings will be printed if the supplied paths
-        do not have conflicts.
+    :param ignore_misses: If False, warnings will be printed if the supplied
+        paths do not have conflicts.
+    :param action: How the conflict should be resolved,
     """
     tree.lock_tree_write()
     try:
         tree_conflicts = tree.conflicts()
         if paths is None:
             new_conflicts = ConflictList()
-            selected_conflicts = tree_conflicts
+            to_process = tree_conflicts
         else:
-            new_conflicts, selected_conflicts = \
-                tree_conflicts.select_conflicts(tree, paths, ignore_misses,
-                    recursive)
+            new_conflicts, to_process = tree_conflicts.select_conflicts(
+                tree, paths, ignore_misses, recursive)
+        for conflict in to_process:
+            try:
+                conflict._do(action, tree)
+                conflict.cleanup(tree)
+            except NotImplementedError:
+                new_conflicts.append(conflict)
         try:
             tree.set_conflicts(new_conflicts)
         except errors.UnsupportedOperation:
             pass
-        selected_conflicts.remove_files(tree)
     finally:
         tree.unlock()
 
@@ -237,12 +286,7 @@
         for conflict in self:
             if not conflict.has_files:
                 continue
-            for suffix in CONFLICT_SUFFIXES:
-                try:
-                    osutils.delete_any(tree.abspath(conflict.path+suffix))
-                except OSError, e:
-                    if e.errno != errno.ENOENT:
-                        raise
+            conflict.cleanup(tree)
 
     def select_conflicts(self, tree, paths, ignore_misses=False,
                          recurse=False):
@@ -301,6 +345,7 @@
 class Conflict(object):
     """Base class for all types of conflict"""
 
+    # FIXME: cleanup should take care of that ? -- vila 091229
     has_files = False
 
     def __init__(self, path, file_id=None):
@@ -355,6 +400,32 @@
         else:
             return None, conflict.typestring
 
+    def _do(self, action, tree):
+        """Apply the specified action to the conflict.
+
+        :param action: The method name to call.
+
+        :param tree: The tree passed as a parameter to the method.
+        """
+        meth = getattr(self, action, None)
+        if meth is None:
+            raise NotImplementedError(self.__class__.__name__ + '.' + action)
+        meth(tree)
+
+    def cleanup(self, tree):
+        raise NotImplementedError(self.cleanup)
+
+    def done(self, tree):
+        """Mark the conflict as solved once it has been handled."""
+        # This method does nothing but simplifies the design of upper levels.
+        pass
+
+    def take_this(self, tree):
+        raise NotImplementedError(self.take_this)
+
+    def take_other(self, tree):
+        raise NotImplementedError(self.take_other)
+
 
 class PathConflict(Conflict):
     """A conflict was encountered merging file paths"""
@@ -364,6 +435,7 @@
     format = 'Path conflict: %(path)s / %(conflict_path)s'
 
     rformat = '%(class)s(%(path)r, %(conflict_path)r, %(file_id)r)'
+
     def __init__(self, path, conflict_path=None, file_id=None):
         Conflict.__init__(self, path, file_id)
         self.conflict_path = conflict_path
@@ -374,6 +446,17 @@
             s.add('conflict_path', self.conflict_path)
         return s
 
+    def cleanup(self, tree):
+        # No additional files have been generated here
+        pass
+
+    def take_this(self, tree):
+        tree.rename_one(self.conflict_path, self.path)
+
+    def take_other(self, tree):
+        # just acccept bzr proposal
+        pass
+
 
 class ContentsConflict(PathConflict):
     """The files are of different types, or not present"""
@@ -384,7 +467,30 @@
 
     format = 'Contents conflict in %(path)s'
 
-
+    def cleanup(self, tree):
+        for suffix in ('.BASE', '.OTHER'):
+            try:
+                osutils.delete_any(tree.abspath(self.path + suffix))
+            except OSError, e:
+                if e.errno != errno.ENOENT:
+                    raise
+
+    # FIXME: I smell something weird here and it seems we should be able to be
+    # more coherent with some other conflict ? bzr *did* a choice there but
+    # neither take_this nor take_other reflect that... -- vila 091224
+    def take_this(self, tree):
+        tree.remove([self.path + '.OTHER'], force=True, keep_files=False)
+
+    def take_other(self, tree):
+        tree.remove([self.path], force=True, keep_files=False)
+
+
+
+# FIXME: TextConflict is about a single file-id, there never is a conflict_path
+# attribute so we shouldn't inherit from PathConflict but simply from Conflict
+
+# TODO: There should be a base revid attribute to better inform the user about
+# how the conflicts were generated.
 class TextConflict(PathConflict):
     """The merge algorithm could not resolve all differences encountered."""
 
@@ -394,6 +500,14 @@
 
     format = 'Text conflict in %(path)s'
 
+    def cleanup(self, tree):
+        for suffix in CONFLICT_SUFFIXES:
+            try:
+                osutils.delete_any(tree.abspath(self.path+suffix))
+            except OSError, e:
+                if e.errno != errno.ENOENT:
+                    raise
+
 
 class HandledConflict(Conflict):
     """A path problem that has been provisionally resolved.
@@ -414,6 +528,10 @@
         s.add('action', self.action)
         return s
 
+    def cleanup(self, tree):
+        """Nothing to cleanup."""
+        pass
+
 
 class HandledPathConflict(HandledConflict):
     """A provisionally-resolved path problem involving two paths.
@@ -460,15 +578,22 @@
 
     format = 'Conflict adding file %(conflict_path)s.  %(action)s %(path)s.'
 
+    def take_this(self, tree):
+        tree.remove([self.conflict_path], force=True, keep_files=False)
+        tree.rename_one(self.path, self.conflict_path)
+
+    def take_other(self, tree):
+        tree.remove([self.path], force=True, keep_files=False)
+
 
 class ParentLoop(HandledPathConflict):
     """An attempt to create an infinitely-looping directory structure.
     This is rare, but can be produced like so:
 
     tree A:
-      mv foo/bar
+      mv foo bar
     tree B:
-      mv bar/foo
+      mv bar foo
     merge A and B
     """
 
@@ -476,6 +601,27 @@
 
     format = 'Conflict moving %(conflict_path)s into %(path)s.  %(action)s.'
 
+    def take_this(self, tree):
+        # just acccept bzr proposal
+        pass
+
+    def take_other(self, tree):
+        # FIXME: We shouldn't have to manipulate so many paths here (and there
+        # is probably a bug or two...)
+        base_path = osutils.basename(self.path)
+        conflict_base_path = osutils.basename(self.conflict_path)
+        tt = transform.TreeTransform(tree)
+        try:
+            p_tid = tt.trans_id_file_id(self.file_id)
+            parent_tid = tt.get_tree_parent(p_tid)
+            cp_tid = tt.trans_id_file_id(self.conflict_file_id)
+            cparent_tid = tt.get_tree_parent(cp_tid)
+            tt.adjust_path(base_path, cparent_tid, cp_tid)
+            tt.adjust_path(conflict_base_path, parent_tid, p_tid)
+            tt.apply()
+        finally:
+            tt.finalize()
+
 
 class UnversionedParent(HandledConflict):
     """An attempt to version a file whose parent directory is not versioned.
@@ -488,18 +634,34 @@
     format = 'Conflict because %(path)s is not versioned, but has versioned'\
              ' children.  %(action)s.'
 
+    # FIXME: We silently do nothing to make tests pass, but most probably the
+    # conflict shouldn't exist (the long story is that the conflict is
+    # generated with another one that can be resolved properly) -- vila 091224
+    def take_this(self, tree):
+        pass
+
+    def take_other(self, tree):
+        pass
+
 
 class MissingParent(HandledConflict):
     """An attempt to add files to a directory that is not present.
     Typically, the result of a merge where THIS deleted the directory and
     the OTHER added a file to it.
-    See also: DeletingParent (same situation, reversed THIS and OTHER)
+    See also: DeletingParent (same situation, THIS and OTHER reversed)
     """
 
     typestring = 'missing parent'
 
     format = 'Conflict adding files to %(path)s.  %(action)s.'
 
+    def take_this(self, tree):
+        tree.remove([self.path], force=True, keep_files=False)
+
+    def take_other(self, tree):
+        # just acccept bzr proposal
+        pass
+
 
 class DeletingParent(HandledConflict):
     """An attempt to add files to a directory that is not present.
@@ -512,9 +674,19 @@
     format = "Conflict: can't delete %(path)s because it is not empty.  "\
              "%(action)s."
 
+    # FIXME: It's a bit strange that the default action is not coherent with
+    # MissingParent from the *user* pov.
+
+    def take_this(self, tree):
+        # just acccept bzr proposal
+        pass
+
+    def take_other(self, tree):
+        tree.remove([self.path], force=True, keep_files=False)
+
 
 class NonDirectoryParent(HandledConflict):
-    """An attempt to add files to a directory that is not a director or
+    """An attempt to add files to a directory that is not a directory or
     an attempt to change the kind of a directory with files.
     """
 
@@ -523,6 +695,27 @@
     format = "Conflict: %(path)s is not a directory, but has files in it."\
              "  %(action)s."
 
+    # FIXME: .OTHER should be used instead of .new when the conflict is created
+
+    def take_this(self, tree):
+        # FIXME: we should preserve that path when the conflict is generated !
+        if self.path.endswith('.new'):
+            conflict_path = self.path[:-(len('.new'))]
+            tree.remove([self.path], force=True, keep_files=False)
+            tree.add(conflict_path)
+        else:
+            raise NotImplementedError(self.take_this)
+
+    def take_other(self, tree):
+        # FIXME: we should preserve that path when the conflict is generated !
+        if self.path.endswith('.new'):
+            conflict_path = self.path[:-(len('.new'))]
+            tree.remove([conflict_path], force=True, keep_files=False)
+            tree.rename_one(self.path, conflict_path)
+        else:
+            raise NotImplementedError(self.take_other)
+
+
 ctype = {}
 
 
@@ -532,7 +725,6 @@
     for conflict_type in conflict_types:
         ctype[conflict_type.typestring] = conflict_type
 
-
 register_types(ContentsConflict, TextConflict, PathConflict, DuplicateID,
                DuplicateEntry, ParentLoop, UnversionedParent, MissingParent,
                DeletingParent, NonDirectoryParent)

=== modified file 'bzrlib/help_topics/en/conflict-types.txt'
--- bzrlib/help_topics/en/conflict-types.txt	2010-01-08 07:37:25 +0000
+++ bzrlib/help_topics/en/conflict-types.txt	2010-02-04 13:06:32 +0000
@@ -3,16 +3,59 @@
 
 Some operations, like merge, revert and pull, modify the contents of your
 working tree.  These modifications are programmatically generated, and so they
-may conflict with the current state of your working tree. Many kinds of changes
-can be combined programmatically, but sometimes only a human can determine the
-right thing to do.  When this happens Bazaar will inform you that there is a
-conflict and then ask you to resolve it.  The command to tell Bazaar a conflict
-is resolved is ``resolve``, but you must perform some action before you can do
-this.
+may conflict with the current state of your working tree. 
+
+When conflicts are present in your working tree (as shown by ``bzr
+conflicts``), you should resolve them and then inform bzr that the conflicts
+have been resolved.
+
+Resolving conflicts is sometimes not obvious. Either because the user that
+should resolve them is not the one responsible for their occurrence, as is the
+case when merging other people's work or because some conflicts are presented
+in a way that is not easy to understand.
+
+Bazaar tries to avoid conflicts ; its aim is to ask you to resolve the
+conflict if and only if there's an actual conceptual conflict in the source
+tree.  Because Bazaar doesn't understand the real meaning of the files being
+versioned, it can, when faced with ambiguities, fall short in either direction
+trying to resolve the conflict itself. Many kinds of changes can be combined
+programmatically, but sometimes only a human can determine the right thing to
+do.
+
+When Bazaar generates a conflict, it adds information into the working tree to
+present the conflicting versions, and it's up to you to find the correct
+resolution.
+
+Whatever the conflict is, resolving it is roughly done in two steps::
+
+ * modify the working tree content so that the conflicted item is now in the
+   state you want to keep,
+
+ * inform Bazaar that the conflict is now solved and ask to cleanup any
+   remaining generated information (``bzr resolve <item>``).
+
+For most conflict types, there are some obvious ways to modify the working
+tree and put it into the desired state. For some types of conflicts, Bazaar
+itself already made a choice, when possible.
+
+Yet, whether Bazaar makes a choice or not, there are some other simple but
+different ways to resolve the conflict.
 
 Each type of conflict is explained below, and the action which must be done to
 resolve the conflict is outlined.
 
+Various actions are available depending on the kind of conflict, for some of
+these actions, Bazaar can provide some help. In the end you should at least
+inform Bazaar that you're done with the conflict with::
+
+  ``bzr resolve FILE --action=done'
+
+Note that this is the default action when a single file is involved so you can
+simply use::
+
+   ``bzr resolve FILE``
+
+See ``bzr help resolve`` for more details.
 
 Text conflicts
 --------------
@@ -54,8 +97,12 @@
 you are done editing, the file should look like it never had a conflict, and be
 ready to commit.
 
-When you have resolved text conflicts, just run "bzr resolve", and Bazaar will
-auto-detect which conflicts you have resolved.
+When you have resolved text conflicts, just run ``bzr resolve --auto``, and
+Bazaar will auto-detect which conflicts you have resolved.
+
+When the conflict is resolved, Bazaar deletes the previously generated
+``.BASE``, ``.THIS`` and ``.OTHER`` files if they are still present in the
+working tree.
 
 Content conflicts
 -----------------
@@ -64,20 +111,29 @@
 
   Contents conflict in FILE
 
-This conflict happens when there are conflicting changes in the target tree and
-the merge source, but the conflicted items are not text files.  They may be
-binary files, or symlinks, or directories.  It can even happen with files that
-are deleted on one side, and modified on the other.
+This conflict happens when there are conflicting changes in the working tree
+and the merge source, but the conflicted items are not text files.  They may
+be binary files, or symlinks, or directories.  It can even happen with files
+that are deleted on one side, and modified on the other.
 
 Like text conflicts, Bazaar will emit THIS, OTHER and BASE files.  (They may be
 regular files, symlinks or directories).  But it will not include a "main copy"
 of the file with herringbone conflict markers.  It will appear that the "main
 copy" has been renamed to THIS or OTHER.
 
-To resolve this, use "bzr mv" to rename the file back to its normal name, and
-combine the changes manually.  When you are satisfied, run "bzr resolve
-FILE".  Bazaar cannot auto-detect when conflicts of this kind have been
-resolved.
+To resolve that kind of conflict, you should rebuild FILE from either version
+or a combination of both.
+
+``bzr resolve`` recognizes the following actions:
+
+ * ``--action=take-this`` will issue ``bzr mv FILE.THIS FILE``,
+ * ``--action=take-other`` will issue ``bzr mv FILE.OTHER FILE``,
+ * ``--action=done`` will just mark the conflict as resolved.
+
+Any action will also delete the previously generated ``.BASE``, ``.THIS`` and
+``.OTHER`` files if they are still present in the working tree.
+
+Bazaar cannot auto-detect when conflicts of this kind have been resolved.
 
 Tag conflicts
 -------------
@@ -95,12 +151,13 @@
 To resolve the conflict, you must apply the correct tags to either the target
 branch or the source branch as appropriate.  Use "bzr tags --show-ids -d
 SOURCE_URL" to see the tags in the source branch.  If you want to make the
-target branch's tags match the source branch, then in the target branch do 
+target branch's tags match the source branch, then in the target branch do
 ``bzr tag --force -r revid:REVISION_ID CONFLICTING_TAG`` for each of the
 CONFLICTING_TAGs, where REVISION_ID comes from the list of tags in the source
-branch.  You need not call "bzr resolve" after doing this.  To resolve in favor of the target branch, you need to similarly use ``tag --force`` in the source
-branch.  (Note that pulling or pushing using --overwrite will overwrite all
-tags as well.)
+branch.  You need not call "bzr resolve" after doing this.  To resolve in
+favor of the target branch, you need to similarly use ``tag --force`` in the
+source branch.  (Note that pulling or pushing using --overwrite will overwrite
+all tags as well.)
 
 Duplicate paths
 ---------------
@@ -110,10 +167,20 @@
   Conflict adding file FILE.  Moved existing file to FILE.moved.
 
 Sometimes Bazaar will attempt to create a file using a pathname that has
-already been used.  The existing file will be renamed to "FILE.moved".  If
-you wish, you can rename either one of these files, or combine their contents.
-When you are satisfied, you can run "bzr resolve FILE" to mark the conflict as
-resolved.
+already been used.  The existing file will be renamed to "FILE.moved".  
+
+To resolve that kind of conflict, you should rebuild FILE from either version
+or a combination of both.
+
+``bzr resolve`` recognizes the following actions::
+
+ * ``--action=take-this`` will issue ``bzr rm FILE ; bzr mv FILE.moved FILE``,
+ *  ``--action=take-other`` will issue ``bzr rm FILE.moved``,
+ * ``--action=done`` will just mark the conflict as resolved.
+
+Note that you must get rid of FILE.moved before using ``--action=done``.
+
+Bazaar cannot auto-detect when conflicts of this kind have been resolved.
 
 Unversioned parent
 ------------------
@@ -137,26 +204,47 @@
 
   Conflict adding files to FILE.  Created directory.
 
-This happens when a file has been deleted in the target, but has new children
-in the source.  This is similar to the "unversioned parent" conflict, except
-that the parent directory does not *exist*, instead of just being unversioned.
-In this situation, Bazaar will create the missing parent.  Resolving this issue
-depends very much on the particular scenario.  You may wish to rename or delete
-either the file or the directory.  When you are satisfied, you can run "bzr
-resolve FILE" to mark the conflict as resolved.
+This happens when a directory has been deleted in the target, but has new
+children in the source.  This is similar to the "unversioned parent" conflict,
+except that the parent directory does not *exist*, instead of just being
+unversioned.  In this situation, Bazaar will create the missing parent.
+Resolving this issue depends very much on the particular scenario.
+
+To resolve that kind of conflict, you should either remove or rename the
+children or the directory or a combination of both.
+
+``bzr resolve`` recognizes the following actions::
+
+ * ``--action=take-this`` will issue ``bzr rm directory``
+   including the children,
+ * ``--action=take-other`` will acknowledge Bazaar choice to keep
+   the children and restoring the directory,
+ * ``--action=done`` will just mark the conflict as resolved.
+
+Bazaar cannot auto-detect when conflicts of this kind have been resolved.
 
 Deleting parent
 ---------------
 
 Typical message::
 
-  Conflict: can't delete FILE because it is not empty.  Not deleting.
+  Conflict: can't delete DIR because it is not empty.  Not deleting.
 
 This is the opposite of "missing parent".  A directory is deleted in the
 source, but has new children in the target.  Bazaar will retain the directory.
-Resolving this issue depends very much on the particular scenario.  You may
-wish to rename or delete either the file or the directory.  When you are
-satisfied, you can run "bzr resolve FILE" to mark the conflict as resolved.
+Resolving this issue depends very much on the particular scenario.
+
+To resolve that kind of conflict, you should either remove or rename the
+children or the directory or a combination of both.
+
+``bzr resolve`` recognizes the following actions::
+
+ * ``--action=take-this`` will acknowledge Bazaar choice to keep the directory,
+ * ``--action=take-other`` will issue ``bzr rm directory`` including the 
+   children,
+ * ``--action=done`` will just mark the conflict as resolved.
+
+Bazaar cannot auto-detect when conflicts of this kind have been resolved.
 
 Path conflict
 -------------
@@ -166,9 +254,19 @@
   Path conflict: PATH1 / PATH2
 
 This happens when the source and target have each modified the name or parent
-directory of a file.  Bazaar will use the path elements from the source.  You
-can rename the file, and once you have, run "bzr resolve FILE" to mark the
-conflict as resolved.
+directory of a file.  Bazaar will use the path elements from the source.
+
+To resolve that kind of conflict, you just have to decide what name should be
+retained for the file involved.
+
+``bzr resolve`` recognizes the following actions::
+
+ * ``--action=take-this`` will revert Bazaar choice and keep ``PATH1`` by
+   issuing ``bzr mv PATH2 PATH1``,
+ * ``--action=take-other`` will acknowledge Bazaar choice of keeping ``PATH2``,
+ * ``--action=done`` will just mark the conflict as resolved.
+
+Bazaar cannot auto-detect when conflicts of this kind have been resolved.
 
 Parent loop
 -----------
@@ -179,45 +277,67 @@
 
 This happens when the source and the target have each moved directories, so
 that, if the change could be applied, a directory would be contained by itself.
-For example::
+For example:
 
   $ bzr init
-  $ bzr mkdir a
-  $ bzr mkdir b
+  $ bzr mkdir white
+  $ bzr mkdir black
   $ bzr commit -m "BASE"
   $ bzr branch . ../other
-  $ bzr mv a b
+  $ bzr mv white black
   $ bzr commit -m "THIS"
-  $ bzr mv ../other/b ../other/a
+  $ bzr mv ../other/black ../other/white
   $ bzr commit ../other -m "OTHER"
   $ bzr merge ../other
 
-In this situation, Bazaar will cancel the move, and leave "a" in "b".
-You can rename the directories if you like, and once you have, run "bzr resolve
-FILE" to mark the conflict as resolved.
+In this situation, Bazaar will cancel the move, and leave ``white`` in
+``black``.  To resolve that kind of conflict, you just have to decide what
+name should be retained for the directories involved.
+
+``bzr resolve`` recognizes the following actions::
+
+ * ``--action=take-this`` will acknowledge Bazaar choice of leaving ``white`` 
+   in ``black``,
+ * ``--action=take-other`` will revert Bazaar choice and move ``black`` in
+   ``white`` by issuing ``bzr mv black/white white ; bzr mv black white``,
+ * ``--action=done`` will just mark the conflict as resolved.
+
+Bazaar cannot auto-detect when conflicts of this kind have been resolved.
 
 Non-directory parent
 --------------------
 
 Typical message::
 
-  Conflict: FILE.new is not a directory, but has files in it.
+  Conflict: foo.new is not a directory, but has files in it.
   Created directory.
 
 This happens when one side has added files to a directory, and the other side
-has changed the directory into a file or symlink.  For example::
+has changed the directory into a file or symlink.  For example:
 
   $ bzr init
-  $ bzr mkdir a
+  $ bzr mkdir foo
   $ bzr commit -m "BASE"
   $ bzr branch . ../other
-  $ rmdir a
-  $ touch a
+  $ rmdir foo
+  $ touch foo
   $ bzr commit -m "THIS"
-  $ bzr mkdir ../other/a/b
+  $ bzr mkdir ../other/foo/bar
   $ bzr commit ../other -m "OTHER"
   $ bzr merge ../other
 
+To resolve that kind of conflict, you have to decide what name should be
+retained for the file, directory or symlink involved.
+
+``bzr resolve`` recognizes the following actions::
+
+ * ``--action=take-this`` will issue ``bzr rm --force foo.new`` and 
+   ``bzr add foo``,
+ * ``--action=take-other`` will issue ``bzr rm --force foo`` and 
+   ``bzr mv foo.new foo``,
+ * ``--action=done`` will just mark the conflict as resolved.
+
+Bazaar cannot auto-detect when conflicts of this kind have been resolved.
 
 MalformedTransform
 ------------------

=== added directory 'bzrlib/help_topics/es'
=== removed directory 'bzrlib/help_topics/es'
=== renamed file 'bzrlib/help_topics/es/conflicts.txt' => 'bzrlib/help_topics/es/conflict-types.txt'
=== modified file 'bzrlib/option.py'
--- bzrlib/option.py	2009-08-18 08:10:44 +0000
+++ bzrlib/option.py	2010-02-04 13:06:31 +0000
@@ -361,7 +361,7 @@
 
         name, help, value_switches and enum_switch are passed to the
         RegistryOption constructor.  Any other keyword arguments are treated
-        as values for the option, and they value is treated as the help.
+        as values for the option, and their value is treated as the help.
         """
         reg = _mod_registry.Registry()
         for name, switch_help in kwargs.iteritems():

=== modified file 'bzrlib/tests/blackbox/test_conflicts.py'
--- bzrlib/tests/blackbox/test_conflicts.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/blackbox/test_conflicts.py	2010-02-04 13:06:32 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006, 2009, 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
@@ -14,21 +14,22 @@
 # 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
-
 from bzrlib import (
-    conflicts
+    conflicts,
+    tests,
+    workingtree,
     )
-from bzrlib.workingtree import WorkingTree
-from bzrlib.tests.blackbox import ExternalBase
 
 # FIXME: These don't really look at the output of the conflict commands, just
 # the number of lines - there should be more examination.
 
-class TestConflicts(ExternalBase):
+class TestConflictsBase(tests.TestCaseWithTransport):
 
     def setUp(self):
-        super(ExternalBase, self).setUp()
+        super(TestConflictsBase, self).setUp()
+        self.make_tree_with_conflicts()
+
+    def make_tree_with_conflicts(self):
         a_tree = self.make_branch_and_tree('a')
         self.build_tree_contents([
             ('a/myfile', 'contentsa\n'),
@@ -53,45 +54,48 @@
         a_tree.rename_one('mydir', 'mydir3')
         a_tree.commit(message='change')
         a_tree.merge_from_branch(b_tree.branch)
-        os.chdir('a')
+
+    def run_bzr(self, cmd, working_dir='a', **kwargs):
+        return super(TestConflictsBase, self).run_bzr(
+            cmd, working_dir=working_dir, **kwargs)
+
+
+class TestConflicts(TestConflictsBase):
 
     def test_conflicts(self):
-        conflicts, errs = self.run_bzr('conflicts')
-        self.assertEqual(3, len(conflicts.splitlines()))
+        out, err = self.run_bzr('conflicts')
+        self.assertEqual(3, len(out.splitlines()))
 
     def test_conflicts_text(self):
-        conflicts = self.run_bzr('conflicts --text')[0].splitlines()
-        self.assertEqual(['my_other_file', 'myfile'], conflicts)
+        out, err = self.run_bzr('conflicts --text')
+        self.assertEqual(['my_other_file', 'myfile'], out.splitlines())
+
+
+class TestResolve(TestConflictsBase):
 
     def test_resolve(self):
         self.run_bzr('resolve myfile')
-        conflicts, errs = self.run_bzr('conflicts')
-        self.assertEqual(2, len(conflicts.splitlines()))
+        out, err = self.run_bzr('conflicts')
+        self.assertEqual(2, len(out.splitlines()))
         self.run_bzr('resolve my_other_file')
         self.run_bzr('resolve mydir2')
-        conflicts, errs = self.run_bzr('conflicts')
-        self.assertEqual(len(conflicts.splitlines()), 0)
+        out, err = self.run_bzr('conflicts')
+        self.assertEqual(0, len(out.splitlines()))
 
     def test_resolve_all(self):
         self.run_bzr('resolve --all')
-        conflicts, errs = self.run_bzr('conflicts')
-        self.assertEqual(len(conflicts.splitlines()), 0)
+        out, err = self.run_bzr('conflicts')
+        self.assertEqual(0, len(out.splitlines()))
 
     def test_resolve_in_subdir(self):
         """resolve when run from subdirectory should handle relative paths"""
-        orig_dir = os.getcwdu()
-        try:
-            os.mkdir("subdir")
-            os.chdir("subdir")
-            self.run_bzr("resolve ../myfile")
-            os.chdir("../../b")
-            self.run_bzr("resolve ../a/myfile")
-            wt = WorkingTree.open_containing('.')[0]
-            conflicts = wt.conflicts()
-            if not conflicts.is_empty():
-                self.fail("tree still contains conflicts: %r" % conflicts)
-        finally:
-            os.chdir(orig_dir)
+        self.build_tree(["a/subdir/"])
+        self.run_bzr("resolve ../myfile", working_dir='a/subdir')
+        self.run_bzr("resolve ../a/myfile", working_dir='b')
+        wt = workingtree.WorkingTree.open_containing('b')[0]
+        conflicts = wt.conflicts()
+        self.assertEqual(True, conflicts.is_empty(),
+                         "tree still contains conflicts: %r" % conflicts)
 
     def test_auto_resolve(self):
         """Text conflicts can be resolved automatically"""
@@ -102,11 +106,10 @@
         self.assertEqual(tree.kind('file_id'), 'file')
         file_conflict = conflicts.TextConflict('file', file_id='file_id')
         tree.set_conflicts(conflicts.ConflictList([file_conflict]))
-        os.chdir('tree')
-        note = self.run_bzr('resolve', retcode=1)[1]
+        note = self.run_bzr('resolve', retcode=1, working_dir='tree')[1]
         self.assertContainsRe(note, '0 conflict\\(s\\) auto-resolved.')
         self.assertContainsRe(note,
             'Remaining conflicts:\nText conflict in file')
-        self.build_tree_contents([('file', 'a\n')])
-        note = self.run_bzr('resolve')[1]
+        self.build_tree_contents([('tree/file', 'a\n')])
+        note = self.run_bzr('resolve', working_dir='tree')[1]
         self.assertContainsRe(note, 'All conflicts resolved.')

=== modified file 'bzrlib/tests/test_conflicts.py'
--- bzrlib/tests/test_conflicts.py	2010-01-04 14:56:59 +0000
+++ bzrlib/tests/test_conflicts.py	2010-02-04 13:06:32 +0000
@@ -21,8 +21,10 @@
     bzrdir,
     conflicts,
     errors,
+    option,
     tests,
     )
+from bzrlib.tests import script
 
 
 # TODO: Test commit with some added, and added-but-missing files
@@ -171,3 +173,504 @@
             if 'conflict_file_id' in stanza:
                 self.assertStartsWith(stanza['conflict_file_id'], u'\xeed')
 
+
+# FIXME: Tests missing for DuplicateID conflict type
+class TestResolveConflicts(script.TestCaseWithTransportAndScript):
+
+    preamble = None # The setup script set by daughter classes
+
+    def setUp(self):
+        super(TestResolveConflicts, self).setUp()
+        self.run_script(self.preamble)
+
+
+class TestResolveTextConflicts(TestResolveConflicts):
+    # TBC
+    pass
+
+
+class TestResolveContentConflicts(TestResolveConflicts):
+
+    # FIXME: We need to add the reverse case (delete in trunk, modify in
+    # branch) but that could wait until the resolution mechanism is implemented.
+
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ echo 'trunk content' >file
+$ bzr add file
+$ bzr commit -m 'Create trunk'
+
+$ bzr branch . ../branch
+$ cd ../branch
+$ bzr rm file
+$ bzr commit -m 'Delete file'
+
+$ cd ../trunk
+$ echo 'more content' >>file
+$ bzr commit -m 'Modify file'
+
+$ cd ../branch
+$ bzr merge ../trunk
+2>+N  file.OTHER
+2>Contents conflict in file
+2>1 conflicts encountered.
+"""
+
+    def test_take_this(self):
+        self.run_script("""
+$ bzr rm file.OTHER --force # a simple rm file.OTHER is valid too
+$ bzr resolve file
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_take_other(self):
+        self.run_script("""
+$ bzr mv file.OTHER file
+$ bzr resolve file
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_this(self):
+        self.run_script("""
+$ bzr resolve --take-this file
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_other(self):
+        self.run_script("""
+$ bzr resolve --take-other file
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+
+class TestResolveDuplicateEntry(TestResolveConflicts):
+
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ echo 'trunk content' >file
+$ bzr add file
+$ bzr commit -m 'Create trunk'
+$ echo 'trunk content too' >file2
+$ bzr add file2
+$ bzr commit -m 'Add file2 in trunk'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ echo 'branch content' >file2
+$ bzr add file2
+$ bzr commit -m 'Add file2 in branch'
+
+$ bzr merge ../trunk
+2>+N  file2
+2>R   file2 => file2.moved
+2>Conflict adding file file2.  Moved existing file to file2.moved.
+2>1 conflicts encountered.
+"""
+
+    def test_keep_this(self):
+        self.run_script("""
+$ bzr rm file2  --force
+$ bzr mv file2.moved file2
+$ bzr resolve file2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_keep_other(self):
+        self.failIfExists('branch/file2.moved')
+        self.run_script("""
+$ bzr rm file2.moved --force
+$ bzr resolve file2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+        self.failIfExists('branch/file2.moved')
+
+    def test_resolve_taking_this(self):
+        self.run_script("""
+$ bzr resolve --take-this file2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_other(self):
+        self.run_script("""
+$ bzr resolve --take-other file2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+
+class TestResolveUnversionedParent(TestResolveConflicts):
+
+    # FIXME: Add the reverse tests: dir deleted in trunk, file added in branch
+
+    # FIXME: While this *creates* UnversionedParent conflicts, this really only
+    # tests MissingParent resolution :-/
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ mkdir dir
+$ bzr add dir
+$ bzr commit -m 'Create trunk'
+$ echo 'trunk content' >dir/file
+$ bzr add dir/file
+$ bzr commit -m 'Add dir/file in trunk'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ bzr rm dir
+$ bzr commit -m 'Remove dir in branch'
+
+$ bzr merge ../trunk
+2>+N  dir/
+2>+N  dir/file
+2>Conflict adding files to dir.  Created directory.
+2>Conflict because dir is not versioned, but has versioned children.  Versioned directory.
+2>2 conflicts encountered.
+"""
+
+    def test_take_this(self):
+        self.run_script("""
+$ bzr rm dir  --force
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_take_other(self):
+        self.run_script("""
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+
+class TestResolveMissingParent(TestResolveConflicts):
+
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ mkdir dir
+$ echo 'trunk content' >dir/file
+$ bzr add
+$ bzr commit -m 'Create trunk'
+$ echo 'trunk content' >dir/file2
+$ bzr add dir/file2
+$ bzr commit -m 'Add dir/file2 in branch'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ bzr rm dir/file --force
+$ bzr rm dir
+$ bzr commit -m 'Remove dir/file'
+
+$ bzr merge ../trunk
+2>+N  dir/
+2>+N  dir/file2
+2>Conflict adding files to dir.  Created directory.
+2>Conflict because dir is not versioned, but has versioned children.  Versioned directory.
+2>2 conflicts encountered.
+"""
+
+    def test_keep_them_all(self):
+        self.run_script("""
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_adopt_child(self):
+        self.run_script("""
+$ bzr mv dir/file2 file2
+$ bzr rm dir --force
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_kill_them_all(self):
+        self.run_script("""
+$ bzr rm dir --force
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_this(self):
+        self.run_script("""
+$ bzr resolve --take-this dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_other(self):
+        self.run_script("""
+$ bzr resolve --take-other dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+
+class TestResolveDeletingParent(TestResolveConflicts):
+
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ mkdir dir
+$ echo 'trunk content' >dir/file
+$ bzr add
+$ bzr commit -m 'Create trunk'
+$ bzr rm dir/file --force
+$ bzr rm dir --force
+$ bzr commit -m 'Remove dir/file'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ echo 'branch content' >dir/file2
+$ bzr add dir/file2
+$ bzr commit -m 'Add dir/file2 in branch'
+
+$ bzr merge ../trunk
+2>-D  dir/file
+2>Conflict: can't delete dir because it is not empty.  Not deleting.
+2>Conflict because dir is not versioned, but has versioned children.  Versioned directory.
+2>2 conflicts encountered.
+"""
+
+    def test_keep_them_all(self):
+        self.run_script("""
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_adopt_child(self):
+        self.run_script("""
+$ bzr mv dir/file2 file2
+$ bzr rm dir --force
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_kill_them_all(self):
+        self.run_script("""
+$ bzr rm dir --force
+$ bzr resolve dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_this(self):
+        self.run_script("""
+$ bzr resolve --take-this dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_other(self):
+        self.run_script("""
+$ bzr resolve --take-other dir
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+
+class TestResolvePathConflict(TestResolveConflicts):
+
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ echo 'Boo!' >file
+$ bzr add
+$ bzr commit -m 'Create trunk'
+$ bzr mv file file-in-trunk
+$ bzr commit -m 'Renamed to file-in-trunk'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ bzr mv file file-in-branch
+$ bzr commit -m 'Renamed to file-in-branch'
+
+$ bzr merge ../trunk
+2>R   file-in-branch => file-in-trunk
+2>Path conflict: file-in-branch / file-in-trunk
+2>1 conflicts encountered.
+"""
+
+    def test_keep_source(self):
+        self.run_script("""
+$ bzr resolve file-in-trunk
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_keep_target(self):
+        self.run_script("""
+$ bzr mv file-in-trunk file-in-branch
+$ bzr resolve file-in-branch
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_this(self):
+        self.run_script("""
+$ bzr resolve --take-this file-in-branch
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_other(self):
+        self.run_script("""
+$ bzr resolve --take-other file-in-branch
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+
+class TestResolveParentLoop(TestResolveConflicts):
+
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ bzr mkdir dir1
+$ bzr mkdir dir2
+$ bzr commit -m 'Create trunk'
+$ bzr mv dir2 dir1
+$ bzr commit -m 'Moved dir2 into dir1'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ bzr mv dir1 dir2
+$ bzr commit -m 'Moved dir1 into dir2'
+
+$ bzr merge ../trunk
+2>Conflict moving dir2/dir1 into dir2.  Cancelled move.
+2>1 conflicts encountered.
+"""
+
+    def test_take_this(self):
+        self.run_script("""
+$ bzr resolve dir2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_take_other(self):
+        self.run_script("""
+$ bzr mv dir2/dir1 dir1
+$ bzr mv dir2 dir1
+$ bzr resolve dir2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_this(self):
+        self.run_script("""
+$ bzr resolve --take-this dir2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+        self.failUnlessExists('dir2')
+
+    def test_resolve_taking_other(self):
+        self.run_script("""
+$ bzr resolve --take-other dir2
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+        self.failUnlessExists('dir1')
+
+
+class TestResolveNonDirectoryParent(TestResolveConflicts):
+
+    preamble = """
+$ bzr init trunk
+$ cd trunk
+$ bzr mkdir foo
+$ bzr commit -m 'Create trunk'
+$ echo "Boing" >foo/bar
+$ bzr add foo/bar
+$ bzr commit -m 'Add foo/bar'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ rm -r foo
+$ echo "Boo!" >foo
+$ bzr commit -m 'foo is now a file'
+
+$ bzr merge ../trunk
+2>+N  foo.new/bar
+2>RK  foo => foo.new/
+# FIXME: The message is misleading, foo.new *is* a directory when the message
+# is displayed -- vila 090916
+2>Conflict: foo.new is not a directory, but has files in it.  Created directory.
+2>1 conflicts encountered.
+"""
+
+    def test_take_this(self):
+        self.run_script("""
+$ bzr rm foo.new --force
+# FIXME: Isn't it weird that foo is now unkown even if foo.new has been put
+# aside ? -- vila 090916
+$ bzr add foo
+$ bzr resolve foo.new
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_take_other(self):
+        self.run_script("""
+$ bzr rm foo --force
+$ bzr mv foo.new foo
+$ bzr resolve foo
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_this(self):
+        self.run_script("""
+$ bzr resolve --take-this foo.new
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+    def test_resolve_taking_other(self):
+        self.run_script("""
+$ bzr resolve --take-other foo.new
+$ bzr commit --strict -m 'No more conflicts nor unknown files'
+""")
+
+
+class TestMalformedTransform(script.TestCaseWithTransportAndScript):
+
+    def test_bug_430129(self):
+        # This is nearly like TestResolveNonDirectoryParent but with branch and
+        # trunk switched. As such it should certainly produce the same
+        # conflict.
+        self.run_script("""
+$ bzr init trunk
+$ cd trunk
+$ bzr mkdir foo
+$ bzr commit -m 'Create trunk'
+$ rm -r foo
+$ echo "Boo!" >foo
+$ bzr commit -m 'foo is now a file'
+
+$ bzr branch . -r 1 ../branch
+$ cd ../branch
+$ echo "Boing" >foo/bar
+$ bzr add foo/bar
+$ bzr commit -m 'Add foo/bar'
+
+$ bzr merge ../trunk
+2>bzr: ERROR: Tree transform is malformed [('unversioned executability', 'new-1')]
+""")
+
+
+class TestResolveActionOption(tests.TestCase):
+
+    def setUp(self):
+        super(TestResolveActionOption, self).setUp()
+        self.options = [conflicts.ResolveActionOption()]
+        self.parser = option.get_optparser(dict((o.name, o)
+                                                for o in self.options))
+
+    def parse(self, args):
+        return self.parser.parse_args(args)
+
+    def test_unknown_action(self):
+        self.assertRaises(errors.BadOptionValue,
+                          self.parse, ['--action', 'take-me-to-the-moon'])
+
+    def test_done(self):
+        opts, args = self.parse(['--action', 'done'])
+        self.assertEqual({'action':'done'}, opts)
+
+    def test_take_this(self):
+        opts, args = self.parse(['--action', 'take-this'])
+        self.assertEqual({'action': 'take_this'}, opts)
+        opts, args = self.parse(['--take-this'])
+        self.assertEqual({'action': 'take_this'}, opts)
+
+    def test_take_other(self):
+        opts, args = self.parse(['--action', 'take-other'])
+        self.assertEqual({'action': 'take_other'}, opts)
+        opts, args = self.parse(['--take-other'])
+        self.assertEqual({'action': 'take_other'}, opts)

=== modified file 'bzrlib/workingtree.py'
--- bzrlib/workingtree.py	2010-01-31 12:05:38 +0000
+++ bzrlib/workingtree.py	2010-02-04 13:06:32 +0000
@@ -111,6 +111,9 @@
 
 
 MERGE_MODIFIED_HEADER_1 = "BZR merge-modified list format 1"
+# TODO: Modifying the conflict objects or their type is currently nearly
+# impossible as there is no clear relationship between the working tree format
+# and the conflict list file format.
 CONFLICT_HEADER_1 = "BZR conflict list format 1"
 
 ERROR_PATH_NOT_FOUND = 3    # WindowsError errno code, equivalent to ENOENT