Content-Type: multipart/alternative;
 boundary="------------omNWrPOkc3AfM0c26HZYUUWn"
Message-ID: <5611b8aa-8496-4632-92b4-e096654850d1@dunslane.net>
Date: Wed, 31 Dec 2025 10:48:00 -0500
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Add "format" target to make and ninja to run pgindent and
 pgperltidy
To: Tom Lane <tgl@sss.pgh.pa.us>,
 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: Jelte Fennema-Nio <postgres@jeltef.nl>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Daniel Gustafsson <daniel@yesql.se>
References: <DFCDD5H4J7VX.3GJKRBBDCKQ86@jeltef.nl>
 <CAExHW5tuJTmuikfkabWq+mvomucY3iQ_vV-10KAsZWM7Yg6tvg@mail.gmail.com>
 <3913298.1767194804@sss.pgh.pa.us>
From: Andrew Dunstan <andrew@dunslane.net>
Content-Language: en-US
Autocrypt: addr=andrew@dunslane.net; keydata=
 xsBNBE7KWFkBCAClridxur2AIc7eW2AR7izbfp3EnNefie2HbLF0izW5Ik5UjX2HBXBx4syI
 gY6b0ugohXrr274+baoAlvSbq6cAoQuEVrk5IZFzt20b1Xkx65FwGSEj526yiKLocqkJceSq
 Xr9xcA5SGY+FZv441chh5SU92v4q6z+6LPpoHOh97ptAVXZYNTtU0LevyvD5lja0TzbvJm6C
 eFXitJfnm1pLEr0DGJCR/iUOl/N62Kh4855zZC7NHIjQHPOvV5Stz/l5ilDhvGVk+xkXFPys
 SjZoUr1rXhYLpiyi5sR0X9FHXT0KnGuz1F5ERO7ZTLSSQ6fJwPj6gOk9K+vvoKvoeql5ABEB
 AAHNJEFuZHJldyBEdW5zdGFuIDxhbmRyZXdAZHVuc2xhbmUubmV0PsLAlwQTAQgAQQIbAwIX
 gAIZAQULCQgHAwUVCgkICwUWAgMBAAIeBRYhBOQ+WEYd/Hy/RGkVpZn6f8tZ/DuBBQJoGNGd
 BQkdEO8nAAoJEJn6f8tZ/DuBq74H/jkTR4Zi3stbw+xC7v2u3QozssK7MYPL2AsVfh7OealS
 h182fiWXpfvmmAB7WUHbhk9GC2RAOnHI/2d2jgKaMLAHsGYOT0YopTVIwRY43fCw/mK67yxc
 wmDcX+zyKfLaivNbf5A7QPLNwda98bEAMSJ8Sn652Uc6cA8t3uKGsVzbRBQOoYzjgvBCfSrE
 9ql3PDNg0l4BfAqabd2f70ZUm9VAMEPrgv/v2xI7M2XiL4g5BVmqLCOwxLM8RMCotCuoweUr
 VO43DeBCIDwLxotMJKvGWDjBzQYlU1NPUAtNcz/gN9ITUe1VUGjyvGj4u1lxBOcQQUw7l1+T
 5moZ4iZxXzvOwE0ETspYWQEIANGc4zQULOxhbqO2dyD51YhqCNRmm9oKWaqf+wmW4tpDe/VV
 cxAnNizd4LWCHfzpb5cHAtGkOPePMfzWVf6nvdF7d3eglbtf59+zG7O7llV0xSSoFiieQBsr
 GvqDInXYX/4mRRXMtyhM353/tixC9RWLs1oofyYmCPPXXY7h9R7en3B8BoVrRFcdzlIY/NFN
 hFGW/9dkEiGjgna2Rk6e15kln4ZvFBWUg23p93w/pqXcxY6+k/8TEk+C4R+M6w7o2PLGOjdZ
 +kPiUcw5H85zf/yZJwQXzisXaNduwWB6Vads9YC9dj6kPR1c4VGRqAaYL++LAEOqrlvm2Tvq
 QqZRtnEAEQEAAcLAfAQYAQgAJgIbDBYhBOQ+WEYd/Hy/RGkVpZn6f8tZ/DuBBQJoGNI2BQkd
 EODdAAoJEJn6f8tZ/DuBfw0IAKTsfD40teP/pp+bsLLMSxPXUYrrprTj7WFB5v61p6dkpSr/
 qXmMlyahdxQFaPmfVgVirB1Vk/kHiWNnnGjfUV9nB2Zg9LI0Xb9/ts3LsUiRWXzG3tkMY6XL
 vsVOxW4XFRND9l2q+WW93aZ1DZl+fqWfYgMvsusFRhmGFOKTRfKPta2Pkv+AhA24N4+PrR5p
 bU4k2MO8PAGiK8eaYKGFG1bHKuAvoDoF7WXJ3FHxuWqLnKEt4dfOLm5pAe3zq1Lt6q8azT9i
 QWGpSAK5vQUWQHBHpiDjdPeqKZ6HiAXIIKfSmb+jrvXBqoP+D6/K7rUjG2aXiRtTIAXms9sm
 VRu7cmw=
In-Reply-To: <3913298.1767194804@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/5611b8aa-8496-4632-92b4-e096654850d1%40dunslane.net>
Precedence: bulk

This is a multi-part message in MIME format.
--------------omNWrPOkc3AfM0c26HZYUUWn
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 2025-12-31 We 10:26 AM, Tom Lane wrote:
> Ashutosh Bapat<ashutosh.bapat.oss@gmail.com> writes:
>> On Wed, Dec 31, 2025 at 5:06 PM Jelte Fennema-Nio<postgres@jeltef.nl> wrote:
>>> This tries to make running formatting a lot easier for committers, but
>>> primarily for new contributors.
>> I generally like the idea. Since perltidy is not enforced regularly
>> (like pgindent), running it usually ends up modifying files which are
>> not part of the patch. So I avoid it if not necessary. Do you propose
>> to make it optional?
> The other obstacle is that not everybody will have the right version
> of perltidy installed, but using some other version will create a
> whole lot of extraneous noise.


pgperltidy actually checks the version now.


>
> On the whole I'd recommend not trying to automate the perltidy
> step yet.  Cost/benefit is just not very good.


I'd kinda like to unify these universes, though. We could import the 
pgperltidy logic into pgindent but disable it unless some flag were 
given and the correct version of perltidy were present.


>
> On the substance of the patch: I wonder whether we could make things
> more reliable by using git metadata to figure out which .h and .c
> files to point pgindent at.
>
> 			


The git pre-commit hook I use operates with this set of files:

     files=$(git diff --cached --name-only --diff-filter=ACMR)

pgindent just currently looks at that set of files and ignores 
everything that's not a .c or .h file.

I guess what you're wanting is a test to see if the file is in git or a 
generated file? That doesn't really arise for me as I always do vpath 
builds, so generated files are always elsewhere.


cheers


andrew


--
Andrew Dunstan
EDB:https://www.enterprisedb.com

--------------omNWrPOkc3AfM0c26HZYUUWn
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2025-12-31 We 10:26 AM, Tom Lane
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:3913298.1767194804@sss.pgh.pa.us">
      <pre wrap="" class="moz-quote-pre">Ashutosh Bapat <a class="moz-txt-link-rfc2396E" href="mailto:ashutosh.bapat.oss@gmail.com">&lt;ashutosh.bapat.oss@gmail.com&gt;</a> writes:
</pre>
      <blockquote type="cite">
        <pre wrap="" class="moz-quote-pre">On Wed, Dec 31, 2025 at 5:06 PM Jelte Fennema-Nio <a class="moz-txt-link-rfc2396E" href="mailto:postgres@jeltef.nl">&lt;postgres@jeltef.nl&gt;</a> wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="" class="moz-quote-pre">This tries to make running formatting a lot easier for committers, but
primarily for new contributors.
</pre>
        </blockquote>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
</pre>
      <blockquote type="cite">
        <pre wrap="" class="moz-quote-pre">I generally like the idea. Since perltidy is not enforced regularly
(like pgindent), running it usually ends up modifying files which are
not part of the patch. So I avoid it if not necessary. Do you propose
to make it optional?
</pre>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
The other obstacle is that not everybody will have the right version
of perltidy installed, but using some other version will create a
whole lot of extraneous noise.</pre>
    </blockquote>
    <p><br>
    </p>
    <p><br>
    </p>
    <p>pgperltidy actually checks the version now.</p>
    <p><br>
    </p>
    <blockquote type="cite" cite="mid:3913298.1767194804@sss.pgh.pa.us">
      <pre wrap="" class="moz-quote-pre">

On the whole I'd recommend not trying to automate the perltidy
step yet.  Cost/benefit is just not very good.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>I'd kinda like to unify these universes, though. We could import
      the pgperltidy logic into pgindent but disable it unless some flag
      were given and the correct version of perltidy were present. <br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite" cite="mid:3913298.1767194804@sss.pgh.pa.us">
      <pre wrap="" class="moz-quote-pre">

On the substance of the patch: I wonder whether we could make things
more reliable by using git metadata to figure out which .h and .c
files to point pgindent at.

			</pre>
    </blockquote>
    <p><br>
    </p>
    <p>The git pre-commit hook I use operates with this set of files:</p>
    <p>    files=$(git diff --cached --name-only --diff-filter=ACMR)</p>
    <p>pgindent just currently looks at that set of files and ignores
      everything that's not a .c or .h file.</p>
    <p>I guess what you're wanting is a test to see if the file is in
      git or a generated file? That doesn't really arise for me as I
      always do vpath builds, so generated files are always elsewhere.<br>
    </p>
    <p><br>
    </p>
    <p>cheers</p>
    <p><br>
    </p>
    <p>andrew<br>
    </p>
    <p><br>
    </p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <pre class="moz-signature"
    signature-switch-id="d0437855-2267-4610-80b3-83167ec45b0b" cols="72">--
Andrew Dunstan
EDB: <a class="moz-txt-link-freetext" href="https://www.enterprisedb.com">https://www.enterprisedb.com</a>
</pre>
  </body>
</html>

--------------omNWrPOkc3AfM0c26HZYUUWn--