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

[Angel Ezquerra]

El 20/09/2013 17:20, "Augie Fackler" <raf at durin42.com> escribió:
>
>
> On Fri, Sep 20, 2013 at 11:19 AM, Angel Ezquerra <angel.ezquerra at gmail.com>
wrote:
>>
>> 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?
>
>
> I don't see anything worth improving, but I also don't have any opinions
about subrepos, so someone else will need to review this.

Ok, thanks!

Angel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130920/564f49b2/attachment.html>