Re: run pgindent on a regular basis / scripted manner

From: Jelte Fennema <postgres(at)jeltef(dot)nl>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)bowt(dot)ie>, Bruce Momjian <bruce(at)momjian(dot)us>, Magnus Hagander <magnus(at)hagander(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Noah Misch <noah(at)leadboat(dot)com>, Jesse Zhang <sbjesse(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: run pgindent on a regular basis / scripted manner
Date: 2023-01-21 23:39:25
Message-ID: CAGECzQQPKK3VBMPRrJKnW03vJXBcAbYM16v3UDP-thpyUrx6XA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Attached are a .clang-format file and an uncrustify.cfg file that I
whipped up to match the current style at least somewhat.

I was unable to get either of them to produce the weird alignment of
declarations that pgindent outputs. Maybe that's just because I don't
understand what this alignment is supposed to do. Because to me the
current alignment seems completely random most of the time (and like
Andres said also not very unhelpful). For clang-format you should use
at least clang-format 15, otherwise it has some bugs in the alignment
logic.

One thing that clang-format really really wants to change in the
codebase, is making it consistent on what to do when
arguments/parameters don't fit on a single line: You can either choose
to minimally pack everything on the minimum number of lines, or to put
all arguments on their own separate line. Uncrustify is a lot less
strict about that and will leave most things as they currently are.

Overall it seems that uncrustify is a lot more customizable, whether
that's a good thing is debatable. Personally I think the fewer
possible debates I have about codestyle the better, so I usually like
the tools with fewer options better. But if the customizability allows
for closer matching of existing codestyle then it might be worth the
extra debates and effort in customization in this case.

> 1) revert the re-indent commits in $backbranch
> 2) merge $backbranch-with-revert into $forkbranch
> 3) re-indent $forkbranch

The git commands to achieve this are something like the following.
I've used such git commands in the past to make big automatic
refactors much easier to get in and it has worked quite well so far.

git checkout forkbranch
git rebase {commit-before-re-indent}
# now, get many merge conflicts
git rebase {re-indent-commit}
# keep your own changes (its --theirs instead of --ours because rebase
flips it around)
git checkout --theirs .
run-new-reindent
git add .
git rebase --continue

On Sat, 21 Jan 2023 at 23:47, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2023-01-21 17:20:35 -0500, Tom Lane wrote:
> > I don't feel wedded to every last detail of what pgindent does (and
> > especially not the bugs). But I think if the new tool is not a pretty
> > close match we'll be in for years of back-patching pain. We have made
> > changes in pgindent itself in the past, and the patching consequences
> > weren't *too* awful, but the changes weren't very big either.
>
> Perhaps we could backpatch formatting changes in a way that doesn't
> inconvenience forks too much. One way to deal with such changes is to
>
> 1) revert the re-indent commits in $backbranch
> 2) merge $backbranch-with-revert into $forkbranch
> 3) re-indent $forkbranch
>
> After that future changes should be mergable again.
>
> Certainly doesn't do away with the pain entirely, but it does make it perhaps
> bearable
>
> Greetings,
>
> Andres Freund

Attachment Content-Type Size
uncrustify.cfg application/octet-stream 574 bytes
.clang-format application/octet-stream 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-01-22 01:16:01 GUCs to control abbreviated sort keys
Previous Message Peter Geoghegan 2023-01-21 23:32:45 Re: run pgindent on a regular basis / scripted manner