Hi,
this reroll applies the comments from Eric, Junio and Michael. In
particular,
* 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
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(-)
--
2.0.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
