On Mon, Jun 18, 2018 at 6:19 AM Alban Gruin <alban.gruin@xxxxxxxxx> wrote:
>
> This rewrites setup_reflog_action() from shell to C.
>
> A new command is added to rebase--helper.c, “setup-reflog”, as such as a
> new flag, “verbose”, to silence the output of the checkout operation
> called by setup_reflog_action().
>
> The shell version is then stripped in favour of a call to the helper. As
> $GIT_REFLOG_ACTION is not longer set at the first call of
> checkout_onto(), a call to comment_for_reflog() is added at the
> beginning of this function.
>
> Signed-off-by: Alban Gruin <alban.gruin@xxxxxxxxx>
> ---
> builtin/rebase--helper.c | 9 +++++++--
> git-rebase--interactive.sh | 16 ++--------------
> sequencer.c | 31 +++++++++++++++++++++++++++++++
> sequencer.h | 3 +++
> 4 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
> index d2990b210..d677fb663 100644
> --- a/builtin/rebase--helper.c
> +++ b/builtin/rebase--helper.c
> @@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
> int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
> {
> struct replay_opts opts = REPLAY_OPTS_INIT;
> - unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
> + unsigned flags = 0, keep_empty = 0, rebase_merges = 0, verbose = 0;
> int abbreviate_commands = 0, rebase_cousins = -1;
> enum {
> CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
> CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
> - ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
> + ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO, SETUP_REFLOG
> } command = 0;
> struct option options[] = {
> OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
> @@ -27,6 +27,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
> OPT_BOOL(0, "rebase-merges", &rebase_merges, N_("rebase merge commits")),
> OPT_BOOL(0, "rebase-cousins", &rebase_cousins,
> N_("keep original branch points of cousins")),
> + OPT_BOOL(0, "verbose", &verbose, N_("verbose")),
verbose is quite a popular flag name, such that the option parsing
dedicated it its own macro OPT__VERBOSE.
> +int setup_reflog_action(struct replay_opts *opts, const char *commit,
> + int verbose)
> +{
> + const char *action;
> +
> + if (commit && *commit) {
While this is defensive programming (checking the pointer before dereferencing
it, the first condition (commit == NULL) should never be false here,
as the caller
checks for argc == 2 ? Maybe we could move the logic of the whole
condition there
if (command == SETUP_REFLOG && argc == 2 && *argv[1])
return !!setup_reflog_action(&opts, argv[1], verbose);
as then we could loose the outer conditional here.
