Обсуждение: Improve pgindent's formatting named fields in struct literals and varidic functions

Hi,

A personal pet peeve of mine has been how pgindent formats struct 
literals with named fields, for example the below:

static RBTNode sentinel =
{
     .color = RBTBLACK,.left = RBTNIL,.right = RBTNIL,.parent = NULL
};

The attached patch fixes the formatting to be nicer:

static RBTNode sentinel =
{
     .color = RBTBLACK, .left = RBTNIL, .right = RBTNIL, .parent = NULL
};

It is currently the only example but I mainly think that is because the 
current formatting looks ugly so we avoid putting struct literals on a 
single line. Plus I have seen this formatting in PostgreSQL extensions.

While fixing this personal annoyance I noticed that my fix also would 
fix the formatting of varidic functions. For example:

-errdetail(const char *fmt,...)
+errdetail(const char *fmt, ...)

What do you think? I think both are clear improvements to the 
readability and that the churn is not big enough to be an issue.

Andreas

Here is a rebased version of the patch.

Andreas

On 3/2/26 11:41 PM, Andreas Karlsson wrote:
> Here is a rebased version of the patch.
Rebased it again.

Andreas

On Tue, 31 Mar 2026 at 08:30, Andreas Karlsson <andreas@proxel.se> wrote:
> Rebased it again.

I agree this looks better. So seems like a reasonable improvement.

On 3/31/26 9:16 AM, Jelte Fennema-Nio wrote:
> On Tue, 31 Mar 2026 at 08:30, Andreas Karlsson <andreas@proxel.se> wrote:
>> Rebased it again.
> 
> I agree this looks better. So seems like a reasonable improvement.

Thanks for taking a look!

But noticed now that the tests were broken so here is a new version 
where they pass and where I also added a new test case for struct 
literals with named fields.

Andreas

> On Mar 31, 2026, at 16:03, Andreas Karlsson <andreas@proxel.se> wrote:
>
> On 3/31/26 9:16 AM, Jelte Fennema-Nio wrote:
>> On Tue, 31 Mar 2026 at 08:30, Andreas Karlsson <andreas@proxel.se> wrote:
>>> Rebased it again.
>> I agree this looks better. So seems like a reasonable improvement.
>
> Thanks for taking a look!
>
> But noticed now that the tests were broken so here is a new version where they pass and where I also added a new test
casefor struct literals with named fields. 
>
> Andreas
>
<v4-0001-Make-pgindent-add-a-space-between-comma-and-perio.patch><v4-0002-Run-pgindent-add-a-space-between-comma-and-period.patch>

I applied the patch and played with it a little bit. I didn’t find any problem. The patch idea looks reasonable to me,
aftera comma, dot is no longer acting as structure member access, so inserting a white space feels good. 

I saw a couple of typos in the commit message:

varidic -> variadic
treaing -> treating

Overall, looks good to me.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On 2026-Mar-31, Andreas Karlsson wrote:

> On 3/31/26 9:16 AM, Jelte Fennema-Nio wrote:
> > On Tue, 31 Mar 2026 at 08:30, Andreas Karlsson <andreas@proxel.se> wrote:
> > > Rebased it again.
> > 
> > I agree this looks better. So seems like a reasonable improvement.
> 
> Thanks for taking a look!
> 
> But noticed now that the tests were broken so here is a new version where
> they pass and where I also added a new test case for struct literals with
> named fields.

Hmm, interesting, I've wondered why variadic functions had such a weird
indentation and this explains it.  I also +1 this, but at the same time
I think the best time to get it in is a few weeks after the feature
freeze.  That way we minimize the impact on any fixes during the
stabilization period.  I'm not sure at what point relative to the betas
though; is it better to do it before, so that we have a chance to fix
any possible problems before they hit users, or is it better to do after
beta1, so that patches that we discover need reverting for pg19 have
already had a chance to be so?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
> I think the best time to get it in is a few weeks after the feature
> freeze.  That way we minimize the impact on any fixes during the
> stabilization period.  I'm not sure at what point relative to the betas
> though; is it better to do it before, so that we have a chance to fix
> any possible problems before they hit users, or is it better to do after
> beta1, so that patches that we discover need reverting for pg19 have
> already had a chance to be so?

It seems unlikely to me that a pgindent change could reach to the
point of causing user-visible problems.  It could break code I guess,
but we'd almost surely see compile failures if so.  Also I rather
imagine we'd have eyeballed all the code changes before committing.

The other consideration is that we don't want to do it too close to
when the next CF opens.  It will likely break some pending patches,
and people will want time to rebase those.

Checking our commit history, it seems the last few intentional
changes in pgindent's behavior were applied here:

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_18_BR [b27644bad] 2025-06-15 13:04:24 -0400

    Sync typedefs.list with the buildfarm.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_16_BR [0245f8db3] 2023-05-19 17:24:48 -0400

    Pre-beta mechanical code beautification.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_16_BR [064750af4] 2023-04-08 11:48:45 -0400

    Improve indentation of multiline initialization expressions.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_13_BR [fa27dd40d] 2020-05-16 11:54:51 -0400

    Run pgindent with new pg_bsd_indent version 2.1.1.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_12_BR [8255c7a5e] 2019-05-22 13:04:48 -0400

    Phase 2 pgindent run for v12.

So we haven't been totally consistent about it, but mid-May (a couple
weeks before making the new branch) seems like the usual choice.

            regards, tom lane

PS: this isn't an endorsement of the proposed patch; I've not
looked at it.

On 3/31/26 8:13 PM, Tom Lane wrote:
> =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
>> I think the best time to get it in is a few weeks after the feature
>> freeze.  That way we minimize the impact on any fixes during the
>> stabilization period.  I'm not sure at what point relative to the betas
>> though; is it better to do it before, so that we have a chance to fix
>> any possible problems before they hit users, or is it better to do after
>> beta1, so that patches that we discover need reverting for pg19 have
>> already had a chance to be so?
> 
> [...]
> 
> So we haven't been totally consistent about it, but mid-May (a couple
> weeks before making the new branch) seems like the usual choice.
Makes sense to merge it while the tree is less noisy. Especially right 
now might be the most annoying time possible.

Andreas

On 3/31/26 8:57 PM, Andreas Karlsson wrote:
> On 3/31/26 8:13 PM, Tom Lane wrote:
>> =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
>>> I think the best time to get it in is a few weeks after the feature
>>> freeze.  That way we minimize the impact on any fixes during the
>>> stabilization period.  I'm not sure at what point relative to the betas
>>> though; is it better to do it before, so that we have a chance to fix
>>> any possible problems before they hit users, or is it better to do after
>>> beta1, so that patches that we discover need reverting for pg19 have
>>> already had a chance to be so?
>>
>> [...]
>>
>> So we haven't been totally consistent about it, but mid-May (a couple
>> weeks before making the new branch) seems like the usual choice.
> Makes sense to merge it while the tree is less noisy. Especially right 
> now might be the most annoying time possible.
Rebased patch on top of HEAD.

Andreas