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

Guillaume Pagès  [off-list ref] writes:

git status gives more information during rebase -i, about the list of
command that are done during the rebase. It displays the two last
commands executed and the two next lines to be executed. It also gives
hints to find the whole files in .git directory.
---

Without 1/4 and 2/4 in the same thread, it is hard to guess what you
wanted to do with these two patches.  Remember, reviewers review not
just your patches but those from many others.

I would in the meantime assume these are replacement patches for the
ones in

  http://thread.gmane.org/gmane.comp.version-control.git/270743/focus=270744

"lines_to_show = 2" or "nr_lines_to_show = 2" would be easier to read.

+
+		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);
+
+		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);
+
+		if (have_done.nr == 0)
+			status_printf_ln(s, color, _("No commands done."));

Do the users even need to be told that, I wonder?

+		else{

Style:	"else {"

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

Hmm, perhaps fold lines like this (and you do not need "begin")?

			for (i = (lines_to_show_nr < have_done.nr)
                             ? have_done.nr - lines_to_show_nr : 0;
                             i < have_done.nr;
                             i++)
				status_printf_ln(...);

+			if (have_done.nr > lines_to_show_nr && s->hints)
+			   status_printf_ln(s, color,
+				_("  (see more in file %s)"), git_path("rebase-merge/done"));

That's a nice touch ;-)

+		}
+		if (yet_to_do.nr == 0)
+			status_printf_ln(s, color,
+					 _("No commands remaining."));

This I can see why we may want to say it.

+		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);
+			}
+			if (s->hints)
+			   status_printf_ln(s, color,
+				_("  (use \"git rebase --edit-todo\" to view and edit)"));
+		}

Make sure you do not leak memory used by two string lists here...

+	}
+}
+
 static void show_rebase_in_progress(struct wt_status *s,
 				struct wt_status_state *state,
 				const char *color)
 {
 	struct stat st;
 
+	show_rebase_information(s, state, color);
 	if (has_unmerged(s) || state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
 		if (state->branch)
 			status_printf_ln(s, color,