Re: Spacing of options in getopt_long processing
Andrew Dunstan <andrew@dunslane.net>
From: Andrew Dunstan <andrew@dunslane.net>
To: Peter Eisentraut <peter@eisentraut.org>,
PostgreSQL-development <pgsql-hackers@postgresql.org>,
Vaibhav Dalvi <vaibhav.dalvi@enterprisedb.com>
Date: 2025-11-06T14:21:35Z
Lists: pgsql-hackers
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