Thread: Add trailing commas to enum definitions

Add trailing commas to enum definitions

From
Peter Eisentraut
Date:
Since C99, there can be a trailing comma after the last value in an enum 
definition.  A lot of new code has been introducing this style on the 
fly.  I have noticed that some new patches are now taking an 
inconsistent approach to this.  Some add the last comma on the fly if 
they add a new last value, some are trying to preserve the existing 
style in each place, some are even dropping the last comma if there was 
one.  I figured we could nudge this all in a consistent direction if we 
just add the trailing commas everywhere once.  See attached patch; it 
wasn't actually that much.

I omitted a few places where there was a fixed "last" value that will 
always stay last.  I also skipped the header files of libpq and ecpg, in 
case people want to use those with older compilers.  There were also a 
small number of cases where the enum type wasn't used anywhere (but the 
enum values were), which ended up confusing pgindent a bit.
Attachment

Re: Add trailing commas to enum definitions

From
Junwang Zhao
Date:
On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> Since C99, there can be a trailing comma after the last value in an enum

C99 allows us to do this doesn't mean we must do this, this is not
inconsistent IMHO, and this will pollute the git log messages, people
may *git blame* the file and see the reason for the introduction of the
line.

There are a lot of 'typedef struct' as well as 'struct', which is not
inconsistent either just like the *enum* case.

> definition.  A lot of new code has been introducing this style on the
> fly.  I have noticed that some new patches are now taking an
> inconsistent approach to this.  Some add the last comma on the fly if
> they add a new last value, some are trying to preserve the existing
> style in each place, some are even dropping the last comma if there was
> one.  I figured we could nudge this all in a consistent direction if we
> just add the trailing commas everywhere once.  See attached patch; it
> wasn't actually that much.
>
> I omitted a few places where there was a fixed "last" value that will
> always stay last.  I also skipped the header files of libpq and ecpg, in
> case people want to use those with older compilers.  There were also a
> small number of cases where the enum type wasn't used anywhere (but the
> enum values were), which ended up confusing pgindent a bit.



--
Regards
Junwang Zhao



Re: Add trailing commas to enum definitions

From
Nathan Bossart
Date:
On Mon, Oct 23, 2023 at 05:55:32PM +0800, Junwang Zhao wrote:
> On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>> Since C99, there can be a trailing comma after the last value in an enum
> 
> C99 allows us to do this doesn't mean we must do this, this is not
> inconsistent IMHO, and this will pollute the git log messages, people
> may *git blame* the file and see the reason for the introduction of the
> line.

I suspect that your concerns about git-blame could be resolved by adding
this commit to .git-blame-ignore-revs.  From a long-term perspective, I
think standardizing on the trailing comma style will actually improve
git-blame because patches won't need to add a comma to the previous line
when adding a value.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com



Re: Add trailing commas to enum definitions

From
Tom Lane
Date:
Nathan Bossart <nathandbossart@gmail.com> writes:
> From a long-term perspective, I
> think standardizing on the trailing comma style will actually improve
> git-blame because patches won't need to add a comma to the previous line
> when adding a value.

Yeah, that's a good point.  I had been leaning towards "this is
unnecessary churn", but with that idea I'm now +1.

            regards, tom lane



Re: Add trailing commas to enum definitions

From
David Steele
Date:
On 10/23/23 17:04, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>>  From a long-term perspective, I
>> think standardizing on the trailing comma style will actually improve
>> git-blame because patches won't need to add a comma to the previous line
>> when adding a value.
> 
> Yeah, that's a good point.  I had been leaning towards "this is
> unnecessary churn", but with that idea I'm now +1.

+1 from me.

-David




Re: Add trailing commas to enum definitions

From
Junwang Zhao
Date:
On Tue, Oct 24, 2023 at 4:34 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Mon, Oct 23, 2023 at 05:55:32PM +0800, Junwang Zhao wrote:
> > On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> >> Since C99, there can be a trailing comma after the last value in an enum
> >
> > C99 allows us to do this doesn't mean we must do this, this is not
> > inconsistent IMHO, and this will pollute the git log messages, people
> > may *git blame* the file and see the reason for the introduction of the
> > line.
>
> I suspect that your concerns about git-blame could be resolved by adding
> this commit to .git-blame-ignore-revs.  From a long-term perspective, I
> think standardizing on the trailing comma style will actually improve
> git-blame because patches won't need to add a comma to the previous line
> when adding a value.

make sense, +1 from me now.

>
> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com



--
Regards
Junwang Zhao



Re: Add trailing commas to enum definitions

From
Andrew Dunstan
Date:
On 2023-10-23 Mo 17:04, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>>  From a long-term perspective, I
>> think standardizing on the trailing comma style will actually improve
>> git-blame because patches won't need to add a comma to the previous line
>> when adding a value.
> Yeah, that's a good point.  I had been leaning towards "this is
> unnecessary churn", but with that idea I'm now +1.
>
>             


+1. It's a fairly common practice in Perl code, too, and I often do it 
for exactly this reason.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com




Re: Add trailing commas to enum definitions

From
Peter Eisentraut
Date:
On 23.10.23 22:34, Nathan Bossart wrote:
> On Mon, Oct 23, 2023 at 05:55:32PM +0800, Junwang Zhao wrote:
>> On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>>> Since C99, there can be a trailing comma after the last value in an enum
>>
>> C99 allows us to do this doesn't mean we must do this, this is not
>> inconsistent IMHO, and this will pollute the git log messages, people
>> may *git blame* the file and see the reason for the introduction of the
>> line.
> 
> I suspect that your concerns about git-blame could be resolved by adding
> this commit to .git-blame-ignore-revs.  From a long-term perspective, I
> think standardizing on the trailing comma style will actually improve
> git-blame because patches won't need to add a comma to the previous line
> when adding a value.

Committed that way.