Re: [PATCH] git-status: show short sequencer state
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Oct 24, 2012, 08:15 UTC
- Message-ID
- <vpqsj94mgjr.fsf@grenoble-inp.fr>
- In-Reply-To
- <5086DBDB.9070606@cisco.com>
Phil Hord <hordp@cisco.com> writes:
Show 11 quoted lines
>>> + if (state->substate==WT_SUBSTATE_NOMINAL)
>>> status_printf_ln(s, color,
>>> _("The current patch is empty."));
>> This looks weird. First, spaces around == (here and below). Then, the
>> logic is unintuitive. The "if" suggests everything is allright, and the
>> message below is very specific. This at least deserves a comment.
>
> Yes, I agree. It was less clear but more reasonable before I tried to
> clear it up some. It's driven by the short-token printer. The state is
> "you're in a 'git am' but I do not see any conflicted files. Therefore,
> your patch must be empty."This was my guess, but I wouldn't have needed to guess if there was a comment in the code ;-).
Show 6 quoted lines
> I'll try to make this more explicit. Currently the short-status > version will say either "am" or "am \n conflicted" when a 'git am' is in > progress. The logical path to follow if I re-add 'git-am-empty' state > tracker is for this to now show either "am \n am-is-empty" or "am \n > conflicted". But I think I should suppress the "am-is-empty" report in > that case. What do you think
I don't think you should remove it from the output (no strong opinion). My point was just that the code looked weird.
Show 7 quoted lines
>>> +static void wt_print_token(struct wt_status *s, const char *color, const char *token)
>>> +{
>>> + color_fprintf(s->fp, color, "%s", token);
>>> + fputc(s->null_termination ? '\0' : '\n', s->fp);
>>> +}
>> The output format seems to be meant only for machine-consumption. Is
>> there any case when we'd want color? [...]Show 6 quoted lines
> > [...]I thought I might be going back there, or that I might combine this > > with full 'git status' again somehow, and colors seemed appropriate > > still. > > [...] > > So I can remove this color decorator until someone finds a need for > > it.
I'm fine with both options, with a slight preference for removing them.
> My own use-case involves $PS1.
That makes sense (indeed, the implementation of status hints was slightly inspired from what the bash prompt in contrib/completion/git-prompt.sh does). The next step could be to use your porcelain there instead of checking manually file existance.
You may want to add a short note about this motivation in the commit message.
-- Matthieu Moy http://www-verimag.imag.fr/~moy/