Re: [GSoC][PATCH v5 2/3] rebase -i: rewrite setup_reflog_action() in C

[Junio C Hamano <gitster@pobox.com>]

Alban Gruin <alban.gruin@gmail.com> writes:

> +	git rebase--helper --prepare-branch "$switch_to" ${verbose:+--verbose}
>  	init_basic_state
>  
>  	init_revisions_and_shortrevisions
> diff --git a/sequencer.c b/sequencer.c
> index d9545b366..dd0cf3cb5 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3134,6 +3134,37 @@ static const char *reflog_message(struct replay_opts *opts,
>  	return buf.buf;
>  }
> ...
> +int prepare_branch_to_be_rebased(struct replay_opts *opts, const char *commit,
> +				 int verbose)
> +{
> +	const char *action;
> +
> +	if (commit && *commit) {

The sloppyness of this callee is forced by the sloppy caller, which
does not check if "$switch_to" has any value before calling "prepare
branch to be rebased" function.  A less sloppy caller would check to
see if we have the optional "before doing anything, check out this
branch, as it is the branch we are trying to rebase" argument, and
refrain from calling this function, so there is no need for this "if
commit is given and is not an empty string" check.

And hopefully, when GSoC is over, the caller, that is still in shell
at this step, would also be rewritten in C and by that time the
caller will become less sloppy, removing the need for this check.

Hence, I think the reason why the check is still here, and our
desire to eventually remove the check, should be documented with an
in-code comment around here (the usual "NEEDSWORK:" comment is
fine).