On 2025-11-05 We 11:41 AM, Peter Eisentraut wrote:
On 04.11.25 20:32, Andrew Dunstan wrote:
Over at [1] Vaibhav complained that the patch was deleting a line
following one of the case branches for handling command line options
in pg_restore.c, and said this was not pertinent to the patch. That's
reasonable, but it made me look into $subject a bit. pg_restore.c has
a mixture, with some options being followed by blank lines and some
not. pg_dumpall.c and pg_dump.c have a blank line after each option,
while psql's startup.c has none. It would be nice to clean this up
and have a consistent style. But what style? Personally I think
having a blank line after each option looks cleaner, and we're not
nearly so concerned with preserving vertical space as we might once
have been. I haven't surveyed other utilities in our suite. Is this
worth even pursuing? Do we care about making each file consistent, or
making all the code consistent?
I think it depends. For example, looking through getopt_long() in
initdb.c or pg_receivewal.c, each option processing is very simple.
Would adding blank lines there add anything in terms of clarity? I
doubt it. But then there is pg_resetwal.c, where each option
processing is rather complex, and so the extra blank lines seem almost
necessary.
Along those lines, I would suggest that pg_waldump.c adds some blank
lines, but perhaps pg_rewind.c could remove them.
Only what pg_restore.c is doing is clearly wrong. ;-)
I am mindful of the vertical space. Horizontal space is rather
cheaper and the stuff toward the right is usually less important, but
that doesn't apply vertically.
OK, I will clean up pg_restore.c and leave it at that.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
