Thread: Add trailing commas to enum definitions
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
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
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
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
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
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
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
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.