Remove trailing comma from enums
I saw one (and then went looking and found some more) enum with a
trailing comma.
These are quite rare in the PG src, so I doubt they are intentional.
PSA a patch to remove the trailing commas for all that I found.
------
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v1-0001-Remove-trailing-commas-from-enums.patchapplication/octet-stream; name=v1-0001-Remove-trailing-commas-from-enums.patchDownload
From 56328be8807cb8f08081abd2b4939e225d51f6b8 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Thu, 6 Jan 2022 10:46:52 +1100
Subject: [PATCH v1] Remove trailing commas from enums
---
src/backend/commands/copyto.c | 2 +-
src/include/catalog/pg_publication.h | 2 +-
src/include/commands/vacuum.h | 2 +-
src/include/libpq/hba.h | 2 +-
src/include/miscadmin.h | 2 +-
src/include/nodes/nodes.h | 2 +-
src/include/parser/parse_node.h | 2 +-
src/include/pgstat.h | 2 +-
src/include/utils/jsonpath.h | 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index b6eacd5..daa4407 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -50,7 +50,7 @@
typedef enum CopyDest
{
COPY_FILE, /* to file (or a piped program) */
- COPY_FRONTEND, /* to frontend */
+ COPY_FRONTEND /* to frontend */
} CopyDest;
/*
diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h
index 902f2f2..ac9cd21 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -105,7 +105,7 @@ typedef enum PublicationPartOpt
{
PUBLICATION_PART_ROOT,
PUBLICATION_PART_LEAF,
- PUBLICATION_PART_ALL,
+ PUBLICATION_PART_ALL
} PublicationPartOpt;
extern List *GetPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt);
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 5a36049..ce3e972 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -202,7 +202,7 @@ typedef enum VacOptValue
VACOPTVALUE_UNSPECIFIED = 0,
VACOPTVALUE_AUTO,
VACOPTVALUE_DISABLED,
- VACOPTVALUE_ENABLED,
+ VACOPTVALUE_ENABLED
} VacOptValue;
/*
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 8d9f382..28fa1b6 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -61,7 +61,7 @@ typedef enum ConnType
ctHostSSL,
ctHostNoSSL,
ctHostGSS,
- ctHostNoGSS,
+ ctHostNoGSS
} ConnType;
typedef enum ClientCertMode
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 90a3016..64f2f61 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -335,7 +335,7 @@ typedef enum BackendType
B_WAL_WRITER,
B_ARCHIVER,
B_STATS_COLLECTOR,
- B_LOGGER,
+ B_LOGGER
} BackendType;
extern BackendType MyBackendType;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 7c657c1..c39e7ea 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -846,7 +846,7 @@ typedef enum LimitOption
{
LIMIT_OPTION_COUNT, /* FETCH FIRST... ONLY */
LIMIT_OPTION_WITH_TIES, /* FETCH FIRST... WITH TIES */
- LIMIT_OPTION_DEFAULT, /* No limit present */
+ LIMIT_OPTION_DEFAULT /* No limit present */
} LimitOption;
#endif /* NODES_H */
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index ee17908..1d4f3a6 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -79,7 +79,7 @@ typedef enum ParseExprKind
EXPR_KIND_CALL_ARGUMENT, /* procedure argument in CALL */
EXPR_KIND_COPY_WHERE, /* WHERE condition in COPY FROM */
EXPR_KIND_GENERATED_COLUMN, /* generation expression for a column */
- EXPR_KIND_CYCLE_MARK, /* cycle mark value */
+ EXPR_KIND_CYCLE_MARK /* cycle mark value */
} ParseExprKind;
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 5b51b58..4e7cb92 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -85,7 +85,7 @@ typedef enum StatMsgType
PGSTAT_MTYPE_CONNECT,
PGSTAT_MTYPE_DISCONNECT,
PGSTAT_MTYPE_SUBSCRIPTIONPURGE,
- PGSTAT_MTYPE_SUBWORKERERROR,
+ PGSTAT_MTYPE_SUBWORKERERROR
} StatMsgType;
/* ----------
diff --git a/src/include/utils/jsonpath.h b/src/include/utils/jsonpath.h
index 87d302b..85387a2 100644
--- a/src/include/utils/jsonpath.h
+++ b/src/include/utils/jsonpath.h
@@ -84,7 +84,7 @@ typedef enum JsonPathItemType
jpiSubscript, /* array subscript: 'expr' or 'expr TO expr' */
jpiLast, /* LAST array subscript */
jpiStartsWith, /* STARTS WITH predicate */
- jpiLikeRegex, /* LIKE_REGEX predicate */
+ jpiLikeRegex /* LIKE_REGEX predicate */
} JsonPathItemType;
/* XQuery regex mode flags for LIKE_REGEX predicate */
--
1.8.3.1
On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.com> wrote:
I saw one (and then went looking and found some more) enum with a
trailing comma.These are quite rare in the PG src, so I doubt they are intentional.
PSA a patch to remove the trailing commas for all that I found.
-1. I don't see the problem with C99 trailing commas. They avoid
noisy diff lines when patches add/remove items.
Thomas Munro <thomas.munro@gmail.com> writes:
On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.com> wrote:
These are quite rare in the PG src, so I doubt they are intentional.
PSA a patch to remove the trailing commas for all that I found.
-1. I don't see the problem with C99 trailing commas. They avoid
noisy diff lines when patches add/remove items.
I think they're rare because up till very recently we catered to
pre-C99 compilers that wouldn't accept them. There's not much
point in insisting on that now, though.
Personally I'm less excited than Thomas about trailing commas
being good for reducing diff noise, mainly because I think
that "add new entries at the end" is an anti-pattern, and
if you put new items where they logically belong then the
problem is much rarer. But I'm not going to argue against
committers who want to do it like that, either.
regards, tom lane
On Thu, Jan 6, 2022 at 12:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Thomas Munro <thomas.munro@gmail.com> writes:
On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.com> wrote:
These are quite rare in the PG src, so I doubt they are intentional.
PSA a patch to remove the trailing commas for all that I found.-1. I don't see the problem with C99 trailing commas. They avoid
noisy diff lines when patches add/remove items.I think they're rare because up till very recently we catered to
pre-C99 compilers that wouldn't accept them. There's not much
point in insisting on that now, though.Personally I'm less excited than Thomas about trailing commas
being good for reducing diff noise, mainly because I think
that "add new entries at the end" is an anti-pattern, and
if you put new items where they logically belong then the
problem is much rarer. But I'm not going to argue against
committers who want to do it like that, either.
FWIW, the background of this was that one of these examples overlapped
with a feature currently in development and it just caused a waste of
everyone's time by firstly "fixing" (removing) the extra comma and
then getting multiple code reviews saying the change was unrelated to
that feature and so having to remove that fix again. So I felt
removing all such commas at HEAD not only makes all the enums
consistent, but it may prevent similar time-wasting for others in the
future.
------
Kind Regards,
Peter Smith.
Fujitsu Australia
At Thu, 6 Jan 2022 12:52:50 +1100, Peter Smith <smithpb2250@gmail.com> wrote in
On Thu, Jan 6, 2022 at 12:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Thomas Munro <thomas.munro@gmail.com> writes:
On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.com> wrote:
These are quite rare in the PG src, so I doubt they are intentional.
PSA a patch to remove the trailing commas for all that I found.-1. I don't see the problem with C99 trailing commas. They avoid
noisy diff lines when patches add/remove items.I think they're rare because up till very recently we catered to
pre-C99 compilers that wouldn't accept them. There's not much
point in insisting on that now, though.Personally I'm less excited than Thomas about trailing commas
being good for reducing diff noise, mainly because I think
that "add new entries at the end" is an anti-pattern, and
if you put new items where they logically belong then the
problem is much rarer. But I'm not going to argue against
committers who want to do it like that, either.FWIW, the background of this was that one of these examples overlapped
with a feature currently in development and it just caused a waste of
everyone's time by firstly "fixing" (removing) the extra comma and
then getting multiple code reviews saying the change was unrelated to
that feature and so having to remove that fix again. So I felt
removing all such commas at HEAD not only makes all the enums
consistent, but it may prevent similar time-wasting for others in the
future.
I don't know where the above conversation took place, but it seems to
me that the first patch is not significant for reviewing and the last
patch seems to be just a waste of time even premising the first patch
survives.
I don't care whether the last item of an enum has a trailing comma or
not. (Or I like comma-less generally but I understand Thomas'
opinion.) I think one may take either way if need to modify the lines
involving such lines. But mildly object to make a change just to fix
them.
# Also, I don't want to see a comma-batttle breaks out at the end of
# an enum though...
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center