Feedback-ID: id4a34324:Fastmail
Date: Thu, 19 Jan 2023 17:59:49 -0800
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: pgindent vs variable declaration across multiple lines
Message-ID: <20230120015949.vdkhzzgrd7jbwpbw@awork3.anarazel.de>
References: <20230120013137.7ky7nl4e4zjorrfa@awork3.anarazel.de>
 <337200.1674179024@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <337200.1674179024@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/20230120015949.vdkhzzgrd7jbwpbw%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2023-01-19 20:43:44 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > There's a few places in the code that try to format a variable definition like this
> 
> >     ReorderBufferChange *next_change =
> >         dlist_container(ReorderBufferChange, node, next);
> 
> > but pgindent turns that into
> 
> >     ReorderBufferChange *next_change =
> >     dlist_container(ReorderBufferChange, node, next);
> 
> Yeah, that's bugged me too.  I suspect that the triggering factor is
> use of a typedef name within the assigned expression, but I've not
> tried to run it to ground.

It's not that - it happens even with just
    int frak =
        1;

since it doesn't happen for plain assignments, I think it's somehow related to
code dealing with variable declarations.


> > I assume we'd again have to dive into pg_bsd_indent's code to fix it :(
> 
> Yeah :-(.  That's enough of a rat's nest that I've not really wanted to.
> But I'd support applying such a fix if someone can figure it out.

It's pretty awful code :(


> > And even if we were to figure out how, would it be worth the
> > reindent-all-branches pain? I'd say yes, but...
> 
> What reindent-all-branches pain?  We haven't done an all-branches
> reindent in the past, even for pgindent fixes that touched far more
> code than this would (assuming that the proposed fix doesn't have
> other side-effects).

Oh. I thought we had re-indented the other branches when we modified
pg_bsd_intent substantially in the past, to reduce backpatching pain. But I
guess we just discussed that option, but didn't end up pursuing it.

Greetings,

Andres Freund