Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)
Дата
Msg-id 29276.1497385419@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventivemaintenance in advance of pgindent run.)  (Piotr Stefaniak <postgres@piotr-stefaniak.me>)
Список 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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Bruce Momjian
Дата:
Сообщение: Re: [HACKERS] WIP: Data at rest encryption
Следующее
От: Kevin Grittner
Дата:
Сообщение: Re: [HACKERS] GSoC 2017 weekly progress reports (week 2)