Fwd: lwlocknames.h beautification attempt

From: Gurjeet Singh <gurjeet(at)singh(dot)im>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Postgres Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Fwd: lwlocknames.h beautification attempt
Date: 2025-03-17 06:44:07
Message-ID: CABwTF4V8edy3mn+wf8eGsibKBzWCfLwYaDpAtzVZ2Uy+xJnwNg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(resending the email because it was held for moderation; replaced image
attachment with a link, which might be the reason for being put in the
moderation queue)

On Sun, Mar 16, 2025 at 7:53 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Fri, Mar 14, 2025 at 3:38 PM Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > I forgot to send a note here that I pushed this patch. Thank you.
>
> I'm confused. Tom and I both said we didn't like this change,

To me, Tom's feedback felt as being between ambivalent to the change and perhaps
agree with the change, as long as pgindent did not throw a fit, which
it did not.
It definitely did not feel like a -1.

On Mon, Mar 3, 2025 at 8:40 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Sun, Mar 2, 2025 at 1:10 AM Gurjeet Singh <gurjeet(at)singh(dot)im> wrote:
> > I propose the following change to the generation script, generate-lwlocknames.pl
> >
> > - print $h "#define ${lockname}Lock (&MainLWLockArray[$lockidx].lock)\n";
> > + printf $h "#define %-30s %s\n", "${lockname}Lock", "(&MainLWLockArray[$lockidx].lock)";
>
> -1. That seems worse to me.

I read your objection to be about the perl code change, whereas the real change
the patch was seeking is in the generated output (lwlocknames.h). I'm guessing
some others may also have taken your feedback to mean the same as I did.

Alvaro committed the following patch, which is better code than mine. If your
objection was about the perl code, perhaps this addresses your concern.

- print $h "#define ${lockname}Lock (&MainLWLockArray[$lockidx].lock)\n";
+ printf $h "#define %-32s (&MainLWLockArray[$lockidx].lock)\n",
+ $lockname . "Lock";

I have ~attached~ linked a comparison of before and after screenshots of the
generated output. It's hard to argue that the generated code in the left/before
image is better than the right/after image.

https://photos.app.goo.gl/hNL3FaUMuEwnaYTt9

Best regards,
Gurjeet
http://Gurje.et

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2025-03-17 06:46:11 Re: Speed up ICU case conversion by using ucasemap_utf8To*()
Previous Message Álvaro Herrera 2025-03-17 06:43:41 Re: lwlocknames.h beautification attempt