Re: pgindent (was Re: [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Piotr Stefaniak <postgres@piotr-stefaniak.me>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-06-13T20:23:39Z
Lists: pgsql-hackers
I've now done a round of comparisons of results of our old indent
with your current version. There's still one serious bug in the latter:
it continues to misformat enum typedefs, for instance
*************** PG_FUNCTION_INFO_V1(pg_prewarm);
*** 33,40 ****
typedef enum
{
PREWARM_PREFETCH,
! PREWARM_READ,
! PREWARM_BUFFER
} PrewarmType;
static char blockbuffer[BLCKSZ];
--- 33,40 ----
typedef enum
{
PREWARM_PREFETCH,
! PREWARM_READ,
! PREWARM_BUFFER
} PrewarmType;
static char blockbuffer[BLCKSZ];
I spent some time trying to diagnose that, and what I found is that
while it's scanning the enum list, ps.in_decl is false, which causes
dump_line() to set ps.ind_stmt to true after the first line, which
causes later calls of compute_code_target() to add continuation_indent.
I was able to make the problem go away by making this change, which
reverts a change you'd apparently made since the old version of indent:
diff -ru /home/postgres/freebsd_indent/indent.c freebsd_indent/indent.c
--- /home/postgres/freebsd_indent/indent.c 2017-06-13 11:53:59.474524563 -0400
+++ freebsd_indent/indent.c 2017-06-13 15:51:23.590319885 -0400
@@ -944,7 +944,7 @@
}
ps.in_or_st = true; /* this might be a structure or initialization
* declaration */
- ps.in_decl = ps.decl_on_line = ps.last_token != type_def;
+ ps.in_decl = ps.decl_on_line = true;
if ( /* !ps.in_or_st && */ ps.dec_nest <= 0)
ps.just_saw_decl = 2;
prefix_blankline_requested = 0;
This also undoes a tendency of the new version to want to insert blank
lines that weren't there before inside struct declarations, eg
*** a/contrib/btree_gist/btree_macaddr8.c
--- b/contrib/btree_gist/btree_macaddr8.c
*************** typedef struct
*** 12,17 ****
--- 12,18 ----
{
macaddr8 lower;
macaddr8 upper;
+
/* make struct size = sizeof(gbtreekey16) */
} mac8KEY;
While I would not necessarily have quibbled with the addition of those
blank lines, I'm just as happy not to have them forced on us. I could
not find any places where reverting this change made the results worse,
so I'm unclear on why you made it.
regards, tom lane
Commits
-
Manually un-break a few URLs that pgindent used to insist on splitting.
- 780b3a4c43fd 10.0 landed
-
Remove entab and associated detritus.
- 81f056c7256f 10.0 landed
-
Phase 3 of pgindent updates.
- 382ceffdf7f6 10.0 landed
-
Phase 2 of pgindent updates.
- c7b8998ebbf3 10.0 landed
-
Initial pgindent run with pg_bsd_indent version 2.0.
- e3860ffa4dd0 10.0 landed
-
Adjust pgindent script to use pg_bsd_indent 2.0.
- 8ff6d4ec7840 10.0 landed
-
Final pgindent run with old pg_bsd_indent (version 1.3).
- 9ef2dbefc7fb 10.0 landed
-
Re-run pgindent.
- 651902deb155 10.0 landed
-
Preventive maintenance in advance of pgindent run.
- c079673dcb7f 10.0 cited