[merge] Re: how to revert update operation

[John Arbash Meinel]

Adeodato Simó wrote:
> * Robert Collins [Thu, 20 Jul 2006 13:19:49 +1000]:
> 
>> On Thu, 2006-07-20 at 05:02 +0200, Adeodato Simó wrote:
> 
>>> * Robert Collins [Thu, 20 Jul 2006 12:32:55 +1000]:
> 
>>>> We should not offer --local, or push, if its a lightweight checkout
>>>> we're working with rather than a heavyweight checkout. (because they
>>>> dont apply there).
> 
>>> Oh, right, forgot about lightcheckouts. Is it reasonable to check for
>>> that, and only add the second sentence for heavyweight checkouts?
> 
>> I think so.
> 
> Oh, huh. After looking more carefully, I disagree that there is the need
> for a check.
> 
> At first, and following your suggestion, I went and wrote something like:
> 
>   except errors.BoundBranchOutOfDate, e:
>       error = "To commit to master branch, do update and then commit"
>       if not lightweight_checkout:
>           error += "\nYou can also pass --local to commit to continue " \
>                    "working offline."
> 
> However, I realized afterwards that that exception is not raised for
> lightweight checkouts (they get OutOfDateTree), _except_ if they are a
> lightweight checkout _of a checkout_, in which case both --local and
> push work.
> 
> With the above explanations, I think that the attached patch should be
> ok. Note that I've left the reference to push in place, since it's not
> clear how http://launchpad.net/bugs/53493 will be fixed (particularly
> after John's comment), so I'd rather have it mentioned for now, since I
> really believe it can be of help to users.

Just to be clear. I do think we should have commit push revisions. But I
wanted to bring up one of the reasons it wasn't, in case it comes up
later, or someone decides that it is a big deal.

> 
> And I've also added Erik's suggestion for update, that is, to print a
> note saying what happened to the local commits.
> 
> Feel free to make me iterate over the patch again, or address this
> concerns in another one of yours, but I think useability would improve a
> bit with it. :)
> 
> Thanks.
> 

...

> === modified file bzrlib/builtins.py
> --- bzrlib/builtins.py
> +++ bzrlib/builtins.py
> @@ -826,6 +826,9 @@
>              conflicts = tree.update()
>              revno = tree.branch.revision_id_to_revno(tree.last_revision())
>              note('Updated to revision %d.' % (revno,))
> +            if tree.pending_merges():
> +                note('Your local commits will show now as pending merges with '
> +                     '`bzr status`, and can be commited with `bzr commit`.')
>              if conflicts != 0:
>                  return 1

The note looks pretty good. 3 comments:
1) I don't think we use backticks when reporting things to the user. I
tried to grep through the source, and didn't find any. (There are some
in docs, but that is because it means something to rST). So I would prefer:
 "'bzr status', and can be committed with 'bzr commit'."

2) committed has 2 t's

3) If you did 'bzr merge ../other/tree' and then 'bzr upgrade' because
you tree was out of date, you wouldn't have any local commits, but you
would have pending merges. So probably the check should be done earlier,
and carried over to here to be reported.


>              else:
> @@ -1754,9 +1757,10 @@
>              raise BzrCommandError("Commit refused because there are unknown "
>                                    "files in the working tree.")
>          except errors.BoundBranchOutOfDate, e:
> -            raise BzrCommandError(str(e)
> -                                  + ' Either unbind, update, or'
> -                                    ' pass --local to commit.')
> +            raise BzrCommandError(str(e) + "\n"
> +                'To commit to master branch, do update and then commit, or '  \
> +                'push if the branches have not diverged.\nYou can also pass ' \
> +                '--local to commit to continue working offline.')

I wouldn't put '\n' in the middle string if I was splitting across
lines. So more like:
'push if the branches have not diverged.\n'
'You can also...'

That makes it more obvious that we are breaking the line.

You don't need '\' at the end of the line, because you are inside
parentheses, so you already have a continuation until the final closing
parenthesis.

I don't think we should be getting 'OutOfDate' if the child branch is a
superset of the master branch. Some other error would be okay, but it
should be distinguished whether the parent has more entries (need to
update), child has more entries (need to push), or both have entries
(need to merge).

But if fixing that is too complex, try this text:

To commit to master branch, run update (or push if the child has local
commits).
You can also use 'commit --local' to continue working disconnected.

(I don't *really* like "working disconnected", but the user isn't
offline at this point).

John
=:->