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

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: May 28, 2012, 20:23 UTC
Message-ID: <vpqobp89hrw.fsf@bauges.imag.fr>
In-Reply-To: <1338226598-16056-1-git-send-email-Lucien.Kong@ensimag.imag.fr>

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

> These messages can be shortened by setting
> the new advice.* config key called advice.statushelp to false.

It may make sense to use the existing advice.statusHint here.

> +	else {
> +		if (!stat(git_path("rebase-apply"), &st)) {

"else { if" is usually spelt "else if", without an additional level of indentation.

> +			status_printf_ln(s, c, _("You have unmerged paths: fix conflicts and then commit the result."));

I'd even say

You have unmerged paths; fix conflicts and run "git commit"

> +				status_printf_ln(s, c, _("One of the patches is empty or corrupted !"));

No space before !

> +				status_printf_ln(s, c, _("When you have resolved this problem run \"git am --resolved\"."));
> +				status_printf_ln(s, c, _("If you would prefer to skip this patch, instead run \"git am --skip\"."));
> +				status_printf_ln(s, c, _("To restore the original branch and stop patching run \"git am --abort\"."));

You have overly long lines. I already suggested splitting the function into smaller functions, and the presence of long lines is usually an indication that the splitting should be done.

Documentation/CodingGuidelines says:

,----
| For C programs:
| 
|  - We use tabs to indent, and interpret tabs as taking up to
|    8 spaces.
| 
|  - We try to keep to at most 80 characters per line.
`----

> +				status_printf_ln(s, c, _("You are currently rebasing%s"),
> +				advice_status_help
> +				? _(": fix conflicts and then run \"git rebase -- continue\".") : ".");

Weird indentation, it feels like 3 different statements.

> +	if(bisect_state) {

Space between if and (.

> +		if (advice_status_help)
> +		status_printf_ln(s, c, _("To get back to the original branch run \"git bisect reset\""));

Broken indentation.

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