[MERGE RFC] Added option --remember to merge command

[Michael Ellerman]

Hi Olaf,

Nice work, a few minor comments below ..

On 3/18/06, Olaf Conradi <oohlaf at gmail.com> wrote:
> === modified file 'a/bzrlib/builtins.py'
> --- a/bzrlib/builtins.py
> +++ b/bzrlib/builtins.py
> @@ -1779,6 +1779,10 @@
>
>  class cmd_merge(Command):
>      """Perform a three-way merge.
> +
> +    If there is no default location set, the first merge will set it. After
> +    that, you can omit the location to use the default.  To change the
> +    default, use --remember.

I think you should put this help text below the existing help text,
perhaps just before the examples.

>      The branch is the branch you will merge from.  By default, it will
>      merge the latest revision.  If you specify a revision, that
> @@ -1812,16 +1816,17 @@
>      --force is given.
>      """
>      takes_args = ['branch?']
> -    takes_options = ['revision', 'force', 'merge-type', 'reprocess',
> +    takes_options = ['remember', 'revision', 'force', 'merge-type',
> 'reprocess',

I'd normally put the new option at the end of the list, but it's not
that important.

> -    def run(self, branch=None, revision=None, force=False, merge_type=None,
> +    def run(self, branch=None, remember=False, revision=None,
> force=False, merge_type=None,
>              show_base=False, reprocess=False):

However this is not a good idea, by putting remember in front of
revision you've changed the order of the arguments. So if someone is
calling run() without using keyword arguments they will break
(although _probably_ no one is), just put remember as the last option.

> -            branch = WorkingTree.open_containing(u'.')[0].branch.get_parent()
> +            branch = tree.branch.get_parent()

I know the current code uses branch, but it might be clearer to call
it other_branch while you're changing this code.

>              if conflict_count != 0:
>                  return 1
>              else:
> +                if tree.branch.get_parent() is None or remember:
> +                    tree.branch.set_parent(branch)

I actually think you should do this before the if block. Having
conflicts in the merge isn't really an error, so it'd be confusing if
I did:

# bzr merge --remember ../other
bzr: conflicts detected in merge, but that's ok, just fix them :D
#
...
# bzr merge ../other
bzr: Error: No merge location known or specified.

Because I said --remember and bzr didn't give me an error, so why
didn't it remember it!

cheers