Re: [showing-off RFC/PATCH 26/26] diff.c: have a "machine parseable" move coloring

From: Stefan Beller <sbeller@google.com>
Date: Jun 21, 2017, 22:40 UTC
Message-ID: <CAGZ79kYDBZG4AOueJjvQ61yyC-DQk-8cb5-y18fg2k4JUsQPKw@mail.gmail.com>
In-Reply-To: <xmqqh8z9m2ed.fsf@gitster.mtv.corp.google.com>

On Wed, Jun 21, 2017 at 2:55 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 26 quoted lines

> Brandon Williams <bmwill@google.com> writes:
>
>> On 06/19, Stefan Beller wrote:
>>> Ævar asked for it, this is how you would do it.
>>> (plus documentation, tests, CLI knobs, options)
>>>
>>> Signed-off-by: Stefan Beller <sbeller@google.com>
>>> ---
>>>  diff.c | 15 +++++++++++----
>>>  diff.h |  2 ++
>>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/diff.c b/diff.c
>>> index 7756f7610c..61caa057ff 100644
>>> --- a/diff.c
>>> +++ b/diff.c
>>> @@ -997,6 +997,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
>>>      static const char *nneof = " No newline at end of file\n";
>>>      const char *context, *reset, *set, *meta, *fraginfo;
>>>      struct strbuf sb = STRBUF_INIT;
>>> +    int sign;
>>
>> should this be a char instead of an int?
>
> Perhaps.  Once we start adding things other than + and -, I think
> they should no longer be called "sign", though.

Well 'sign' is currently meant as a mathematical operation, but it could also be meant as a street sign?

The empty sign ' ' tells you it is context, and not subject to bikeshedding whilst reviewing a patch. :)

Show 5 quoted lines

>
> If Stefan chose '*' as a replacement for '+' because both makes
> things larger (yes, I am a positive person and my numbers are all
> positive), probably the replacement for '-' should be '/' (or '%'),
> not '_'.  But that is just bikeshedding.

I was trying to choose characters that sort of resembled the shape, but slightly differently. But a machine would not care any ways.

There was also temptation to use bogus signs like 'M' and 'm' for moved (capital M for + and lower m for -, obviously).

But this is a proof of concept on top of the series showing that this way with bits set in the flags field we do not need to have colors. The older series was operating on the colors and by that figuring out if it was say at a boundary, but here we would just use the flagbits to come up with a reasonable first character. (for example: Would the machine parse-able format also care about dimming? I defer such a discussion in the future.)