On 11/06/18 16:49, Elijah Newren wrote: > Another thing I missed... > > On Sun, Jun 10, 2018 at 12:40 PM, Phillip Wood > <phillip.wood@xxxxxxxxxxxx> wrote: >> On 07/06/18 06:06, Elijah Newren wrote: > >>> Some exceptions I left out: >>> >>> * --merge and --interactive are technically incompatible since they are >>> supposed to run different underlying scripts, but with a few small >>> changes, --interactive can do everything that --merge can. In fact, >>> I'll shortly be sending another patch to remove git-rebase--merge and >>> reimplement it on top of git-rebase--interactive. >> >> Excellent I've wondered about doing that but never got round to it. One >> thing I was slightly concerned about was that someone maybe parsing the >> output of git-rebase--merge and they'll get a nasty shock when that output >> changes as a result of using the sequencer. > > I can see the minor worry, but I think upon inspection it's not > something that concerns me, for a few reasons: > > In terms of use, given that rebase --merge was introduced to handle > renames in mid-2006, but plain rebase has been able to handle them > since late 2008 (though directory renames changes that again), the > utility of rebase --merge has been somewhat limited. I think that > limits the exposure. But to address the 'break' more directly... > > If we were to agree that we needed to support folks parsing rebase > output, that would be a really strict requirement that I think would > prevent lots of fixes. And if so, it's one we've violated a number of > times. For example, I certainly wasn't thinking about rebase when I > modified messages in merge-recursive.c over the years, but they'd leak > through for rebase --merge. (Those messages would not leak through to > rebase --interactive as much, since the sequencer sets o.buffer_output > and then only conditionally shows the output.) Also, changes that > have occurred in the past, like adding 'git gc --auto' to rebase, > modifying error messages directly found in git-rebase--merge.sh would > have been considered breaks. > > Finally, looking over all the differences in output while fixing up > testcases makes me think we've done much less around designing the > output based on what we want the user to see, and more around what > minimal fixups can we do to these lower level commands that provide > useful functionality to the user? We linearize history differently > for different rebase modes, have different entries in the reflog > depending on which mode, and we often times implement features for > just one mode and then sometimes add it to others. In fact, I think > the primary reason that am-based and merge-based rebases had a --quiet > option and the interactive rebases didn't, is mostly attributable to > the defaults of the lower level commands these three were built on top > of (git-am vs. git-merge-recursive vs. git-cherry-pick). The noiser > two merited a quiet option, and the option was never added for the > last one. > > Anyway, that's my rationale. I'm curious to hear if folks disagree or > see things I'm overlooking or have reasons I might be weighting > tradeoffs less than optimally. > I agree that there are already plenty of inconsistencies, (it's great to see you reducing them). If we can avoid emulating the ouput of git-rebase--merge.sh in sequencer.c that would definitely be my preferred option (the code is already a bit hard to follow in places where there it's doing slightly different things for cherry-pick and rebase -i). Hopefully no one is relying on the output, as you say it's just whatever the underlying plumbing prints rather than designed for a specific purpose. Best Wishes Phillip
