Re: run pgindent on a regular basis / scripted manner

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