Subject: Re: [PATCH v2 05/34] sequencer (rebase -i): learn about the 'verbose' mode

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Dec 2016 13:51:36 -0800

Johannes Schindelin <[email-masked]> writes:

> @@ -1493,9 +1498,26 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
>  	}
>  
>  	if (is_rebase_i(opts)) {
> +		struct strbuf buf = STRBUF_INIT;
> +
>  		/* Stopped in the middle, as planned? */
>  		if (todo_list->current < todo_list->nr)
>  			return 0;
> +
> +		if (opts->verbose) {
> +			const char *argv[] = {
> +				"diff-tree", "--stat", NULL, NULL
> +			};
> +
> +			if (!read_oneliner(&buf, rebase_path_orig_head(), 0))
> +				return error(_("could not read '%s'"),
> +					rebase_path_orig_head());
> +			strbuf_addstr(&buf, "..HEAD");
> +			argv[2] = buf.buf;
> +			run_command_v_opt(argv, RUN_GIT_CMD);
> +			strbuf_reset(&buf);
> +		}
> +		strbuf_release(&buf);
>  	}

It's a bit curious that the previous step avoided running a separate
process and instead did "diff-tree -p" all in C, but this one does not.

I think it is because this one is outside the loop?  The original,
being a scripted Porcelain, formulates a lazy and loose command
line, but you may want to tighten it up a bit if you spawn a
process.  If your user happens to have a file whose name is
$orig_head..HEAD, the command line you are creating (which is
identical to the scripted version) will barf with "ambiguous
argument".

One good thing about a complete C rewrite is that it won't have an
issue like this one because you'd be working with in-core objects.

> diff --git a/sequencer.h b/sequencer.h
> index cb21cfddee..f885b68395 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -24,6 +24,7 @@ struct replay_opts {
>  	int allow_empty;
>  	int allow_empty_message;
>  	int keep_redundant_commits;
> +	int verbose;
>  
>  	int mainline;