Re: [PATCHv2 1/2] wt-status: better advices for git status

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: May 27, 2012, 13:10 UTC
Message-ID: <vpq8vgdlqg8.fsf@bauges.imag.fr>
In-Reply-To: <1338035905-24166-1-git-send-email-Lucien.Kong@ensimag.imag.fr>

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

> +	const char *git_dir = getenv(GIT_DIR_ENVIRONMENT);

get_git_dir() would do the same more cleanly I think. And you don't actually need it because git_path("...") does what you're doing with mkpath already.

> +		it = &(s->change.items[i]);
> +		d = it->util;

Isn't it a complex way to say

                d = s->change.items[i].util

> +				status_printf_ln(s, c, _("You are currently editing in a rebase progress."));

I find the wording strange.

"You are currently editing a commit during a rebase"

> +	wt_status_print_in_progress(s);
> +

I think this deserves several functions (e.g. one for rebase in progress, one for bisect, ...). The one you have is already rather long, and may get longer when you start using the advice mechanism.

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