https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=2269 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- It should be installed at the correct place and used by the IDE, as well as by the QA check that should exist to catch incorrect tidy. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #3 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 149027 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=149027&action=edit Bug 30002: Add .perltidyrc The options set in this file try to match the current code style without deviating too much from perltidy's defaults. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I would extend the max line length --maximum-line-length=178 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- What is the status of this? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- The current max line length is too short, and 178 is too much in my testing. For instance I'd like to see my $h = { one => 1, two => 3, }; always on 2 lines my $h = { one => 1, two => 3, }; Otherwise 178 leads to hard to read structure. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #12 from Julian Maurice <julian.maurice@biblibre.com> --- --comma-arrow-breakpoints=0 appears to solve the issue as well. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I can see why Julian didn't extend the line length.. I thought I'd have a go with the 178 suggestion and didn't like it at all. For me, I max out at around 120 for preference. Reasoning behind that for me is that I often read code on a widescreen monitor in portrait orientation. and at my comfortable font size 120 chars at (my term actually fits 128, but I don't mind the small right margin.. 120 happens to match what the mojolicious project uses). This also allows me to put two windows side by side on a wide screen and not have my lines get wrapped. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- How to move this forward the best? I think we need to decide on line length, does 120 sound like a reasonable compromise? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #16 from David Nind <david@davidnind.com> --- Discussion at the 26 April 2023 Development IRC Meeting. To vote on this at the next Development IRC Meeting - see https://meetings.koha-community.org/2023/development_irc_meeting_26_april_20... Added to the agenda: https://wiki.koha-community.org/wiki/Development_IRC_meeting_10_May_2023 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150517 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150517&action=edit Bug 30002: Adjust perltidy Remove the following 3 options that are not available in perltidy v20190601 that is the version shipped with bullseye --add-terminal-newline --valign-exclusion-list --extended-continuation-indentation Extend max line length --maximum-line-length=120 Prevent perltidy to format lists (see bug 30002 comment 10) --break-at-old-comma-breakpoints -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #21 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Jonathan Druart from comment #18)

Remove the following 3 options that are not available in perltidy v20190601 that is the version shipped with bullseye --add-terminal-newline --valign-exclusion-list --extended-continuation-indentation

That's something I haven't considered : different versions of perltidy will generate different code. For instance, vertical alignment of qw() after use is on by default since v20220613. Should we force usage of a specific version then ? Maybe add a version check in the QA scripts too. Also, since it's a developer-only tool, do we need to restrict ourselves to an old version ? (P.S. That doesn't change anything, but bullseye's version of perltidy is v20200110, not v20190601) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Julian Maurice from comment #21)

Also, since it's a developer-only tool, do we need to restrict ourselves to an old version ?

We could package the latest version on CPAN and have it in our repo. Mason, would that work for you? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_23_11_candidate --- Comment #25 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Please consider this as a candidate for 23.11 early pushes. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #149027|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150517|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #26 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 152161 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152161&action=edit Bug 30002: Add .perltidyrc The options set in this file try to match the current code style without deviating too much from perltidy's defaults. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #28 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 152163 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152163&action=edit Bug 30002: Adjust perltidy Remove the following 3 options that are not available in perltidy v20190601 that is the version shipped with bullseye --add-terminal-newline --valign-exclusion-list --extended-continuation-indentation Extend max line length --maximum-line-length=120 Prevent perltidy to format lists (see bug 30002 comment 10) --break-at-old-comma-breakpoints Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #31 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #30)

