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

From: Junio C Hamano <gitster@pobox.com>
Date: May 11, 2017, 02:45 UTC
Message-ID: <xmqqmvakcdqw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CA+P7+xqu1oB2J2C9OoqaYAwdNOXT7f0kvHEHHZ4nXaefewBrNQ@mail.gmail.com>

Jacob Keller <jacob.keller@gmail.com> writes:

Show 7 quoted lines

> On Wed, Apr 12, 2017 at 11:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jacob Keller <jacob.keller@gmail.com> writes:
>>
>>> Personally, I would want this to become the default and not have a new
>>> option to trigger it. I think we could also extend the porcelain
>>> format to include this information as well, but I'm not too familiar
>>> with how the v2 format extends or not.

I started to do a simple s/inprogress/in-progress/ while waiting, but Ævar reminded me of this discussion---and I agree with you that this probably should be on. Moreover, I think this should not be optional (which makes s/inprogress/in-progress/ a moot point).

Michael went the cautious route to make it optional just like the "-b/--branch" option, but I think "-b/--branch" was a design mistake and not something we want to mimic. The long output format gives the same information without "--branch", and giving "--no-branch" does not even disable the information in the long format; "--branch" is only effective in the short format, even though giving it to long format does not diagnose any error.

We should have just enhanced the feature unconditionally, I would think. But fixing that past mistake is a separate issue.

Show 16 quoted lines

>> I think the general rule of thumb for --porcelain is that we can
>> freely introduce new record types without version bump, and expect
>> the reading scripts to ignore unrecognised records (we may need to
>> describe this a bit more strongly in our document, though), while
>> changes to the format of existing records must require a command
>> line option that cannot be turned on by default with configuration
>> (or a version bump, if you want to change the output format by
>> default).
>>
>> I am getting the impression that this "we are doing X" is a new and
>> discinct record type that existing readers can safely ignore?  If
>> that is the case, it may be better to add it without making it
>> optional.
>
> Correct. But either way we should be free to change and extend the
> non-porcelain format without worry I thought?

While I think we should update "short" and "porcelain" at the same time when it is clear that we can (e.g. we are adding a new record type and we make sure that the current readers of "porcelain" output would ignore the new record type), an update to "porcelain" can come later, as long as we don't forget. Otherwise people will script around "short", ignoring "porcelain", no?

Thanks.