On Thu Mar 12, 2026 at 10:10 AM CET, Peter Eisentraut wrote:
> Apparently, this is still work in progress, but here are some comments
> from me.
Thanks for the review. I went over the code in detail now myself, and
haven't found anything hugehely weird (although I did a bit of cleanup
and additional comments). As said before I'm definetly not a Perl expert
though, so I might have missed some wrong details. But all the logic at least
seems sound.
I also reorderd the commits to have the perltidy integration as the last
ones, and the general pgindent QoL imorovements first.
> I'm very interested in the patch "pgindent: Clean up temp files on
> SIGINT", because this is an annoying problem. But I didn't find any
> documentation about why this is the correct solution. I didn't find
> anything on the File::Temp man page, for example. Do you have more details?
I explained this black magic better now, and also did the same for
SIGTERM. I also added a second commit which cleans up the .BAK files
that pg_bsd_indent creates. Feel free to combine those commits if you
think that's better.
> The patch "pgindent: Use git ls-files to discover files" is also
> interested and pretty straightforward. I guess we don't really care
> about being able to run this in non-git trees?
I don't think we care. This is a tool for Postgres developers, and I
think we can assume all of those are using git to work on Postgres.
> The other stuff I don't think I'm really on board with. In particular,
> I don't like integrating pgperltidy into pgindent. These things are in
> practice run at different times and the underlying tools update
> differently and require different management, so integrating them all
> might lead to various annoyances.
So, to be clear. My goal is to get to a point where they ARE run at the
same time, because their goal is the same, just for different filetypes.
Afaict the primary reason that pg_bsd_indent is run on every commit, but
perltidy is not, is that the later is currently much more annoying to run.
Integrating it into pgindent is my attempt at making that barier much
lower, so that we can make it part of the standard workflow.
> I kind of liked the original idea of a "make format", and then you could
> have subtargets like "make format-c" and "make format-perl" if you don't
> have all the tools.
I liked it too initially, but I then realized that means you lose all
the nice flags that pgindent provides, e.g. --commit=@ or --check.
Ofcourse you could add a FORMATARGS variable, but at that point why not
just have people run pgindent directly? Also, perltidy is very slow to
run. Running it on the whole codebase takes about a minute for me. So
the new parallel and git ls-files support in pgindent are really helpful
to have it finish in a reasonable time.
> The parallel pgindent is pretty nice, but I wonder how "use POSIX" works
> on Windows?
Removed the POSIX thing. That wasn't needed. I'm wondering how many
people use pginden ton Windows though.
> (But in practice I mostly use pgindent --commit=@, which is still much
> faster.)
(--commit=@ should also get faster with the parallelization, because now
it can run of the files from the commit in parallel)
