Content-Type: multipart/mixed; boundary="------------U6lEBhhSs2bDHhEvE9ALJX0g"
Message-ID: <3cdb09f1-6375-4198-e8c1-e33ca4c96304@dunslane.net>
Date: Sat, 21 Jan 2023 10:02:15 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: run pgindent on a regular basis / scripted manner
Content-Language: en-US
From: Andrew Dunstan <andrew@dunslane.net>
To: pgsql-hackers@lists.postgresql.org
References: <CABUevExkB=+FX5Ht2sU6Tmk1fF0h8LqugW5camQ-r00ftPZoSg@mail.gmail.com>
 <20200814202651.GA31108@alvherre.pgsql>
 <CABUevEwGPPaUtObrDywO+vhpFDTJXO+-aeWJd7D46uXTE2sQpA@mail.gmail.com>
 <CAGECzQQs+hz4uQSmD_8oRLr74GtxpE0iXuYE85+RHAzvkF_yvQ@mail.gmail.com>
 <531251.1674234545@sss.pgh.pa.us>
 <20230120175806.iogdcra2xpmmh5de@awork3.anarazel.de>
 <538672.1674238787@sss.pgh.pa.us>
 <c9c9fa6d-6de6-48c2-4f8b-0fbeef026439@dunslane.net>
 <17455b0f-0161-9aa9-b6dd-1279e947b8f4@dunslane.net>
In-Reply-To: <17455b0f-0161-9aa9-b6dd-1279e947b8f4@dunslane.net>
Archived-At: <https://www.postgresql.org/message-id/3cdb09f1-6375-4198-e8c1-e33ca4c96304%40dunslane.net>
Precedence: bulk

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


On 2023-01-21 Sa 10:00, Andrew Dunstan wrote:
> On 2023-01-21 Sa 08:26, Andrew Dunstan wrote:
>> On 2023-01-20 Fr 13:19, Tom Lane wrote:
>>> Andres Freund <andres@anarazel.de> writes:
>>>> On 2023-01-20 12:09:05 -0500, Tom Lane wrote:
>>>>> The core problem here is that requiring that would translate to
>>>>> requiring every code contributor to have a working copy of pg_bsd_indent.
>>>> Wouldn't just every committer suffice?
>>> Not if we have cfbot complaining about it.
>>>
>>> (Another problem here is that there's a sizable subset of committers
>>> who clearly just don't care, and I'm not sure we can convince them to.)
>> I think we could do better with some automation tooling for committers
>> here. One low-risk and simple change would be to provide a
>> non-destructive mode for pgindent that would show you the changes if any
>> it would make. That could be worked into a git pre-commit hook that
>> committers could deploy. I can testify to the usefulness of such hooks -
>> I have one that while not perfect has saved me on at least two occasions
>> from forgetting to bump the catalog version.
>>
>> I'll take a look at fleshing this out, for my own if no-one else's use.
>>
>>
> Here's a quick patch for this. I have it in mind to use like this in a
> pre-commit hook:
>
>     # only do this on master
>     test `git rev-parse --abbrev-ref HEAD` = "master" || exit 0
>
>     src/tools/pgindent/pg_indent --silent `git diff --cached --name-only` || \
>
>       { echo "Need a pgindent run" >&2 ; exit 1; }
>
>
> The committer could then run
>
>     src/tools/pgindent/pg_indent --show-diff `git diff --cached --name-only`
>
> to see what changes it thinks are needed.
>
>
This time with patch.


cheers


andrew

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

--------------U6lEBhhSs2bDHhEvE9ALJX0g
Content-Type: text/x-patch; charset=UTF-8;
 name="pgindent_silent_plus_showdiff.patch"
Content-Disposition: attachment;
 filename="pgindent_silent_plus_showdiff.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy90b29scy9wZ2luZGVudC9wZ2luZGVudCBiL3NyYy90b29scy9w
