Re: [PATCH 04/26] diff.c: introduce emit_diff_symbol

From: Stefan Beller <sbeller@google.com>
Date: Jun 21, 2017, 21:23 UTC
Message-ID: <CAGZ79kZnDdtB6O5jnt7ouG-qoOS90kZALqZB_-gOT0OQcJzvRg@mail.gmail.com>
In-Reply-To: <xmqq7f05niwt.fsf@gitster.mtv.corp.google.com>

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

Show 16 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>>>>
>>>> +enum diff_symbol {
>>>> +     DIFF_SYMBOL_SEPARATOR,
>>>
>>> Drop the last comma from enum?
>>
>> I looked through out code base and for enums this is
>> actually strictly enforced, so I guess I have to play
>> by the rules here as I do not want to be the first
>> to deviate from an upheld standard.
>
> You sound like you are trying to find an excuse "not to play by the
> rules".  Don't.
>

When the rules are not yielding best results, adapting the rules should be considered. And having no comma at the last element produces review noise when working in a line oriented format, such as diffs. (Add another element at the end, and then you have to have the diff that just changes the comma)

  C99 allows a trailing comma to follow the last enumeration
  constant initializer within an enumeration type declaration,
  similar to structure member initialization lists.
  C99: §6.7.2.2

  For struct member lists we already do that, e.g.
  builtin/help.c: struct common_guides[]
  builtin/repack.c: struct exts[]
  ...

Yet, I will construct this series in a resend to not have this issue.

Thanks, Stefan