this reroll applies the comments from Eric, Junio and Michael. In

 * It turned out that `pick_one` does not need option handling at all
   and the option-like argument `-n` determines whether `pick_one` or
   `do_pick` creates the replay commit. The latter happens if the
   task wants to rewrite the commit being picked (f.i., for the
   purpose of editing the log message or resetting the authorship).

   `do_pick` still seems to require a flexible parsing of options,
   i.e. a relatively expensive loop, since it receives the
   whitelisted user-supplied options. Unsupported and unknown options
   are treated as an "unknown command" error.

 * The `do_pick` options are documented in the same order they are
   listed in the function signature. Moreover, it is mentioned which
   options rewrite commits being picked.

 * The test cases output differing actual values as changes to the
   expected values and not the other way around. Moreover, the failed
   rebases are not cleaned up until the respective test succeeds.

Two stages (and two sub-stages) can be identified in the rerolled
patch series:

 1. Route primary to-do list commands through `do_pick`

     a. Implement reword in terms of do_pick (5/19)
     b. Implement squash in terms of do_pick (14/19)

 2. Add user options to main commands

    Enable options --signoff, --reset-author for pick, reword (19/19)

The last stage was added in this reroll. It enables the parsing of
line options for to-do list commands, which is still restricted to
options without arguments because it is unclear how spaces can be
parsed as characters rather than separators where needed. For
instance, if we were to support

    pick --author="A U Thor" fa1afe1 Some changes

then read(1) would hand us the tokens `--author="A`, `U` and `Thor"`
instead of `--author=A U Thor`, which we would want to relay to
`do_pick`. Interpreting the shell quoting would help. However,
eval(1) seems to disqualify itself as an interpreter because the
to-do list entry could potentially contain any shell command line.
This could be both a security and a usability issue. For this reason,
the patch series still hasn't graduated from being RFC.


Fabian Ruch (19):
  rebase -i: Failed reword prints redundant error message
  rebase -i: reword complains about empty commit despite --keep-empty
  rebase -i: reword executes pre-commit hook on interim commit
  rebase -i: Teach do_pick the option --edit
  rebase -i: Implement reword in terms of do_pick
  rebase -i: Stop on root commits with empty log messages
  rebase -i: The replay of root commits is not shown with --verbose
  rebase -i: Root commits are replayed with an unnecessary option
  rebase -i: Commit only once when rewriting picks
  rebase -i: Do not die in do_pick
  rebase -i: Teach do_pick the option --amend
  rebase -i: Teach do_pick the option --file
  rebase -i: Prepare for squash in terms of do_pick --amend
  rebase -i: Implement squash in terms of do_pick
  rebase -i: Explicitly distinguish replay commands and exec tasks
  rebase -i: Parse to-do list command line options
  rebase -i: Teach do_pick the option --reset-author
  rebase -i: Teach do_pick the option --signoff
  rebase -i: Enable options --signoff, --reset-author for pick, reword

 git-rebase--interactive.sh    | 277 ++++++++++++++++++++++++++++++++++--------
 t/t3404-rebase-interactive.sh |   8 ++
 t/t3412-rebase-root.sh        |  39 ++++++
 3 files changed, 273 insertions(+), 51 deletions(-)


To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to