Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: pgindent versus struct members and typedefs
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <aS9hkwnkWf3dZIA_@nathan>
Date: Wed, 3 Dec 2025 06:46:33 +0800
Cc: Rahila Syed <rahilasyed90@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <347EA055-D626-414C-B10D-7E70041B9B55@gmail.com>
References: <aS9hkwnkWf3dZIA_@nathan>
To: Nathan Bossart <nathandbossart@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/347EA055-D626-414C-B10D-7E70041B9B55%40gmail.com>
Precedence: bulk


> On Dec 3, 2025, at 06:00, Nathan Bossart <nathandbossart@gmail.com> =
wrote:
>=20
> On Mon, Dec 01, 2025 at 05:04:23PM -0600, Nathan Bossart wrote:
>> On Tue, Dec 02, 2025 at 05:35:34AM +0800, Chao Li wrote:
>>> ```
>>> + else if (entry->type =3D=3D DSMR_ENTRY_TYPE_DSH &&
>>> + entry->dsh.dsa_handle !=3DDSA_HANDLE_INVALID)
>>> ```
>>>=20
>>> Missing a white space after !=3D.
>>=20
>> I agree, but for some reason, pgindent insists on removing that =
space.  I'm
>> leaving that for another thread.
>=20
> So, this seems to have something to do with the struct member having =
the
> same name as a typedef.  If I rename the member, pgindent adds the =
space as
> expected.  Interestingly, changing the !=3D to a =3D=3D also fixes the =
spacing.
> There are a couple of other examples in the code:
>=20
> src/backend/storage/ipc/dsm_registry.c: entry->dsh.dsa_handle =
!=3DDSA_HANDLE_INVALID)
> src/backend/replication/logical/logicalfuncs.c: =
ctx->options.output_type !=3DOUTPUT_PLUGIN_TEXTUAL_OUTPUT)
> src/bin/pg_basebackup/pg_basebackup.c: if (state.manifest_file !=3DNULL)=

> src/bin/pg_basebackup/pg_basebackup.c: state->manifest_file !=3DNULL)
> src/bin/pg_basebackup/pg_basebackup.c: else if (state->manifest_file =
!=3DNULL)
>=20
> I used the following command to find these:
>=20
> grep -E "!=3D[A-Za-z]" ./* -rI
>=20
> AFAICT this is a special case of the note added to pgindent's README =
by
> commit c4133ec:
>=20
> pgindent will mangle both declaration and definition of a C function =
whose
> name matches a typedef.  Currently the best workaround is to choose
> non-conflicting names.
>=20
> I tried to fix pgindent for a few, but the code is basically =
impenetrable.
> I didn't find any fixes upstream [0], either.  As noted above, we =
could
> also fix it by avoiding the naming conflicts.  However, I can't =
imagine
> that's worth the churn, and I've already spent way too much time on =
this,
> so IMHO the best thing to do here is nothing.
>=20

I think that=E2=80=99s fine.

Actually I see the other problem with pgindent, where if a =E2=80=9Celse=E2=
=80=9D clause contains a multiple-line comment and a single statement =
without braces, for example:

```
else
     /*
      * comment
      */
     printf(=E2=80=A6);
```

Then pgindent will blindly add an empty line after =E2=80=9Celse=E2=80=9D,=
 so we get:
```
else

     /*
      * comment
      */
     printf(=E2=80=A6);

```

I tried to fix but failed. For that problem, a solution is to add braces =
to the =E2=80=9Celse=E2=80=9D clause.

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