Re: [PATCHv5 1/3] wt-status.*: better advices for git status added

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 1, 2012, 08:42 UTC
Message-ID: <vpq8vg7jue4.fsf@bauges.imag.fr>
In-Reply-To: <1338477344-15940-1-git-send-email-Lucien.Kong@ensimag.imag.fr>

Kong Lucien <Lucien.Kong@ensimag.imag.fr> writes:

> +	cat >expect <<-\EOF &&

[...]

Show 8 quoted lines

> +	# Untracked files:
> +	#   (use "git add <file>..." to include in what will be committed)
> +	#
> +	#	expect
> +	#	output
> +	no changes added to commit (use "git add" and/or "git commit -a")
> +	EOF
> +	git status >output &&

You can run

  git status --untracked-files=no

instead, to get rid of these "expect" and "output". as Junio mentionned on another topic, these make the test scripts a bit harder to maintain, for example you should rename "output" to "actual", and you don't want that to interfere with the content of "expect".

> +	} else if (state->rebase_in_progress) {
> +		status_printf_ln(s, color, _("You are currently rebasing%s"),
> +			advice_status_hints
> +			? _(": all conflicts fixed: run \"git rebase --continue\".") : ".");

Avoid having two ":" in the same sentense.

Show 6 quoted lines

> +		if (advice_status_hints) {
> +			status_printf_ln(s, color, _("  You can amend the commit with"));
> +			status_printf_ln(s, color, _("	git commit --amend"));
> +			status_printf_ln(s, color, _("  Once you are satisfied with your changes, run"));
> +			status_printf_ln(s, color, _("	git rebase --continue"));
> +		}

Can this be more consistant with other status hints? When a hint follows a statement on the next line, it's usually

# Say something # (use "git whatever" to change the current state)

e.g.

# Changes not staged for commit: # (use "git add <file>..." to update what will be committed)

So this may be turned into

# Rebase in progress # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes)

Show 10 quoted lines

> +	if (state->merge_in_progress)
> +		merge_in_progress_show(s, state, state_color);
> +	else if (state->am_in_progress)
> +		am_in_progress_show(s, state, state_color);
> +	else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
> +		rebase_in_progress_show(s, state, state_color);
> +	else if (state->cherry_pick_in_progress)
> +		cherry_pick_in_progress_show(s, state, state_color);
> +	if (state->bisect_in_progress)
> +		bisect_in_progress_show(s, state, state_color);

I still disagree with the "else if" logic here. If the user started two operations, I think we should show both messages.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/