Re: pgindent && weirdness

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgindent && weirdness
Date: 2020-05-21 22:40:06
Message-ID: 17361.1590100806@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me> <VE1P192MB07504EB33625F9A23F41CCD0F2B70(at)VE1P192MB0750(dot)EURP192(dot)PROD(dot)OUTLOOK(dot)COM> writes:
> On 16/01/2020 03.59, Thomas Munro wrote:
>> One way to fix that in the cases Alvaro is referring to is to tell
>> override the setting so that && (and likewise ||) are never considered
>> to be unary, though I haven't tested this much and there are surely
>> other ways to achieve this:

> I think this is a better approach then the one committed to
> pg_bsd_indent. It's ubiquitous that the operators are binary, except -
> as you mentioned - in a nonstandard GCC syntax. The alternative given
> has more disadvantages, with potential impact on FreeBSD code
> formatting, which it should support as well as everything else -- to a
> reasonable extent. sys/kern/ is usually a decent litmus test, but I
> don't claim it should show anything interesting in this particular case.

I think that the fix we chose is better, at least for our purposes.
You can see its effects on our source tree at

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fa27dd40d5c5f56a1ee837a75c97549e992e32a4

and while certainly most of the diffs are around && or || operators,
there are a fair number that are not, such as

- dummy_lex.input = unconstify(char *, str) +1;
+ dummy_lex.input = unconstify(char *, str) + 1;

or more interestingly

- strncmp(text, "pg_", 3) !=0)
+ strncmp(text, "pg_", 3) != 0)

where the previous misformatting is because "text" is a known typedef
name. So it appears to me that this change reduces the misformatting
cost of typedef names that chance to match field or variable names,
and that's actually quite a big issue for us. We have, ATM, 3574 known
typedefs in typedefs.list, a fair number of which are not under our
control (since they come from system headers on various platforms).
So it's inevitable that there are going to be collisions.

In short, I'm inclined to stick with the hack we've got. I'm sad that
it will result in further divergence from FreeBSD indent; but it does
useful stuff for us, and I don't want to give it up.

(That said, I don't see any penalty to carrying both changes; so we'll
probably also absorb the &&/|| change at some convenient time.)

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2020-05-21 23:39:38 Re: [PATCH] Keeps tracking the uniqueness with UniqueKey
Previous Message Thomas Munro 2020-05-21 22:27:15 Re: Parallel Seq Scan vs kernel read ahead