pgsql: Fix perltidy breaking perlcritic

Started by Alvaro Herreraabout 4 years ago8 messageshackerscomitters
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t46569
psql -h localhost -U postgres

Built from patchset v6 (message #6), August 18, 2026 at 03:44 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t46569_6 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t46569_6 && git checkout t46569_6

Patchset v6 (message #6) is on t46569_6

Jump to latest
#1Alvaro Herrera
alvherre@2ndquadrant.com
hackerscomitters

Fix perltidy breaking perlcritic

perltidying a "##no critic" line moves the marker to where it becomes
useless. Put the line back to how it was, and protect it from further
malfeasance.

Per buildfarm member crake.

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/12d40d4a8d0495cf2c7b564daa8aaa7f107a6c56

Modified Files
--------------
src/backend/catalog/Catalog.pm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

#2Andrew Dunstan
andrew@dunslane.net
In reply to: Alvaro Herrera (#1)
hackerscomitters
Re: pgsql: Fix perltidy breaking perlcritic

On Thu, Sep 8, 2022 at 5:23 AM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

Fix perltidy breaking perlcritic

perltidying a "##no critic" line moves the marker to where it becomes
useless. Put the line back to how it was, and protect it from further
malfeasance.

A better way do do this IMNSHO is to put the eval in a block on its own
along with the no critic marker on its own line, like this:

{
## no critic (ProhibitStringyEval)
eval ...
}

perlcritic respects block boundaries for its directives.

cheers

andrew

#3John Naylor
john.naylor@enterprisedb.com
In reply to: Andrew Dunstan (#2)
hackerscomitters
Re: pgsql: Fix perltidy breaking perlcritic

On Fri, Sep 9, 2022 at 3:32 AM Andrew Dunstan <andrew@dunslane.net> wrote:

A better way do do this IMNSHO is to put the eval in a block on its own along with the no critic marker on its own line, like this:

{
## no critic (ProhibitStringyEval)
eval ...
}

perlcritic respects block boundaries for its directives.

I tried that in the attached -- it looks a bit nicer but requires more
explanation. I don't have strong feelings either way.

--
John Naylor
EDB: http://www.enterprisedb.com

Attachments:

perlcritic-block-scope.patchtext/x-patch; charset=US-ASCII; name=perlcritic-block-scope.patchDownload+8-5
#4Andrew Dunstan
andrew@dunslane.net
In reply to: John Naylor (#3)
hackerscomitters
Re: pgsql: Fix perltidy breaking perlcritic

On Fri, Sep 9, 2022 at 10:44 PM John Naylor <john.naylor@enterprisedb.com>
wrote:

On Fri, Sep 9, 2022 at 3:32 AM Andrew Dunstan <andrew@dunslane.net> wrote:

A better way do do this IMNSHO is to put the eval in a block on its own

along with the no critic marker on its own line, like this:

{
## no critic (ProhibitStringyEval)
eval ...
}

perlcritic respects block boundaries for its directives.

I tried that in the attached -- it looks a bit nicer but requires more
explanation. I don't have strong feelings either way.

Maybe even better would be just this, which I bet perltidy would not monkey
with, and would require no explanation:

eval "\$hash_ref = $_"; ## no critic (ProhibitStringyEval)

cheers

andrew

In reply to: Andrew Dunstan (#4)
hackerscomitters
Re: pgsql: Fix perltidy breaking perlcritic

[resending to -hackers instead of -committers]

Andrew Dunstan <andrew@dunslane.net> writes:

On Fri, Sep 9, 2022 at 10:44 PM John Naylor <john.naylor@enterprisedb.com>
wrote:

On Fri, Sep 9, 2022 at 3:32 AM Andrew Dunstan <andrew@dunslane.net> wrote:

A better way do do this IMNSHO is to put the eval in a block on its own

along with the no critic marker on its own line, like this:

{
## no critic (ProhibitStringyEval)
eval ...
}

perlcritic respects block boundaries for its directives.

I tried that in the attached -- it looks a bit nicer but requires more
explanation. I don't have strong feelings either way.

Maybe even better would be just this, which I bet perltidy would not monkey
with, and would require no explanation:

eval "\$hash_ref = $_"; ## no critic (ProhibitStringyEval)

I didn't see this until it got committed, since I'm not subscribed to
-committers, but I think it would be even better to rely on the fact
that eval returns the value of the last expression in the string, which
also gets rid of the ugly quoting and escaping, per the attached.

- ilmari

Attachments:

0001-Use-return-value-of-eval-instead-of-assigning-inside.patchtext/x-diffDownload+2-3
In reply to: Andrew Dunstan (#4)
hackerscomitters
Re: pgsql: Fix perltidy breaking perlcritic

Andrew Dunstan <andrew@dunslane.net> writes:

On Fri, Sep 9, 2022 at 10:44 PM John Naylor <john.naylor@enterprisedb.com>
wrote:

On Fri, Sep 9, 2022 at 3:32 AM Andrew Dunstan <andrew@dunslane.net> wrote:

A better way do do this IMNSHO is to put the eval in a block on its own

along with the no critic marker on its own line, like this:

{
## no critic (ProhibitStringyEval)
eval ...
}

perlcritic respects block boundaries for its directives.

I tried that in the attached -- it looks a bit nicer but requires more
explanation. I don't have strong feelings either way.

Maybe even better would be just this, which I bet perltidy would not monkey
with, and would require no explanation:

eval "\$hash_ref = $_"; ## no critic (ProhibitStringyEval)

I didn't see this until it got committed, since I'm not subscribed to
-committers, but I think it would be even better to rely on the fact
that eval returns the value of the last expression in the string, which
also gets rid of the ugly quoting and escaping, per the attached.

- ilmari

Attachments:

t46569_6
0001-Use-return-value-of-eval-instead-of-assigning-inside.patchtext/x-diffDownload+2-3
#7John Naylor
john.naylor@enterprisedb.com
In reply to: Dagfinn Ilmari Mannsåker (#6)
hackerscomitters
Re: pgsql: Fix perltidy breaking perlcritic

On Mon, Sep 12, 2022 at 4:54 PM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:

eval "\$hash_ref = $_"; ## no critic (ProhibitStringyEval)

I didn't see this until it got committed, since I'm not subscribed to
-committers, but I think it would be even better to rely on the fact
that eval returns the value of the last expression in the string, which
also gets rid of the ugly quoting and escaping, per the attached.

Hmm, interesting.
--
John Naylor
EDB: http://www.enterprisedb.com

#8Andrew Dunstan
andrew@dunslane.net
In reply to: John Naylor (#7)
hackerscomitters
Re: pgsql: Fix perltidy breaking perlcritic

On 2022-09-13 Tu 05:25, John Naylor wrote:

On Mon, Sep 12, 2022 at 4:54 PM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:

eval "\$hash_ref = $_"; ## no critic (ProhibitStringyEval)

I didn't see this until it got committed, since I'm not subscribed to
-committers, but I think it would be even better to rely on the fact
that eval returns the value of the last expression in the string, which
also gets rid of the ugly quoting and escaping, per the attached.

Hmm, interesting.

I agree it's a slight stylistic improvement. I was trying to keep as
close as possible to the original.

cheers

andrew

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