Bug 39149

Summary: .PL files not tidy
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Pushed to main --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, katrin.fischer, tomascohen, victor
Version: MainKeywords: RM_priority
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:
Bug Depends on: 39096    
Bug Blocks:    
Attachments: Bug 39149: Make tidy.pl deal with .PL files
Bug 39149: Tidy .PL files
Bug 39149: Make tidy.pl deal with .PL files
Bug 39149: Tidy .PL files
Bug 39149: Make perltidy.t also check .PL files
Bug 39149: Make perltidy.t also check .PL files

Description Tomás Cohen Arazi (tcohen) 2025-02-17 17:18:49 UTC
Somehow we missed we have `.PL` files in the codebase. They are not considered in `tidy.pl` and remain untidy.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-02-17 17:41:28 UTC
Created attachment 178148 [details] [review]
Bug 39149: Make tidy.pl deal with .PL files

This patch adds handling for files with the `.PL` extension to the
`tidy.pl` helper script.

It will now consider them Perl files and tidy them as appropriate.

I opted for explicitly listing `PL` instead of making the existing check
case-insensitive because the only files I found with changed case (i.e.
Perl scripts that don't have `.pl`) are:

$ find . -type f -iname "*.pl" ! -name "*.pl"
./fix-perl-path.PL
./build-resources.PL
./rewrite-config.PL
./Makefile.PL

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2025-02-17 17:41:31 UTC
Created attachment 178149 [details] [review]
Bug 39149: Tidy .PL files

This patch is the result of running:

```shell
perl misc/devel/tidy.pl *.PL
```

Commiting raised an error about fix-perl-path.PL calling `warn` which I dismiss for the
time being.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Victor Grousset/tuxayo 2025-02-17 18:10:44 UTC
Good news, it seems there aren't other cases of files with slightly diverging extensions that escaped tidying.

After looking at all the stuff not having the most common extensions:
find . -type f -not -path "./.git/*" -not -path "./misc/translator/po/.git/*" -regextype posix-egrep -not -regex ".*\.(sql|pl|yml|md|yaml|js|txt|xml|json|xsl|chr|pm|t|ts|mrc|csv|inc|pod|att|tag|abs|mar|cfg|conf|properties|ods|po|png|svg|idx|gif|tt|css|vue|html|jpg|in|ogg|woff|woff2|ttf|map|scss|map|py|pref|sh|pdf|odt)$"
Comment 4 Victor Grousset/tuxayo 2025-02-17 19:13:56 UTC
Created attachment 178150 [details] [review]
Bug 39149: Make tidy.pl deal with .PL files

This patch adds handling for files with the `.PL` extension to the
`tidy.pl` helper script.

It will now consider them Perl files and tidy them as appropriate.

I opted for explicitly listing `PL` instead of making the existing check
case-insensitive because the only files I found with changed case (i.e.
Perl scripts that don't have `.pl`) are:

$ find . -type f -iname "*.pl" ! -name "*.pl"
./fix-perl-path.PL
./build-resources.PL
./rewrite-config.PL
./Makefile.PL

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2025-02-17 19:14:00 UTC
Created attachment 178151 [details] [review]
Bug 39149: Tidy .PL files

This patch is the result of running:

```shell
perl misc/devel/tidy.pl *.PL
```

Commiting raised an error about fix-perl-path.PL calling `warn` which I dismiss for the
time being.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2025-02-17 19:15:04 UTC
It works! :)

Testing notes: apply last commit of Bug 39096 before this, otherwise it won't apply.
Because that follow-up isn't on main yet.
Bug 39096: Add .git-blame-ignore-revs mapping to Makefile.PL
Comment 7 Victor Grousset/tuxayo 2025-02-17 19:41:01 UTC
Created attachment 178152 [details] [review]
Bug 39149: Make perltidy.t also check .PL files

Now perltidy.t will check 2958 files instead of 2954. Which matches
with the 4 .PL files.
Comment 8 Tomás Cohen Arazi (tcohen) 2025-02-18 11:02:44 UTC
Created attachment 178170 [details] [review]
Bug 39149: Make perltidy.t also check .PL files

Now perltidy.t will check 2958 files instead of 2954. Which matches
with the 4 .PL files.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Katrin Fischer 2025-02-18 14:36:20 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 10 Jonathan Druart 2025-03-10 11:38:05 UTC
Added a commit to koha-testing-docker for the pre-commit git hooks

https://gitlab.com/koha-community/koha-testing-docker/-/commit/d2b480e2c2462b155153d57bf246de20c4481266