On Thu, Jun 21, 2018 at 12:47 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Elijah Newren <newren@xxxxxxxxx> writes: > >> @@ -280,8 +286,10 @@ other words, the sides are swapped. >> + >> Because 'git rebase' replays each commit from the working branch >> on top of the <upstream> branch using the given strategy, using >> -the 'ours' strategy simply discards all patches from the <branch>, >> +the 'ours' strategy simply empties all patches from the <branch>, > > I think overall the direction this series takes, and especially what > this step does---clarify what the current code & design does first > before attempting to improve the status quo, makes sense, but I had > trouble justifying this change. Do you want to emphasize that the > operation discards the changes but still leaves empty commits, and > "discards all patches" imply there won't be any commits left on top > of the "onto" point? Yes, is there an alternative wording that you'd find more clear? (Also, I'd like to push to make rebase -i/-m behave the same as am-based rebase with empty commits, meaning they are just discarded by default rather than halted at. But I haven't gotten to that point...) >> @@ -487,6 +510,52 @@ recreates the topic branch with fresh commits so it can be remerged >> successfully without needing to "revert the reversion" (see the >> link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for details). >> >> +INCOMPATIBLE OPTIONS >> +-------------------- >> + >> +git-rebase has many flags that are incompatible with each other, >> +predominantly due to the fact that it has three different underlying >> +implementations: >> + >> + * one based on linkgit:git-am[1] (the default) >> + * one based on linkgit:git-merge-recursive[1] (merge backend) >> + * one based on linkgit:git-cherry-pick[1] (interactive backend) >> + >> +Flags only understood by the am backend: >> + >> + * --committer-date-is-author-date >> + * --ignore-date >> + * --whitespace >> + * --ignore-whitespace >> + * -C >> + >> +Flags understood by both merge and interactive backends: >> + >> + * --merge >> + * --strategy >> + * --strategy-option >> + * --allow-empty-message >> + >> +Flags only understood by the interactive backend: >> + >> + * --[no-]autosquash >> + * --rebase-merges >> + * --preserve-merges >> + * --interactive >> + * --exec >> + * --keep-empty >> + * --autosquash >> + * --edit-todo >> + * --root when used in combination with --onto >> + >> +Other incompatible flag pairs: >> + >> + * --preserve-merges and --interactive >> + * --preserve-merges and --signoff >> + * --preserve-merges and --rebase-merges >> + * --rebase-merges and --strategy >> + * --rebase-merges and --strategy-option >> + >> include::merge-strategies.txt[] > > It is a bit unclear what these lists want to say. If --ignore-date > is only understood by "rebase" and not "rebase -i", wouldn't that > make "--interactive and --ignore-date" an incompatible flag pair? > Or do you mean the "Other incompatible" list to enumerate the ones > that are explicitly rejected, as opposed to getting silently > ignored? Yes, --interactive and --ignore-date are an incompatible flag pair but I didn't want to list all 65 of those pairs explicitly. Anything that only works by being passed to git-am is incompatible with anything that implies the --merge or --interactive backends. The list becomes simpler once we nuke git-rebase--merge.sh, implementing it atop git-rebase--interactive.sh (which comes in a separate series that depends on this one). > In any case, I'll need to read through the remainder of the series > to come to any conclusion, but given that -p is getting killed by > its primary author, with help from others to split out its > implementation from the --interactive codepath, it might be a bad > time to do this series. Thanks for working on this topic. This series doesn't conflict with ag/rebase-p at all; it merges cleanly with next and pu. The follow-on work to nuke git-rebase--merge.sh, however, does conflict so I'm waiting on (re-)sending it.
