[PATCH] merge: let the user choose to merge, keep local or keep remote subrepo revisions

[Angel Ezquerra]

On Fri, Sep 20, 2013 at 4:52 PM, Augie Fackler <raf at durin42.com> wrote:
> On Sun, Sep 08, 2013 at 02:07:02AM +0200, Angel Ezquerra wrote:
>> On Sun, Sep 8, 2013 at 1:46 AM, Martin Geisler <martin at geisler.net> wrote:
>> > Angel Ezquerra <angel.ezquerra at gmail.com> writes:
>> >
>> >> # HG changeset patch
>> >> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> >> # Date 1378420708 -7200
>> >> #      Fri Sep 06 00:38:28 2013 +0200
>> >> # Node ID 15548e5b1c00bd2f261e3c81d39f2465263bc3c3
>> >> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
>> >> merge: let the user choose to merge, keep local or keep remote subrepo revisions
>> >>
>> >> When a subrepo has changed on the local and remote revisions, prompt
>> >> the user whether it wants to merge those subrepo revisions, keep the
>> >> local revision or keep the remote revision.
>> >>
>> >> Up until now mercurial would always perform a merge on a subrepo that
>> >> had changed on the local and the remote revisions. This is often
>> >> inconvenient. For example:
>> >>
>> >> - You may want to perform the actual subrepo merge after you have
>> >> merged the parent subrepo files.
>> >>
>> >> - Some subrepos may be considered "read only", in the sense that you
>> >> are not supposed to add new revisions to them. In those cases "merging
>> >> a subrepo" means choosing which _existing_ revision you want to use on
>> >> the merged revision. This is often the case for subrepos that contain
>> >> binary dependencies (such as DLLs, etc).
>> >>
>> >> This new prompt makes mercurial better cope with those common
>> >> scenarios.
>> >>
>> >> Notes:
>> >>
>> >> - The default behavior (which is the one that is used when ui is not
>> >> interactive) remains unchanged (i.e. merge is the default action).
>> >> - This prompt will be shown even if the ui --tool flag is set.
>> >> - I don't know of a way to test the "keep local" and "keep remote"
>> >> options (i.e. to force the test to choose those options).
>> >>
>> >> diff -r 1d07bf106c2a -r 15548e5b1c00 mercurial/subrepo.py
>> >> --- a/mercurial/subrepo.py    Wed Sep 04 18:42:55 2013 -0700
>> >> +++ b/mercurial/subrepo.py    Fri Sep 06 00:38:28 2013 +0200
>> >> @@ -201,9 +201,24 @@
>> >>                  wctx.sub(s).get(r, overwrite)
>> >>                  sm[s] = r
>> >>              else:
>> >> -                debug(s, "both sides changed, merge with", r)
>> >> -                wctx.sub(s).merge(r)
>> >> -                sm[s] = l
>> >> +                debug(s, "both sides changed")
>> >> +                option = repo.ui.promptchoice(
>> >> +                    _(' subrepository %s diverged (local revision: %s, '
>> >> +                      'remote revision: %s)\n'
>> >> +                      '(M)erge, keep (l)ocal or keep (r)emote?'
>> >> +                      '$$ &Merge $$ &Local $$ &Remote')
>> >> +                    % (s, l[1][:12], r[1][:12]), 0)
>> >
>> > I can see in the tests that one can answer "m" to this prompt, but the
>> > "(M)erge" suggests to me that I should answer "M". I quickly checked the
>> > other prompts and I think all the other options are in lowercase.
>>
>> Martin,
>>
>> thanks for reviewing this patch.
>>
>> I hesitated when deciding the case for that option. The description
>> example given on the ui.promptchoice method docstring seems to suggest
>> that the default option should be capitalized:
>>
>> "would you like fries with that (Yn)? $$ &Yes $$ &No"
>>
>> That being said I also saw that most (if not all) other prompts do not
>> follow that advice and usually are all lowercase, so I would not mind
>> changing it for consistency's sake.
>
> If 'm' is the default, then it should be captialized (this is pretty
> standard in CLI apps.) The prompt parser should accept both M and m,
> as well as L and l etc.

That is what I thought from looking at the example in the
ui.promptchoice docstring. I will not send a new version of the patch
then, unless you think there is something else that is worth
improving?

Cheers,

Angel