Thread (2 messages) flat view 2 messages, 2 authors, 2017-06-12

Re: [PATCH] wt-status.c: Modified status message shown for a parent-less branch

From: Jeff King <hidden>
Date: 2017-06-12 21:20:38

Possibly related (same subject, not in this thread)

On Mon, Jun 12, 2017 at 11:28:56AM -0700, Junio C Hamano wrote:
Kaartic Sivaraam [off-list ref] writes:
quoted
quoted
Adding a bit to "struct wt_status" is a good first step to allow all
three (i.e. in addition to "Initial commit" and "Changes to be
committed", "Changes not staged for commit" is the other one that
shares this potential confusion factor) to be phrased in a way that
is more appropriate in an answer to the question "what is the status
of my working area?", I would think.

Thanks.
It seems that the current change has to be discarded altogether and
further the change required doesn't look trivial. This seems to warrant
some bit of research of the code base. As a first step I would like to
know which part of the code base creates the commit template. I guess
much can't be done without knowing how it's created.
Perhaps something along this line (warning: not even compile
tested)?
So I think the addition of the bit here is obviously correct, and I'm
not opposed to the idea of giving wt-status more information so that it
can make better messages. But I'm not sure it's actually helping for
some of these cases. E.g.:
-	status_printf_ln(s, c, _("Changes not staged for commit:"));
+	if (s->commit_template)
+		status_printf_ln(s, c, _("Changes not staged for commit:"));
+	else
+		status_printf_ln(s, c, _("Changes not yet in the index:"));
I think "staged for commit" still makes perfect sense even when we are
just asking "what's the current status" and not "what would it look like
if I were to commit".

And avoiding the word "index" is worth-while here, I think. I am not in
general of the "let's hide the index" camp" but it is a technical term.
If we can say the same thing in a way that is understood both by people
who know what the index is and people who do not, that seems like a win.
-	status_printf_ln(s, c, _("Changes to be committed:"));
+	if (s->commit_template)
+		status_printf_ln(s, c, _("Changes to be committed:"));
+	else
+		status_printf_ln(s, c, _("Changes already in the index:"));
This one is less obvious, because "to be committed" more strongly
implies making an actual commit. At the same time, I don't think it's
unclear what it means in the context of status. It could be "Changes
staged for commit" to match the other "not staged" message, though I
think I prefer the existing wording.
quoted hunk ↗ jump to hunk
@@ -1578,7 +1584,10 @@ static void wt_longstatus_print(struct wt_status *s)
 
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
-		status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial commit"));
+		status_printf_ln(s, color(WT_STATUS_HEADER, s),
+				 s->commit_template
+				 ? _("Initial commit")
+				 : _("No commit yet on the branch"));
This one I think is an improvement. :)

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help