On 16/01/2020 03.59, Thomas Munro wrote:
On Wed, Jan 15, 2020 at 11:30 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvhe...@2ndquadrant.com> writes:
I just ran pgindent over some patch, and noticed that this hunk ended up
in my working tree:
- if (IsA(leftop, Var) && IsA(rightop, Const))
+ if (IsA(leftop, Var) &&IsA(rightop, Const))
Yeah, it's been doing that for decades. I think the triggering
factor is the typedef name (Var, here) preceding the &&.
It'd be nice to fix properly, but I've tended to take the path
of least resistance by breaking such lines to avoid the ugliness:
if (IsA(leftop, Var) &&
IsA(rightop, Const))
I am on vacation away from the Internet this week but somehow saw this
on my phone and couldn't stop myself from peeking at pg_bsd_ident
again. Yeah, "(Var)" (where Var is a known typename) causes it to
think that any following operator must be unary.
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:
diff --git a/lexi.c b/lexi.c
index d43723c..6de3227 100644
--- a/lexi.c
+++ b/lexi.c
@@ -655,6 +655,12 @@ stop_lit:
unary_delim = state->last_u_d;
break;
}
+
+ /* && and || are never unary */
+ if ((token[0] == '&' && *buf_ptr == '&') ||
+ (token[0] == '|' && *buf_ptr == '|'))
+ state->last_u_d = false;
+
while (*(e_token - 1) == *buf_ptr || *buf_ptr == '=') {
/*
* handle ||, &&, etc, and also things as in int *****i
The problem with that is that && sometimes *should* be formatted like
a unary operator: when it's part of the nonstandard GCC computed goto
syntax.
These comments are made in the context of pushing this change or
equivalent to FreeBSD repository.
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.
This change may seem hacky, but it would be far from the worst hack in
this program's history or even in its present form. It's actually very
much in indent's spirit, which is an attribute I neither support nor
condemn.
In any case, this change, or equivalent, should be committed to FreeBSD
repository together with a test case or two.
