[PATCH 2 of 3] rebase: choose default destination the same way as 'hg merge' (BC)

[Augie Fackler]

On Thu, Feb 18, 2016 at 12:05:26AM +0900, Yuya Nishihara wrote:
> On Mon, 15 Feb 2016 10:49:15 +0000, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at fb.com>
> > # Date 1455456359 0
> > #      Sun Feb 14 13:25:59 2016 +0000
> > # Node ID 1b45c38d02ddca7ff763366e732528c658ccb13c
> > # Parent  dfac70f47ed5ac6802eba6a331f36fb843c81bc7
> > # EXP-Topic destination
> > # Available At http://hg.netv6.net/marmoute-wip/mercurial/
> > #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r 1b45c38d02dd
> > rebase: choose default destination the same way as 'hg merge' (BC)
>
> I like the new behavior even though it is a big BC.
>
> > This changeset finally make 'hg rebase' choose its default destination using the
> > same logic as 'hg merge'. The previous default was "tipmost changeset on the
> > current branch", the new default is "the other head if there is only one".
>
> "help rebase" still says "the current branch tip as the destination." We'll
> have to update it.

+1 - I'd like a v2 that at least has this fix. I don't feel strongly
about the other comments below, but for now I'll await a v2 of the
series (eagerly, as this is a huge win for users everywhere).

>
> >  @revsetpredicate('_destrebase')
> >  def _revsetdestrebase(repo, subset, x):
> >      # ``_rebasedefaultdest()``
> >
> >      # default destination for rebase.
> >      # # XXX: Currently private because I expect the signature to change.
> > -    # # XXX: - taking rev as arguments,
> >      # # XXX: - bailing out in case of ambiguity vs returning all data.
> > -    # # XXX: - probably merging with the merge destination.
> >      # i18n: "_rebasedefaultdest" is a keyword
> > -    revset.getargs(x, 0, 0, _("_rebasedefaultdest takes no arguments"))
> > -    return subset & revset.baseset([_destrebase(repo)])
> > +    sourceset = None
> > +    if x is not None:
> > +        sourceset = revset.getset(repo, revset.fullreposet(repo), x)
> > +    return subset & revset.baseset([destutil.destmerge(repo, action='rebase',
> > +                                                       sourceset=sourceset,
> > +                                                       onheadcheck=False)])
>
> If this isn't an intermediate state of the rebase refactoring series, can we
> have a wrapper for destutil.destmerge() ? There are 4 copies.
>
> >  @command('rebase',
> >      [('s', 'source', '',
> >       _('rebase the specified changeset and descendants'), _('REV')),
> >      ('b', 'base', '',
> > @@ -279,10 +275,16 @@ def rebase(ui, repo, **opts):
> >          else:
> >              dest, rebaseset = _definesets(ui, repo, destf, srcf, basef, revf)
> >              if dest is None:
> >                  return _nothingtorebase()
> >
> > +            if not destf:
> > +                dest = repo[destutil.destmerge(repo, action='rebase',
> > +                                               sourceset=rebaseset,
> > +                                               onheadcheck=False)]
> > +                destf = str(dest)
>
> Is it necessary to recalculate dest here?
> If it is, perhaps _definesets() wouldn't do the right thing.
>
> >  def externalparent(repo, state, targetancestors):
> >      """Return the revision that should be used as the second parent
> >      when the revisions in state is collapsed on top of targetancestors.
> > @@ -1165,13 +1177,18 @@ def pullrebase(orig, ui, repo, *args, **
> >                      del opts['rev']
> >                  # positional argument from pull conflicts with rebase's own
> >                  # --source.
> >                  if 'source' in opts:
> >                      del opts['source']
> > -                if rebase(ui, repo, **opts) == _nothingtorebase():
> > +                try:
> > +                    rebase(ui, repo, **opts)
> > +                except error.NoMergeDestAbort:
> >                      rev, _a, _b = destutil.destupdate(repo)
> > -                    if rev != repo['.'].rev(): # we could update
> > +                    if rev == repo['.'].rev(): # we could update
> > +                        ui.status(_('nothing to rebase\n'))
> > +                    else:
>
> Nit: we could update on "else:" case
>
> > +                        ui.status(_('nothing to rebase - updating instead\n'))
> >                          # not passing argument to get the bare update behavior
> >                          # with warning and trumpets
> >                          commands.update(ui, repo)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel