Re: run pgindent on a regular basis / scripted manner

> 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.

No, because at no point a "git add" is happening, so the changes
made by pgindent are not staged. As long as you don't run the
second "git commit" with the -a flag the commit will be exactly
the same as you prepared it before.

> Sure, that's your choice. My intended audience here is committers, who
> of course do work on master.

Yes I understand, I meant it would be nice if the script had a environment
variable like PG_COMMIT_HOOK_ALL_BRANCHES (bad name)
for this purpose.

On Thu, 26 Jan 2023 at 17:54, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> 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
>