Re: [PATCH] git-status: show short sequencer state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Phil Hord <hordp@xxxxxxxxx> writes:

>>> +	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 ;-).

> 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.

>>> +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? [...]

> > [...]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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]