Re: perlcritic and perltidy - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: perlcritic and perltidy
Date
Msg-id 4d944dd9-db54-a1ef-862c-fe20737428c2@2ndQuadrant.com
Whole thread Raw
In response to Re: perlcritic and perltidy  (David Steele <david@pgmasters.net>)
Responses Re: perlcritic and perltidy  (Andrew Dunstan <andrew.dunstan@2ndquadrant.com>)
List 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@paquier.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@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



pgsql-hackers by date:

Previous
From: Euler Taveira
Date:
Subject: Re: Exposing guc_malloc/strdup/realloc for plugins?
Next
From: Peter Eisentraut
Date:
Subject: Re: MAP syntax for arrays