Re: perlcritic and perltidy

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: David Steele <david(at)pgmasters(dot)net>, Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: perlcritic and perltidy
Date: 2018-05-08 14:06:25
Message-ID: 4d944dd9-db54-a1ef-862c-fe20737428c2@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 05/08/2018 08:31 AM, David Steele wrote:
> On 5/8/18 8:11 AM, Stephen Frost wrote:
>> Greetings,
>>
>> * Michael Paquier (michael(at)paquier(dot)xyz) wrote:
>>> On Sun, May 06, 2018 at 09:14:06PM -0400, Stephen Frost wrote:
>>>> While I appreciate the support, I'm not sure that you're actually
>>>> agreeing with me.. I was arguing that braces should be on their own
>>>> line and therefore there would be a new line for the brace.
>>>> Specifically, when moving lines between hashes, it's annoying to have to
>>>> also worry about if the line being copied/moved has braces at the end or
>>>> not- much easier if they don't and the braces are on their own line.
>>> I should have read that twice. Yes we are not on the same line. Even
>>> if a brace is on a different line, per your argument it would still be
>>> nicer to add a comma at the end of each last element of a hash or an
>>> array, which is what you have done in the tests of pg_dump, but not
>>> something that the proposed patch does consistently. If the formatting
>>> is automated, the way chosen does not matter much, but the extra last
>>> comma should be consistently present as well?
>> Yes, that would be nice as well, as you'd be able to move entries around
>> more easily that way.
> I'm a fan of the final comma as it makes diffs less noisy.
>

Me too.

AFAICT there is no perltidy setting to add them where they are missing.
There is a perlcritic setting to detect them in lists, however. Here is
the output:

andrew(at)emma:pg_head (master)$ {         find . -type f -a \( -name
'*.pl' -o -name '*.pm' \) -print;         find . -type f -perm -100
-exec file {} \; -print                | egrep -i
':.*perl[0-9]*\>'                | cut -d: -f1;     }     | sort -u  |
xargs perlcritic --quiet --single CodeLayout::RequireTrailingCommas
./src/backend/catalog/Catalog.pm: List declaration without trailing
comma at line 30, column 23.  See page 17 of PBP.  (Severity: 1)
./src/backend/catalog/genbki.pl: List declaration without trailing comma
at line 242, column 19.  See page 17 of PBP.  (Severity: 1)
./src/backend/catalog/genbki.pl: List declaration without trailing comma
at line 627, column 20.  See page 17 of PBP.  (Severity: 1)
./src/backend/utils/mb/Unicode/UCS_to_most.pl: List declaration without
trailing comma at line 23, column 16.  See page 17 of PBP.  (Severity: 1)
./src/backend/utils/mb/Unicode/UCS_to_SJIS.pl: List declaration without
trailing comma at line 21, column 19.  See page 17 of PBP.  (Severity: 1)
./src/interfaces/ecpg/preproc/check_rules.pl: List declaration without
trailing comma at line 38, column 20.  See page 17 of PBP.  (Severity: 1)
./src/test/perl/PostgresNode.pm: List declaration without trailing comma
at line 1468, column 14.  See page 17 of PBP.  (Severity: 1)
./src/test/perl/PostgresNode.pm: List declaration without trailing comma
at line 1657, column 16.  See page 17 of PBP.  (Severity: 1)
./src/test/perl/PostgresNode.pm: List declaration without trailing comma
at line 1697, column 12.  See page 17 of PBP.  (Severity: 1)
./src/test/recovery/t/003_recovery_targets.pl: List declaration without
trailing comma at line 119, column 20.  See page 17 of PBP.  (Severity: 1)
./src/test/recovery/t/003_recovery_targets.pl: List declaration without
trailing comma at line 125, column 20.  See page 17 of PBP.  (Severity: 1)
./src/test/recovery/t/003_recovery_targets.pl: List declaration without
trailing comma at line 131, column 20.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Install.pm: List declaration without trailing comma at
line 22, column 28.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Install.pm: List declaration without trailing comma at
line 81, column 17.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Install.pm: List declaration without trailing comma at
line 653, column 14.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Install.pm: List declaration without trailing comma at
line 709, column 15.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Mkvcbuild.pm: List declaration without trailing comma
at line 43, column 24.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Mkvcbuild.pm: List declaration without trailing comma
at line 55, column 29.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Mkvcbuild.pm: List declaration without trailing comma
at line 59, column 31.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Mkvcbuild.pm: List declaration without trailing comma
at line 75, column 25.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/Mkvcbuild.pm: List declaration without trailing comma
at line 623, column 15.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/vcregress.pl: List declaration without trailing comma
at line 102, column 13.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/vcregress.pl: List declaration without trailing comma
at line 121, column 13.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/vcregress.pl: List declaration without trailing comma
at line 147, column 17.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/vcregress.pl: List declaration without trailing comma
at line 167, column 13.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/vcregress.pl: List declaration without trailing comma
at line 352, column 14.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/vcregress.pl: List declaration without trailing comma
at line 404, column 13.  See page 17 of PBP.  (Severity: 1)
./src/tools/msvc/vcregress.pl: List declaration without trailing comma
at line 554, column 10.  See page 17 of PBP.  (Severity: 1)

I'll take a look those. There doesn't seem to be a reliable one to
detect them in things other than lists (like anonymous hash and array
contructors using {} and []). There is one in the Pulp collection of
policies, but it doesn't seem to work very well.

But this is all a bit away from the original discussion.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-05-08 14:15:33 Re: MAP syntax for arrays
Previous Message Euler Taveira 2018-05-08 14:02:48 Re: Exposing guc_malloc/strdup/realloc for plugins?