Date: Wed, 6 May 2026 09:54:54 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Prepping for annual pgindent run
Message-ID: <aftWPpyZ_yGQWiN9@nathan>
References: <669384.1778006390@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <669384.1778006390@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/aftWPpyZ_yGQWiN9%40nathan>
Precedence: bulk

On Tue, May 05, 2026 at 02:39:50PM -0400, Tom Lane wrote:
> One thing to be done is to update src/tools/pgindent/typedefs.list
> from the canonical list constructed by the buildfarm.  We've been
> pretty good about maintaining that file manually, but not perfect,
> so this has a few small effects --- see attached for what that
> would look like today.

Seems fine.

> I also propose applying the pgindent patches discussed in [1] and [2],
> which would have rather larger effects.  [1] improves formatting of
> multiline comment blocks.  99% of the changes it would cause look like

In general, +1.

> @@ -933,7 +933,8 @@ _PG_init(void)
>  	MarkGUCPrefixReserved("isn");
>  }
>  
> -/* isn_out
> +/*
> + * isn_out
>   */
>  PG_FUNCTION_INFO_V1(isn_out);
>  Datum
> 
> While that's not that big a deal, it improves style uniformity since
> most of our multiline comments do not have any text on the first line.

Yeah, this seems like a good change, provided we've minimized all the
unintended side-effects.  I know there was some discussion about whether
this change should be made in the Perl script, but I really can't blame
anyone for not touching the pg_bsd_indent source code.

> [2] adds a space between comma and an immediately following period,
> which mostly affects variadic functions:
> 
> @@ -147,7 +147,7 @@ px_set_debug_handler(void (*handler) (const char *))
>  }
>  
>  void
> -px_debug(const char *fmt,...)
> +px_debug(const char *fmt, ...)
>  {
>  	va_list		ap;
> 
> 
> It also helps a few struct constructors:
> 
> @@ -62,7 +62,7 @@ struct RBTree
>  
>  static RBTNode sentinel =
>  {
> -	.color = RBTBLACK,.left = RBTNIL,.right = RBTNIL,.parent = NULL
> +	.color = RBTBLACK, .left = RBTNIL, .right = RBTNIL, .parent = NULL
>  };
> 
> I don't see any places where it makes anything worse, and the
> improvement in variadic functions is quite pleasing IMO.
> So I think we should do that too.

I read through that patch and it LGTM.

-- 
nathan