Re: [PATCH 3/4] status: give more information during rebase -i

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 8, 2015, 16:17 UTC
Message-ID: <vpqeglm41jm.fsf@anie.imag.fr>
In-Reply-To: <1433775308-5451-1-git-send-email-guillaume.pages@ensimag.grenoble-inp.fr>

Please, include a version number in the subject. git send-email -v2 does this for you.

Guillaume Pagès <guillaume.pages@ensimag.grenoble-inp.fr> writes:

Show 17 quoted lines

> +static void show_rebase_information(struct wt_status *s,
> +				    struct wt_status_state *state,
> +				    const char *color)
> +{
> +	if (state->rebase_interactive_in_progress) {
> +		int i, begin;
> +		int lines_to_show_nr = 2;
> +
> +		struct strbuf buf = STRBUF_INIT;
> +		struct string_list have_done = STRING_LIST_INIT_DUP;
> +		struct string_list yet_to_do = STRING_LIST_INIT_DUP;
> +
> +		strbuf_read_file(&buf, git_path("rebase-merge/done"), 0);
> +		stripspace(&buf, 1);
> +		have_done.nr = string_list_split(&have_done, buf.buf, '\n', -1);
> +		string_list_remove_empty_items(&have_done, 1);
> +		strbuf_release(&buf);

I'd release buf one line earlier (meaning "I'm done with buf, now let's talk about something else"), but it's OK like this too

Show 5 quoted lines

> +		strbuf_read_file(&buf, git_path("rebase-merge/git-rebase-todo"), 0);
> +		stripspace(&buf, 1);
> +		string_list_split(&yet_to_do, buf.buf, '\n', -1);
> +		string_list_remove_empty_items(&yet_to_do, 1);
> +		strbuf_release(&buf);

Nice. Much simpler than the previous iteration.

> +		if (have_done.nr == 0)
> +			status_printf_ln(s, color, _("No commands done."));
> +		else{

Missing space before {

Show 6 quoted lines

> +			status_printf_ln(s, color,
> +				Q_("Last command done (%d command done):",
> +					"Last commands done (%d commands done):",
> +					have_done.nr),
> +				have_done.nr);
> +			begin = (have_done.nr > lines_to_show_nr) ? have_done.nr-lines_to_show_nr : 0;

Missing space around '-'

> +			for (i = begin; i < have_done.nr; i++) {
> +				status_printf_ln(s, color, "   %s", have_done.items[i].string);
> +			}

Useless braces.

> +			if (have_done.nr > lines_to_show_nr && s->hints)
> +			   status_printf_ln(s, color,

Indentation with space (open in Emacs, M-x whitespace-mode RET to see this kind of things).

Show 15 quoted lines

> +				_("  (see more in file %s)"), git_path("rebase-merge/done"));
> +		}
> +		if (yet_to_do.nr == 0)
> +			status_printf_ln(s, color,
> +					 _("No commands remaining."));
> +		else{
> +
> +			status_printf_ln(s, color,
> +				Q_("Next command to do (%d remaining command):",
> +					"Next commands to do (%d remaining commands):",
> +					yet_to_do.nr),
> +				yet_to_do.nr);
> +			for (i = 0; i < lines_to_show_nr && i < yet_to_do.nr; i++) {
> +				status_printf_ln(s, color, "   %s", yet_to_do.items[i].string);
> +			}

Useless braces.

> +			if (s->hints)
> +			   status_printf_ln(s, color,

Indentation with space again.

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