On Thu, Jun 21, 2018 at 12:47 PM, Junio C Hamano wrote:
> Elijah Newren 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 branch using the given strategy, using
>> -the 'ours' strategy simply discards all patches from the ,
>> +the 'ours' strategy simply empties all patches from the ,
>
> 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.
