Re: Spacing of options in getopt_long processing

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Vaibhav Dalvi <vaibhav(dot)dalvi(at)enterprisedb(dot)com>
Subject: Re: Spacing of options in getopt_long processing
Date: 2025-11-06 14:21:35
Message-ID: 83ad0f6c-0f5d-4487-a7c6-5c96bfd7496b@dunslane.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2025-11-06 14:27:04 Re: Consistently use the XLogRecPtrIsInvalid() macro
Previous Message Bryan Green 2025-11-06 14:13:31 Re: [PATCH] Fix orphaned backend processes on Windows using Job Objects