On Mon, Jun 12, 2017 at 11:28:56AM -0700, Junio C Hamano wrote:
> Kaartic Sivaraam <kaarticsivaraam91196@xxxxxxxxx> writes:
>
> >> 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.
> @@ -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