Re: [PATCH] status: show in-progress info for short status

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Apr 13, 2017, 10:43 UTC
Message-ID: <CAM0VKjkbs1SD3YK6Um7t_Z72z8S4-xz5XpV3ywEmphMJRrmTzQ@mail.gmail.com>
In-Reply-To: <33eebca1-b78f-1e9c-539c-120e32ecfded@grubix.eu>

On Fri, Apr 7, 2017 at 4:05 PM, Michael J Gruber <git@grubix.eu> wrote:

Show 18 quoted lines

> SZEDER Gábor venit, vidit, dixit 06.04.2017 16:33:
>>> @@ -1779,6 +1780,31 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
>>>      }
>>>
>>>      color_fprintf(s->fp, header_color, "]");
>>> +
>>> + inprogress:
>>> +    if (!s->show_inprogress)
>>> +            goto conclude;
>>> +    memset(&state, 0, sizeof(state));
>>> +    wt_status_get_state(&state,
>>> +                        s->branch && !strcmp(s->branch, "HEAD"));
>>> +    if (state.merge_in_progress)
>>> +            color_fprintf(s->fp, header_color, "; %s", LABEL(N_("MERGING")));
>>> +    else if (state.am_in_progress)
>>> +            color_fprintf(s->fp, header_color, "; %s", LABEL(N_("AM")));
>>> +    else if (state.rebase_in_progress)
>>> +            color_fprintf(s->fp, header_color, "; %s", LABEL(N_("REBASE-m")));

In the prompt "REBASE-m" is only shown during 'rebase --merge', while "REBASE" is shown during a plain 'rebase'. Here "REBASE-m" will be shown during both.

Show 11 quoted lines

>>> +    else if (state.rebase_interactive_in_progress)
>>> +            color_fprintf(s->fp, header_color, "; %s", LABEL(N_("REBASE-i")));
>>> +    else if (state.cherry_pick_in_progress)
>>> +            color_fprintf(s->fp, header_color, "; %s", LABEL(N_("CHERRY-PICKING")));
>>> +    else if (state.revert_in_progress)
>>> +            color_fprintf(s->fp, header_color, "; %s", LABEL(N_("REVERTING")));
>>> +    if (state.bisect_in_progress)
>>
>> else if?
>
> No. You can do all of the above during a bisect.

Right indeed. I think the prompt should do the same. Onto the TODO list it goes, then.

Show 18 quoted lines

>>> +            color_fprintf(s->fp, header_color, "; %s", LABEL(N_("BISECTING")));
>>> +    free(state.branch);
>>> +    free(state.onto);
>>> +    free(state.detached_from);
>>> +
>>>   conclude:
>>>      fputc(s->null_termination ? '\0' : '\n', s->fp);
>>>  }
>>
>> This reminded me of a patch that I have been using for almost two
>> years now...
>>
>> git-prompt.sh's similar long conditional chain to show the ongoing
>> operation has an else-branch at the end showing "AM/REBASE".  Your
>> patch doesn't add an equivalent branch.  Is this intentional or an
>> oversight?
>
> I go over all states that wt_status_get_state can give.

Yeah, but are those states exclusive? Or is it possible that both 'am_in_progress' and 'rebase_in_progress' are set? I suppose it's not possible. It would certainly be more obvious if it were a single enum field instead of a bunch of ints.

Show 10 quoted lines

>> I suppose it's intentional, because that "AM/REBASE" branch in the
>> prompt seems to be unreachable (see below), but I never took the
>> effort to actually check that (hence the "seems" and that's why I
>> never submitted it).
>
> Note that wt_status_get_state and the prompt script do things quite
> differently.
>
> I suppose that the prompt could make use of the in-progress info being
> exposed by "git status" rather doing its on thing.

The prompt currently gets all this in-progress info using only Bash builtins, which is much faster than running a git process in a subshell. This is especially important on Windows, where the overhead of running a git process is large enough to make the runtime of __git_ps1() noticeable when displaying the prompt. That's the main reason for much of the shell complexity and ugliness in git-prompt.sh.

Besides, current output formats make 'git status' unsuitable for the prompt.