From: Andrew Dunstan <andrew@dunslane.net>
To: Jelte Fennema <postgres@jeltef.nl>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: Bruce Momjian <bruce@momjian.us>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Noah Misch <noah@leadboat.com>
Cc: Jesse Zhang <sbjesse@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: run pgindent on a regular basis / scripted manner
Date: Thu, 26 Jan 2023 11:54:11 -0500
Message-ID: <8a58ec86-8a0d-1023-87f8-2aa47a860698@dunslane.net> (raw)
In-Reply-To: <CAGECzQRfkuw-A4xvPMm8=uJ=T5dQ35qBcU-wPYE68E0Ciu-cfg@mail.gmail.com>
References: <1097305.1674334785@sss.pgh.pa.us>
<CAH2-WzkHHcPRMj3G1b_dUfY0aR4W4OAHQ9_Z6sDGvL8Ew+aRrA@mail.gmail.com>
<20230121224312.qmwpagrebidr6hi7@awork3.anarazel.de>
<CAH2-Wzm0niYBPcN6eX3PzHABfj0dE3fwifo5bdturvMp=9FGqw@mail.gmail.com>
<20230122094957.x542ojaqgi4lalq4@awork3.anarazel.de>
<CAGECzQRYjHdNQ-Bq2yrBb-8s3Pjx7pfvYPDdNv9tcvDcscDhbg@mail.gmail.com>
<20230122223806.3sgew6qswhsczae7@awork3.anarazel.de>
<1495023.1674430107@sss.pgh.pa.us>
<20230123001518.6hxyiczhn4kadvmf@awork3.anarazel.de>
<1502504.1674433722@sss.pgh.pa.us>
<20230123011002.fzcaa3krql3mqsfn@awork3.anarazel.de>
<1509591.1674437376@sss.pgh.pa.us>
<CAGECzQQG=VLe71VokP9SPMOVc1P9jLPnU0-13bf-fjASMNdo9g@mail.gmail.com>
<b7e3bb37-412b-0eff-2528-63ee264f0078@dunslane.net>
<CAGECzQQmM=hOpV683gdSFj4+VVArY1XpZ+kD8YctBNQpPsMQ+Q@mail.gmail.com>
<f8006b81-2646-bfa8-d770-501e4bf31241@dunslane.net>
<CAGECzQRfkuw-A4xvPMm8=uJ=T5dQ35qBcU-wPYE68E0Ciu-cfg@mail.gmail.com>
On 2023-01-26 Th 11:16, Jelte Fennema wrote:
> On Thu, 26 Jan 2023 at 15:40, Andrew Dunstan <andrew@dunslane.net> wrote:
>> I didn't really like your hook, as it forces a reindent, and many people
>> won't want that (for reasons given elsewhere in this thread).
> I'm not sure what you mean by "forces a reindent". Like I explained
> you can simply run "git commit" again to ignore the changes and
> commit anyway. As long as the files are indented on your filesystem
> the hook doesn't care if you actually included the indentation changes
> in the changes that you're currently committing.
Your hook does this:
+git diff --cached --name-only --diff-filter=ACMR | grep '\.[ch]$' |\
+ xargs src/tools/pgindent/pgindent --silent-diff \
+ || {
+ echo ERROR: Aborting commit because pgindent was not run
+ git diff --cached --name-only --diff-filter=ACMR | grep
'\.[ch]$' | xargs src/tools/pgindent/pgindent
+ exit 1
+ }
At this stage the files are now indented, so if it failed and you run
`git commit` again it will commit with the indention changes.
>
> So to be completely clear you can do the following with my hook:
> git commit # runs pgindent and fails
> git commit # commits changes anyway
> git commit -am 'Run pgindent' # commit indentation changes separately
>
> Or what I usually do:
> git commit # runs pgindent and fails
> git add --patch # choose relevant changes to add to commit
> git commit # commit the changes
> git checkout -- . # undo irrelevant changes on filesystem
>
> Honestly PGAUTOINDENT=no seems stricter, since the only
> way to bypass the failure is now to run manually run pgindent
> or git commit with the --no-verify flag.
>
>> files=$(git diff --cached --name-only --diff-filter=ACMR)
>> src/tools/pgindent/pgindent $files
> That seems like it would fail if there's any files or directories with
> spaces in them. Maybe this isn't something we care about though.
We don't have any, and the filenames git produces are relative to the
git root. I don't think this is an issue.
>
>> # no need to filter files - pgindent ignores everything that isn't a
>> # .c or .h file
> If the first argument is a non .c or .h file, then pgindent interprets
> it as the typedefs file. So it's definitely important to filter non .c
> and .h files out. Because now if you commit a single
> non .c or .h file this hook messes up the indentation in all of
> your files. You can reproduce by running:
> src/tools/pgindent/pgindent README
I have a patch at [1] to remove this misfeature.
>
>> # only do this on master
>> test "$branch" = "master" || return 0
> I would definitely want a way to disable this check. As a normal
> submitter I never work directly on master.
Sure, that's your choice. My intended audience here is committers, who
of course do work on master.
cheers
andrew
[1] https://postgr.es/m/21bb8573-9e56-812b-84cf-1e4f3c4c2a7b@dunslane.net
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: andrew@dunslane.net, postgres@jeltef.nl, tgl@sss.pgh.pa.us, andres@anarazel.de, pg@bowt.ie, bruce@momjian.us, magnus@hagander.net, alvherre@2ndquadrant.com, sfrost@snowman.net, noah@leadboat.com, sbjesse@gmail.com
Subject: Re: run pgindent on a regular basis / scripted manner
In-Reply-To: <8a58ec86-8a0d-1023-87f8-2aa47a860698@dunslane.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox