On 28/04/17 20:22, Johannes Schindelin wrote: > Hi Philip, > > On Fri, 28 Apr 2017, Phillip Wood wrote: > >> On 26/04/17 12:59, Johannes Schindelin wrote: >> >>> The first step of an interactive rebase is to generate the so-called >>> "todo script", to be stored in the state directory as >>> "git-rebase-todo" and to be edited by the user. >>> >>> Originally, we adjusted the output of `git log <options>` using a >>> simple sed script. Over the course of the years, the code became more >>> complicated. We now use shell scripting to edit the output of `git >>> log` conditionally, depending whether to keep "empty" commits (i.e. >>> commits that do not change any files). >>> >>> On platforms where shell scripting is not native, this can be a >>> serious drag. And it opens the door for incompatibilities between >>> platforms when it comes to shell scripting or to Unix-y commands. >>> >>> Let's just re-implement the todo script generation in plain C, using >>> the revision machinery directly. >>> >>> This is substantially faster, improving the speed relative to the >>> shell script version of the interactive rebase from 2x to 3x on >>> Windows. >> >> This changes the behaviour of git -c rebase.instructionFormat= rebase -i >> The shell version treats the rebase.instructionFormat being unset or set >> to the empty string as equivalent. This version generates a todo list >> with lines like 'pick <abbrev sha1>' rather than 'pick <abbrev sha1> >> <subject>' >> >> I only picked this up because I have a script that does 'git -c >> rebase.instructionFormat= rebase -i' with a custom sequence editor. I >> can easily add '%s' in the appropriate place but I thought I'd point it >> out in case other people are affected by the change. > > While I would argue that the C version is more correct, it would be > backwards-incompatible.
I was going to make a point about resetting config variables to their default value on the command line but Junio beat me to it. > So I changed it. That's great, thanks > BTW in the future you could help me a *lot* by providing a patch that adds > a test case to our test suite that not only demonstrates what exactly goes > wrong, but also will help prevent future regressions. I'll bear that in mind, it does assume that reporters have a good understanding of the test suite layout and helper functions though. Is there a particular reason you put the test case in the autosquash tests? I wouldn't have thought of doing that. Thanks again Phillip
