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
Attachments:
v1-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchtext/x-patch; charset=UTF-8; name=v1-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchDownload+2-1
v1-0002-Run-pgindent-add-a-space-between-comma-and-period.patchtext/x-patch; charset=UTF-8; name=v1-0002-Run-pgindent-add-a-space-between-comma-and-period.patchDownload+163-164
Here is a rebased version of the patch.
Andreas
Attachments:
v2-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchtext/x-patch; charset=UTF-8; name=v2-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchDownload+2-1
v2-0002-Run-pgindent-add-a-space-between-comma-and-period.patchtext/x-patch; charset=UTF-8; name=v2-0002-Run-pgindent-add-a-space-between-comma-and-period.patchDownload+163-164
On 3/2/26 11:41 PM, Andreas Karlsson wrote:
Here is a rebased version of the patch.
Rebased it again.
Andreas
Attachments:
v3-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchtext/x-patch; charset=UTF-8; name=v3-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchDownload+2-1
v3-0002-Run-pgindent-add-a-space-between-comma-and-period.patchtext/x-patch; charset=UTF-8; name=v3-0002-Run-pgindent-add-a-space-between-comma-and-period.patchDownload+167-168
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
Attachments:
v4-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchtext/x-patch; charset=UTF-8; name=v4-0001-Make-pgindent-add-a-space-between-comma-and-perio.patchDownload+6-1
v4-0002-Run-pgindent-add-a-space-between-comma-and-period.patchtext/x-patch; charset=UTF-8; name=v4-0002-Run-pgindent-add-a-space-between-comma-and-period.patchDownload+168-169
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 case for 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, after a 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