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

[Jeff King <peff@peff.net>]

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

> > 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