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
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? |