Re: Add --check option to pgindent

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, Tristan Partin <tristan(at)neon(dot)tech>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Euler Taveira <euler(at)eulerto(dot)com>, Michael Banck <mbanck(at)gmx(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add --check option to pgindent
Date: 2023-12-20 16:05:14
Message-ID: b6ea6f6f-ac14-07e3-5276-783f2bae9adb@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2023-12-19 Tu 08:57, Daniel Gustafsson wrote:
>> On 19 Dec 2023, at 14:51, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>
>> On 2023-12-18 Mo 11:14, Jelte Fennema-Nio wrote:
>>> On Mon, 18 Dec 2023 at 13:42, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>>>> I think this is pretty much ready to go, the attached v4 squashes the changes
>>>> and fixes the man-page which also needed an update. The referenced Wiki page
>>>> will need an edit or two after this goes in, but that's easy enough.
>>> One thing I'm wondering: When both --check and --diff are passed,
>>> should pgindent still early exit with 2 on the first incorrectly
>>> formatted file? Or should it show diffs for all failing files? I'm
>>> leaning towards the latter making more sense.
>> It should show them all.
> Agreed.
>
>>> Related (but not required for this patch): For my pre-commit hook I
>>> would very much like it if there was an option to have pgindent write
>>> the changes to disk, but still exit non-zero, e.g. a --write flag that
>>> could be combined with --check just like --diff and --check can be
>>> combined with this patch. Currently my pre-commit hook need two
>>> separate calls to pgindent to both abort the commit and write the
>>> required changes to disk.
>> Not sold on this. I don't want pgindent applied automatically to my uncommitted changes, but I do want a reminder that I forgot to run it. In any case, as you say it's a different topic.
> I think we risk making the tool confusing if we add too many options which can
> all be combined to suit every need. The posted v5 seems like a good compromise
> I reckon.
>
> Andrew: When applying this, how do we synchronize with the buildfarm to avoid
> false negatives due to the BF using the wrong options?

The only buildfarm animal involved here is koel, which I run, so the
simplest way will be for me to commit the core patch and adjust the
buildfarm code at the same time.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michail Nikolaev 2023-12-20 16:18:18 Re: Revisiting {CREATE INDEX, REINDEX} CONCURRENTLY improvements
Previous Message Nathan Bossart 2023-12-20 16:03:24 Re: introduce dynamic shared memory registry