Subject: Re: [PATCH 1/4] status: factor two rebase-related messages together

From: Junio C Hamano <[email-masked]>
Date: Mon, 08 Jun 2015 15:00:14 -0700

Guillaume Pagès  <[email-masked]> writes:

> ---
>  wt-status.c | 30 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)

Hmmm, it obviously does not break anything but it is not obvious why
this is a good change.

Is it that you wanted to have a single instance of "if on a branch,
we say 'you are rebasing that branch', otherwise we say 'you are
rebasing'"?  Even then, I am not sure if this code movement was the
best way to do so (an obvious alternative is to use a shared helper
function and call from the two arms of if/elseif/... chain).

>
> diff --git a/wt-status.c b/wt-status.c
> index 33452f1..fec6e85 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1032,7 +1032,7 @@ static void show_rebase_in_progress(struct wt_status *s,
>  {
>  	struct stat st;
>  
> -	if (has_unmerged(s)) {
> +	if (has_unmerged(s) || state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
>  		if (state->branch)
>  			status_printf_ln(s, color,
>  					 _("You are currently rebasing branch '%s' on '%s'."),
> @@ -1042,25 +1042,17 @@ static void show_rebase_in_progress(struct wt_status *s,
>  			status_printf_ln(s, color,
>  					 _("You are currently rebasing."));
>  		if (s->hints) {
> -			status_printf_ln(s, color,
> -				_("  (fix conflicts and then run \"git rebase --continue\")"));
> -			status_printf_ln(s, color,
> -				_("  (use \"git rebase --skip\" to skip this patch)"));
> -			status_printf_ln(s, color,
> -				_("  (use \"git rebase --abort\" to check out the original branch)"));
> +			if (has_unmerged(s)) {
> +				status_printf_ln(s, color,
> +					_("  (fix conflicts and then run \"git rebase --continue\")"));
> +				status_printf_ln(s, color,
> +					_("  (use \"git rebase --skip\" to skip this patch)"));
> +				status_printf_ln(s, color,
> +					_("  (use \"git rebase --abort\" to check out the original branch)"));
> +			} else
> +				status_printf_ln(s, color,
> +					_("  (all conflicts fixed: run \"git rebase --continue\")"));
>  		}
> -	} else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
> -		if (state->branch)
> -			status_printf_ln(s, color,
> -					 _("You are currently rebasing branch '%s' on '%s'."),
> -					 state->branch,
> -					 state->onto);
> -		else
> -			status_printf_ln(s, color,
> -					 _("You are currently rebasing."));
> -		if (s->hints)
> -			status_printf_ln(s, color,
> -				_("  (all conflicts fixed: run \"git rebase --continue\")"));
>  	} else if (split_commit_in_progress(s)) {
>  		if (state->branch)
>  			status_printf_ln(s, color,