Re: Feature idea: git rebase --exec $CMD

From: Jeff King <peff@peff.net>
Date: May 6, 2012, 12:26 UTC
Message-ID: <20120506122649.GB26194@sigill.intra.peff.net>
In-Reply-To: <vpqobq1mxru.fsf@bauges.imag.fr>

On Sun, May 06, 2012 at 12:03:49PM +0200, Matthieu Moy wrote:

Show 11 quoted lines

> > Maybe this -x option should conflict with -i to simplify its "execute
> > the command after each commit" semantics (what if it is combined with -i
> > and 'x/exec' lines?).
> 
> Actually, implementation-wise, it's simpler to have '-x' imply '-i', and
> suggest a todo-list containing 'x' lines. Then, the code would simply
> have to add these "x whatever" lines, and let the
> "git-rebase--interactive.sh" mechanics do the job. That would show the
> "x whatever" lines to the user, but that can be seen as added value,
> since it gives an opportunity to the user to remove or edit some of them
> if needed.

Yeah, that makes a lot of sense to me. FWIW, I use this trick now:

  GIT_EDITOR='sed -i "/^pick .*/aexec $test"' \
  git rebase -i "$@"

to test individual commits on a topic before publishing it. But it would be awesome to do:

  git rebase -ix "$test" "$@"

instead (and clean up the quoting disaster waiting to happen in my sed invocation). We should perhaps start slow and call this "--exec" instead of stealing the short-and-sweet "-x" until the feature is more proven, though.

> I'm not familiar with the code behind non-interactive rebase, but it
> doesn't seem to use the same todo-list at all. Maybe the sequencer would
> help, I don't know.

With "-m", it's basically just a for loop over the commits, so I don't know that it would be too hard, but there may be bad interactions. With stock rebase using the "git-rebase--am" backend, it's a bit harder, as we are just bulk-feeding the contents between format-patch and am.

However, I like that the "-i" case already has a concept of execute-and-stop-if-fail, and that we can just build on that. I hope one day that it will all be unified via the sequencer code, but for now, it's not. Having the option mean "just add some exec lines to the todo file" is very simple and not likely to cause bugs.

As tempting as it would be to have "-x" imply "-i", I think it makes sense for it to simply fail in the non-interactive case (and say "sorry, not supported yet"). Then people can experiment with making it work for the non-interactive case (or when the non-interactive case eventually just uses the same code without the editor invocation), we won't be trapped into always having "-x" start the editor.

-Peff