[Commented On] D7177: rebase: introduce optional parent mapping

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D7177#128760 <https://phab.mercurial-scm.org/D7177#128760>, @martinvonz wrote:
  
  > In D7177#128757 <https://phab.mercurial-scm.org/D7177#128757>, @marmoute wrote:
  >
  >> In D7177#128756 <https://phab.mercurial-scm.org/D7177#128756>, @martinvonz wrote:
  >>
  >>> In D7177#128740 <https://phab.mercurial-scm.org/D7177#128740>, @marmoute wrote:
  >>>
  >>>> In D7177#128389 <https://phab.mercurial-scm.org/D7177#128389>, @martinvonz wrote:
  >>>>
  >>>>> Maybe another option is to allow multiple `-d` arguments for this case? Something like `hg rebase -r C -d B -d D`. I haven't thought through BC, but I think that's what I'd prefer if we were writing rebase from scratch. I know we can't support `hg rebase -r C -d 'B + D'` for backward-compatibility reasons (because we already support that -- it rebases to the highest revnum in the set).
  >>>>
  >>>> That's interresting. How would that works with a practical case ?
  >>>
  >>> The example I have was meant to be a practical example for how to do it in the case given in the patch description :) But let me know if you meant some other aspect of "how it works". As I said, I'm not sure ant the BC bit of it at least.
  >>
  >> I meant how that would work in general, esepcially case more complicated than the basic base shown by @joerg.sonnenberger.
  >
  > I think the parents chosen by `-d` should only apply to the roots of the rebase set. If your rebase set has multiple roots and only some of them are merge commits, then we should probably error out.
  
  So you suggestion would be to allow to specify two -d that would definive the two new parents of the root ? I suspect it would be a bit too narrow compared to the varienty of case we might have to handle. Maybe this is a mission for PlanPlan.
  
  > Anyway, I'm not sure it's realistic to make it work that way now, especially because of how inconsistent it would be with other arguments that accept revsets and only care about the highest revnum in the set (I think that was a mistake to do, but I understand why it was done that way and it's obviously too late to change it).
  
  I think the multiple -d value approach would works fine. (but for the other issue).
  
  >> Not that in @joerg base, B being ancestors of both C and C', we could probably behave better by default directly. Could we not ?
  >
  > As @joerg said in the patch description, there's no reasonable way to choose which of A and B to preserve (the current version of rebase wouldn't preserve either and would instead rebase all of A, B, and C).
  
  The description got me confused, (I though C was rebased on C' (finding the naming strange)) while this is about C being rebased on D.

INLINE COMMENTS

> rebase.py:884
> +            _(b'map old parent to new parent (ADVANCED)'),
> +            _(b'REV:REV')),
>          (b'k', b'keep', False, _(b'keep original changesets')),

`REV:REV` is valid revset so the result is quite confusing, if we go that route, we need anothr syntax.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7177/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7177

To: joerg.sonnenberger, martinvonz, #hg-reviewers, baymax
Cc: mercurial-patches, marmoute, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200610/102b25f7/attachment-0002.html>