Re: pgindent behavior we could do without - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: pgindent behavior we could do without
Date
Msg-id 20140130184233.GB20260@momjian.us
Whole thread Raw
In response to pgindent behavior we could do without  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: pgindent behavior we could do without
List pgsql-hackers
On Thu, Jul 18, 2013 at 12:27:21AM -0400, Tom Lane wrote:
> It's always annoyed me that pgindent insists on adjusting vertical
> whitespace around #else and related commands.  This has, for example,
> rendered src/include/storage/barrier.h nigh illegible: you get things
> like
> 
> /*
>  * lwsync orders loads with respect to each other, and similarly with stores.
>  * But a load can be performed before a subsequent store, so sync must be used
>  * for a full memory barrier.
>  */
> #define pg_memory_barrier()     __asm__ __volatile__ ("sync" : : : "memory")
> #define pg_read_barrier()       __asm__ __volatile__ ("lwsync" : : : "memory")
> #define pg_write_barrier()      __asm__ __volatile__ ("lwsync" : : : "memory")
> #elif defined(__alpha) || defined(__alpha__)    /* Alpha */
> 
> which makes it look like this block of code has something to do with
> Alpha.
> 
> By chance, I noticed today that this misbehavior comes from a discretely
> identifiable spot, to wit lines 289-290 in src/tools/pgindent/pgindent:
> 
>     # Remove blank line(s) before #else, #elif, and #endif
>     $source =~ s!\n\n+(\#else|\#elif|\#endif)!\n$1!g;
> 
> This seems pretty broken to me: why exactly is whitespace there such a
> bad idea?  Not only that, but the next action is concerned with undoing
> some of the damage this rule causes:
> 
>     # Add blank line before #endif if it is the last line in the file
>     $source =~ s!\n(#endif.*)\n\z!\n\n$1\n!;
> 
> I assert that we should simply remove both of these bits of code, as
> just about every committer on the project is smarter about when to use
> vertical whitespace than this program is.

OK, I have developed the attached patch that shows the code change of
removing the test for #else/#elif/#endif.  You will see that the new
output has odd blank lines for cases like:
#ifndef WIN32static int    copy_file(const char *fromfile, const char *tofile, bool force);
-->#elsestatic int    win32_pghardlink(const char *src, const char *dst);
-->#endif

I can't think of a way to detect tight blocks like the above from cases
where there are sizable blocks, like:
#ifndef WIN32
static int    copy_file(const char *fromfile, const char *tofile, bool force);.........#else
static int    win32_pghardlink(const char *src, const char *dst);.........
#endif

Ideas?

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +



pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: jsonb and nested hstore
Next
From: Tom Lane
Date:
Subject: Re: pgindent behavior we could do without