Z2luZGVudC9wZ2luZGVudAppbmRleCA3NDFiMGNjYjU4Li4yODM0ZjVjOWZjIDEwMDc1NQot
LS0gYS9zcmMvdG9vbHMvcGdpbmRlbnQvcGdpbmRlbnQKKysrIGIvc3JjL3Rvb2xzL3BnaW5k
ZW50L3BnaW5kZW50CkBAIC0yMSw3ICsyMSw4IEBAIG15ICRpbmRlbnRfb3B0cyA9CiAKIG15
ICRkZXZudWxsID0gRmlsZTo6U3BlYy0+ZGV2bnVsbDsKIAotbXkgKCR0eXBlZGVmc19maWxl
LCAkdHlwZWRlZl9zdHIsICRjb2RlX2Jhc2UsICRleGNsdWRlcywgJGluZGVudCwgJGJ1aWxk
KTsKK215ICgkdHlwZWRlZnNfZmlsZSwgJHR5cGVkZWZfc3RyLCAkY29kZV9iYXNlLCAkZXhj
bHVkZXMsICRpbmRlbnQsICRidWlsZCwKKwkkc2hvd19kaWZmLCAkc2lsZW50X2RpZmYpOwog
CiBteSAlb3B0aW9ucyA9ICgKIAkidHlwZWRlZnM9cyIgICAgICAgICA9PiBcJHR5cGVkZWZz
X2ZpbGUsCkBAIC0yOSw5ICszMCwxNCBAQCBteSAlb3B0aW9ucyA9ICgKIAkiY29kZS1iYXNl
PXMiICAgICAgICA9PiBcJGNvZGVfYmFzZSwKIAkiZXhjbHVkZXM9cyIgICAgICAgICA9PiBc
JGV4Y2x1ZGVzLAogCSJpbmRlbnQ9cyIgICAgICAgICAgID0+IFwkaW5kZW50LAotCSJidWls
ZCIgICAgICAgICAgICAgID0+IFwkYnVpbGQsKTsKKwkiYnVpbGQiICAgICAgICAgICAgICA9
PiBcJGJ1aWxkLAorICAgICJzaG93LWRpZmYiICAgICAgICAgID0+IFwkc2hvd19kaWZmLAor
ICAgICJzaWxlbnRfZGlmZiIgICAgICAgID0+IFwkc2lsZW50X2RpZmYsKTsKIEdldE9wdGlv
bnMoJW9wdGlvbnMpIHx8IGRpZSAiYmFkIGNvbW1hbmQgbGluZSBhcmd1bWVudFxuIjsKIAor
ZGllICJDYW5ub3QgaGF2ZSBib3RoIC0tc2lsZW50LWRpZmYgYW5kIC0tc2hvdy1kaWZmIgor
ICBpZiAkc2lsZW50X2RpZmYgJiYgJHNob3dfZGlmZjsKKwogcnVuX2J1aWxkKCRjb2RlX2Jh
c2UpIGlmICgkYnVpbGQpOwogCiAjIGNvbW1hbmQgbGluZSBvcHRpb24gd2lucywgdGhlbiBm
aXJzdCBub24tb3B0aW9uIGFyZywKQEAgLTI4MywzMCArMjg5LDE5IEBAIHN1YiBydW5faW5k
ZW50CiAKIH0KIAotCi0jIGZvciBkZXZlbG9wbWVudCBkaWFnbm9zdGljcwotc3ViIGRpZmYK
K3N1YiBzaG93X2RpZmYKIHsKLQlteSAkcHJlICAgPSBzaGlmdDsKLQlteSAkcG9zdCAgPSBz
aGlmdDsKLQlteSAkZmxhZ3MgPSBzaGlmdCB8fCAiIjsKKwlteSAkaW5kZW50ZWQgICA9IHNo
aWZ0OworCW15ICRzb3VyY2VfZmlsZW5hbWUgID0gc2hpZnQ7CiAKLQlwcmludCBTVERFUlIg
InJ1bm5pbmcgZGlmZlxuIjsKLQotCW15ICRwcmVfZmggID0gbmV3IEZpbGU6OlRlbXAoVEVN
UExBVEUgPT4gInBnZGlmZmJYWFhYWCIpOwogCW15ICRwb3N0X2ZoID0gbmV3IEZpbGU6OlRl
bXAoVEVNUExBVEUgPT4gInBnZGlmZmFYWFhYWCIpOwogCi0JcHJpbnQgJHByZV9maCAkcHJl
OwotCXByaW50ICRwb3N0X2ZoICRwb3N0OworCXByaW50ICRwb3N0X2ZoICRpbmRlbnRlZDsK
IAotCSRwcmVfZmgtPmNsb3NlKCk7CiAJJHBvc3RfZmgtPmNsb3NlKCk7CiAKLQlzeXN0ZW0o
ICJkaWZmICRmbGFncyAiCi0JCSAgLiAkcHJlX2ZoLT5maWxlbmFtZSAuICIgIgotCQkgIC4g
JHBvc3RfZmgtPmZpbGVuYW1lCi0JCSAgLiAiID4mMiIpOwotCXJldHVybjsKKwlteSAkZGlm
ZiA9IGBkaWZmIC11ICRzb3VyY2VfZmlsZW5hbWUgJHBvc3RfZmgtPmZpbGVuYW1lICAyPiYx
YCk7CisJcmV0dXJuICRkaWZmOwogfQogCiAKQEAgLTQwNCw2ICszOTksOCBAQCBwdXNoKEBm
aWxlcywgQEFSR1YpOwogCiBmb3JlYWNoIG15ICRzb3VyY2VfZmlsZW5hbWUgKEBmaWxlcykK
IHsKKwkjIGlnbm9yZSBhbnl0aGluZyB0aGF0J3Mgbm90IGEgLmMgb3IgLmggZmlsZQorCW5l
eHQgdW5sZXNzICRzb3VyY2VfZmlsZW5hbWUgPX4vXC5bY2hdJC87CiAKIAkjIEF1dG9tYXRp
Y2FsbHkgaWdub3JlIC5jIGFuZCAuaCBmaWxlcyB0aGF0IGNvcnJlc3BvbmQgdG8gYSAueSBv
ciAubAogCSMgZmlsZS4gIGluZGVudCB0ZW5kcyB0byBnZXQgYmFkbHkgY29uZnVzZWQgYnkg
Qmlzb24vZmxleCBvdXRwdXQsCkBAIC00MjksNyArNDI2LDI0IEBAIGZvcmVhY2ggbXkgJHNv
dXJjZV9maWxlbmFtZSAoQGZpbGVzKQogCiAJJHNvdXJjZSA9IHBvc3RfaW5kZW50KCRzb3Vy
Y2UsICRzb3VyY2VfZmlsZW5hbWUpOwogCi0Jd3JpdGVfc291cmNlKCRzb3VyY2UsICRzb3Vy
Y2VfZmlsZW5hbWUpIGlmICRzb3VyY2UgbmUgJG9yaWdfc291cmNlOworCWlmICgkc291cmNl
IG5lICRvcmlnX3NvdXJjZSkKKwl7CisJCWlmICgkc2lsZW50X2RpZmYpCisJCXsKKwkJCWV4
aXQgMjsKKwkJfQorCQllbHNpZiAoJHNob3dfZGlmZikKKwkJeworCQkJcHJpbnQgc2hvd19k
aWZmKCRzb3VyY2UsICRzb3VyY2VfZmlsZW5hbWUpOworCQl9CisJCWVsc2UKKwkJeworCQkJ
d3JpdGVfc291cmNlKCRzb3VyY2UsICRzb3VyY2VfZmlsZW5hbWUpOworCQl9CisJfQorCiB9
CiAKIGJ1aWxkX2NsZWFuKCRjb2RlX2Jhc2UpIGlmICRidWlsZDsKKworZXhpdCAwOwo=

--------------U6lEBhhSs2bDHhEvE9ALJX0g--