This is an archive of the discontinued Mercurial Phabricator instance.

rebase: change "result would have 3 parent" error message (BC)
ClosedPublic

Authored by quark on Aug 11 2017, 2:13 AM.

Details

Summary

The old error message "cannot use revision REV as base, result would have 3
parents" is confusing - why use REV as base? why add a new parent?.

This patch changes it to "cannot move parent", which seems better.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

quark created this revision.Aug 11 2017, 2:13 AM
martinvonz added inline comments.
hgext/rebase.py
1087–1096

I feel like "Cannot rebase merge commit %d without rebasing at least one its parents" would be more specific and still be accurate.

It would be nice if we could detect the case upfront, but if we determine the destination dynamically including looking at obsmarkers, that may be difficult

quark planned changes to this revision.Aug 12 2017, 2:12 PM
quark added inline comments.
hgext/rebase.py
1087–1096

I like the new error message. Will change.

A dry-run of rebase is not feasible until we have in-memory DAG operations, which is one of my future interests.

quark added a comment.Aug 12 2017, 8:35 PM

I guess the old error message was intended for the below case:

$ hg debugdrawdag <<'EOF'
>   H
>  /|
> D F
> | |\
> C E G
>  \|/
>   B Z
>   |/
>   A
> EOF
$ hg rebase -d Z -r 'C+E+G+H'

Removing D, having C connected to H directly is another interesting case. I think adjustdest could be changed to detect that situation (when adjusting H's destination considering its parent F, there are multiple candidates - E and G that may worth a warning or an error. Things could be more fun if E and G has obsolete or ancestor relationship).

quark edited the summary of this revision. (Show Details)Aug 13 2017, 12:10 AM
quark updated this revision to Diff 831.
quark updated this revision to Diff 892.Aug 14 2017, 8:09 PM
martinvonz accepted this revision.Aug 15 2017, 12:31 AM
This revision is now accepted and ready to land.Aug 15 2017, 12:31 AM
This revision was automatically updated to reflect the committed changes.