(In reply to Jonathan Druart from comment #24)

...

Suggestion addition for the wiki at https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL1:_Perltidy

""" The code must be tidy using perltidy and the .perltidyrc file presents at the root of the Koha directory. You must run perltidy on any new perl files added. When you add changes to existing files you should consider setting your code editor accordingly. For vim you can add the following line to you `.vimrc`: `vmap <F8> :!perltidy -q<CR>` Then switch to visual move (ctrl+v), select a block and press F8. """

Added to the wiki.

I added the VSCode counterpart. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #33 from David Cook <dcook@prosentient.com.au> --- Does anything need to be done for folk who code in koha-testing-docker using vim? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #35 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to David Cook from comment #33)

Does anything need to be done for folk who code in koha-testing-docker using vim?

read comment 24? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #37 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 152263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152263&action=edit Bug 30002: (QA follow-up) Add .perltidyrc to Makefile.PL mapping Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #39 from David Nind <david@davidnind.com> --- I've updated the Perltidy page on the Wiki with some consistent structure and formatting - feel free to edit to fix up any errors that I may have inadvertently introduced: https://wiki.koha-community.org/wiki/Perltidy Questions: Q1. I'm not sure what the perltidy -pro=.perltidyrc command is supposed to do - it did nothing for me in KTD (pertidy version is 20200110), it just sat there with a flashing cursor. Outside KTD with perltidy installed, it did the same as well. Q2. For usage in Geany, I'm not sure what "Select part of your file or script to perltidy -pro=/home/path/to/.perltidyrc" means either. (Possibly, the answer to these questions are obvious to developers 8-).) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #41 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- It is not very clear what we exactly need to change now (or even why). A simple example. IIRC we needed to do this (no space after if and before parenthesis, but spaces around the condition): if( $a ) { But perltidy changes this to if ($a) { Space after if, and no spaces around one condition. But hey, things change for if( $a && $b ) { This should be now: if ( $a && $b ) { Spaces around multiple conditions. Similar thing for function calls: Instead of test( 1 ) we now need test(1) But test( 1, 2 ) is fine. These are indeed stupid examples, but probably the things that we type most of the time. We should imo not fail patches for occurrences of if( $a ) or test( 1 ), since that was our style until this report.. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #43 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #41)

It is not very clear what we exactly need to change now (or even why). A simple example.

IIRC we needed to do this (no space after if and before parenthesis, but spaces around the condition): if( $a ) { But perltidy changes this to if ($a) { Space after if, and no spaces around one condition.

But hey, things change for if( $a && $b ) { This should be now: if ( $a && $b ) { Spaces around multiple conditions.

Similar thing for function calls: Instead of test( 1 ) we now need test(1) But test( 1, 2 ) is fine.

If these are the default perltidy's rules and there are no consensus in the team, we must pick the default perltidy's behaviour. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #45 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to David Nind from comment #39)

I've updated the Perltidy page on the Wiki with some consistent structure and formatting - feel free to edit to fix up any errors that I may have inadvertently introduced: https://wiki.koha-community.org/wiki/Perltidy

Questions:

Q1. I'm not sure what the perltidy -pro=.perltidyrc command is supposed to do - it did nothing for me in KTD (pertidy version is 20200110), it just sat there with a flashing cursor. Outside KTD with perltidy installed, it did the same as well.

Q2. For usage in Geany, I'm not sure what "Select part of your file or script to perltidy -pro=/home/path/to/.perltidyrc" means either.

(Possibly, the answer to these questions are obvious to developers 8-).)

--pro= takes the path to Koha's perltidyrc file. I have updated the section for Geany again. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for all the hard work! Pushed to 23.05.x for the next release -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #48 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #50 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #49)

(In reply to Mason James from comment #38)

...

...

Thanks, Mason! Yes, please push it!

ok, pushed :)

We have 20230309-1~koha1 but it's not the version install in ktd.

root@kohadevbox:koha$ perltidy --version This is perltidy, v20200110

Ping Mason? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 --- Comment #52 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Katrin and I do not have the same version installed. I pulled this morning, ktd (KOHA_IMAGE=master) 12:18 < cait> perltidy: 12:18 < cait> Installed: 20230309-1~koha3 12:18 < cait> Candidate: 20230309-1~koha3 12:18 < cait> Version table: 12:18 < cait> *** 20230309-1~koha3 500 12:18 < cait> 500 http://debian.koha-community.org/koha-staging dev/main amd64 Packages 12:18 < cait> 100 /var/lib/dpkg/status 12:18 < cait> 20200110-1 500 12:18 < cait> 500 https://ftp.uni-stuttgart.de/debian bullseye/main amd64 Packages 12:19 < Joubu> I don't have 2023 installed 12:19 -!- Irssi: Pasting 5 lines to #koha. Press Ctrl-K if you wish to do this or Ctrl-C to cancel. 12:19 < Joubu> Version table: 12:19 < Joubu> 20230309-1~koha3 500 12:19 < Joubu> 500 http://debian.koha-community.org/koha-staging dev/main amd64 Packages 12:19 < Joubu> *** 20200110-1 500 12:19 < Joubu> 500 http://deb.debian.org/debian bullseye/main amd64 Packages 12:19 < Joubu> 100 /var/lib/dpkg/status Mason, how is that possible? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30002 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.