Hi I agree with both of you. kohaisms are not really interesting. And we need to try and strengthen the Coding Style GuideLines. But then. Who, How, When ? Who ? should it be up to contributors or to QA manager to ensure that coding style GuideLines are abided. Coding Style guidelines could also state a kind of hierarchy in folders/templates design, pod commenting. Posting some hooks and/or some vimrc (for indentation) may help. We could improve one another way to test and develop. I think both should have some responsibility. How ? For already sent patches, new guidelines should be considered as not appliable and it could be up to the QA queue to update the patch and send a followup For new patches, as soon as it is publicised in the wiki, it could be up to the developer to comply with the guidelines. The problem is that when the developer donot have time enough to update the patch and the patch either fixes a bug OR add a much desired feature. Then anyone willing to have that patch could send a sign off and a followup and ask for sign-off. my 2 cts. -- Henri-Damien LAURENT Le 16/09/2011 14:33, Ian Walls a écrit :

Colin,

I made the decision to require parentheses around the variables for the sake of consistency. All the other template in Koha do it that way, right or not. I'm certainly open to it dropping the parentheses; they don't seem to follow standard practice, and while I'm sure no complications are introduced by them, it does add extra keystrokes and one more "little Koha nuance" to any patch submitted.

We have not revisited the Coding Style Guidelines for Koha since switching to Template Toolkit, and I think this should certainly be one of the issues we address when we do. Until then, I recommend we maintain a consistent style in this, even if we're eventually going to drop it.

What does the rest of the community think? Is it worth being picky about such a thing until we can decide for sure one way or the other? Or, more pragmatically, should such a thing impede QA?

Cheers,

-Ian

On Fri, Sep 16, 2011 at 8:13 AM, Colin Campbell <colin.campbell@ptfs-europe.com <mailto:colin.campbell@ptfs-europe.com>> wrote:

Hi I noticed that in QA someone is changing tt constructs from [% IF variable %] to [% IF (variable) %] as style issues

I think this is very bad style for the following reasons: You'll not see it in any of the documentation for tt either the perldoc, on the tt site or in the badger book. (and I've never seen it on any tt using project). It adds nothing (we hope) syntactically. My initial response is that as we are not using normal tt syntax, something "clever" or magic is going on here rather than a usual [% IF %] It detracts from readability. (ok slightly subjective but the environment already makes full use of the top row of the keyboard.) Whats its relation with the legitimate use of brackets e.g. to call vmethods in regexps [% IF variable1( variable2 ) %] The authors didn't require brackets around more complex boolean expressions [% IF variable == 0 %] why bring em back in for simple variables. Could it have unforeseen side effects (don't know but I don't want to spend time researching it) It confuses the reader of the code and the writer of subsequent code - 'should I use () no? when? why? It strikes me as weird (!!)

In short I'm arguing for clarity...

I realise that Chris bracketed things in the great template conversion but I think that was defensive programming when he couldn't rely on variable always being one. And we shouldn't be encouraging or enforcing others to use a peculiar idiolect rather than standard practice.

Colin

-- Ian Walls Lead Development Specialist ByWater Solutions Phone # (888) 900-8944 http://bywatersolutions.com ian.walls@bywatersolutions.com <mailto:ian.walls@bywatersolutions.com> Twitter: @sekjal

On 2011-09-17, at 4:01 AM, Colin Campbell wrote:

On 16/09/11 16:04, LAURENT Henri-Damien wrote:

...

Hi I agree with both of you. kohaisms are not really interesting. And we need to try and strengthen the Coding Style GuideLines. But then. Who, How, When ? Who ? should it be up to contributors or to QA manager to ensure that coding style GuideLines are abided.

Style guidelines should be a) simple and b) mechanizable

Here's the style guide for perl in the Apache::Lucy project

...

All code should follow perlstyle.

Formatting is handled automatically using Perltidy with a perltidyrc file derived from the guidelines laid out in Damian Conway's book 'Perl Best Practices'.

Its nice because it points to an way of automatically maintaining consistency (perltidy) and two sources of recommendations and rationales for those recommendations.

nice spotting Colin :) i agree, and always run perltidy over my code as a habit i just checked and there's no mention yet of default .perltidyrc standard on the 'Koha coding guidelines' wiki page http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML_Templates can we discuss, then add one?!? :p i'm keen on the -pbp 'Perl Best Practices' style, myself what do other folk think? cheers, Mason

On 2011-09-19, at 9:42 AM, Chris Nighswonger wrote:

On Sun, Sep 18, 2011 at 3:43 PM, Mason James <mtj@kohaaloha.com> wrote: nice spotting Colin :)

i agree, and always run perltidy over my code as a habit

i just checked and there's no mention yet of default .perltidyrc standard on the 'Koha coding guidelines' wiki page http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML_Templates

Actually this has been discussed before and there is a perltidyrc available in the main repo:

http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=xt/perltidyrc;h=f7...

yep, i use that xt/perltidyrc example for my own Koha perl code, like a good Koha developer

can we discuss, then add one?!? :p i'm keen on the -pbp 'Perl Best Practices' style, myself

IIRC we ended up agreeing that there was much disagreement in coding style.

My opinion, FWIW, leans toward PBP as well, though.

me too PBP++, if its good enough for Damian Conway - its good enough for us :p a quick google shows that this topic has been discussed a few times over the years... with no firm agreement :/ http://lists.gnu.org/archive/html/koha-devel/2007-02/msg00013.html http://lists.koha-community.org/pipermail/koha-devel/2010-February/033658.ht... http://lists.koha-community.org/pipermail/koha-devel/2003-May/025915.html shall we have another go at an agreement on this Folks? is this a topic for the next IRC meeting? i too, sure would love to get an agreement on a consistent, mechanised formatting style for perl code in Koha fyi: Koha's current .perltidyrc was never agreed upon, it was simply submitted and accepted is there anyone that *doesnt* want -pbp 'Perl Best Practices' style formatting as the default?? cheers, Mason -- KohaAloha, NZ

On 2011-09-19, at 4:32 PM, Robin Sheat wrote:

Op maandag 19-09-2011 om 14:06 uur [tijdzone +1200], schreef Mason James:

...

is there anyone that *doesnt* want -pbp 'Perl Best Practices' style formatting as the default??

That was a silly question ;)

Maybe better to just mandate it and deal with issues as they come up? I don't agree with 100% of PBP,

yes, i dont like all of PBP style too...

but I'd be happy to go with it in order to make the code nice and consistent.

yep, but i'm very happy to go with PBP style over another formatting style so it sounds like a tentative vote for PBP style formatting from Robin, Chris.N and Mason... anyone else have a better idea?

Op dinsdag 20 september 2011 04:18:02 schreef MJ Ray:

Probably. I dislike delegating this decision to an O'Reilly book

I prefer to delegate to Conway than some other arbitrary standard. The publisher is irrelevant.

and it would change some historic practices, so probably reformat a lot of code. I think -pbp is equivalent to:

-l=78 -i=4 -ci=4 -st -se -vt=2 -cti=0 -pt=1 -bt=1 -sbt=1 -bbt=1 -nsfs -nolq -wbb="% + - * / x != == >= <= =~ !~ < > | & = **= += *= &= <<= &&= -= /= |= >>= ||= //= .= %= ^= x="

but the perltidy man page doesn't say why those are best practices!

Because you could fill an entire book with that, and that's too big for a man page.

They look as arbitrary as the GNU Coding Standards to me and rather more invasive, but would anyone who has the book like to enlighten us?

Each one is justified fairly well. For the one or two I disagree with (that I've found so far) I can see the rationale, I'm really just used to doing it another way. I'm not, however, going to write out everything, because that's a lot of effort for little gain. If you have a particular question, I'm sure I can answer it.

The Koha current perltidyrc conflicts with -pbp in that it has -l=178 -ci=2 -bbt=0 -sfs -olq which disagree with the above.

Of these, I think that each is "wrong", not because it's different to pbp, but because it hinders read/writeability. And -l=178? You must be kidding. That's terrible in-and-of itself. If I were to disregard something because of where it came from, this would be enough to make me disregard the rest of your suggestions ;)

suggested -bar -ce

-bar isn't addressed in pbp (iirc), however -ce is in direct contradiction to it (this is something I'm not in total agreement with, but I'm willing to overlook it happily enough. I think my bias here comes from several years doing Java.)

which I don't think affects -pbp either way, -vt=2 which agrees, -pt=2 which disagrees and -en=4 which doesn't even seem to exist!

-pt=1 (the default) is another where my usual way of writing differs, but I do find myself doing that in the more confusing constructs, which leads me to think that it's actually not such a bad idea. As for -en=4, that's almost as terrible an idea as -l=178. I'll pretend you didn't say it.

http://wiki.koha-community.org/wiki/Coding_Guidelines#To_be_decided also specifies a capitalisation approach, -> for hashrefs and use of ODLIS terms which I don't think perltidy can enforce.

Again, I prefer the PBP method for naming. It's what (almost) every other Perl module does, and for fairly good reason. (Nutshell version: sub_name, $identifier_name, %title_of{$book_id}, @flowers, Package::Name, $CONSTANT) As for -> for hash/arrayrefs, definitely.

So, why would it be worth changing the line length limits, indentation, outdentation, brace-tightness, and semicolon spacing to pbp?

Because: a) it's a standard that many perl editors understand, b) many perl programmers understand, c) it's less arbitrary than any other standard (as the reasoning is quite meticulously backed up), d) it's a pretty good standard, e) if you don't pick a standard then the code will continue to be quite ugly, and this is one we have now without years of quibbling over where braces should go, f) many people have access to the book and so can read the justifications if they wish. There's probably more reasons I haven't thought of. -- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204

Robin Sheat wrote:

Op dinsdag 20 september 2011 04:18:02 schreef MJ Ray:

...

Probably. I dislike delegating this decision to an O'Reilly book

I prefer to delegate to Conway than some other arbitrary standard. The publisher is irrelevant.

I dislike that publisher in particular, but also I'm a democrat, so in general I'm against appointing Conway as a pope. This discussion should be about whether that book is correct, not who is the best person to follow.

...

and it would change some historic practices, so probably reformat a lot of code. I think -pbp is equivalent to:

-l=78 -i=4 -ci=4 -st -se -vt=2 -cti=0 -pt=1 -bt=1 -sbt=1 -bbt=1 -nsfs -nolq -wbb="% + - * / x != == >= <= =~ !~ < > | & = **= += *= &= <<= &&= -= /= |= >>= ||= //= .= %= ^= x="

but the perltidy man page doesn't say why those are best practices!

Because you could fill an entire book with that, and that's too big for a man page.

...

They look as arbitrary as the GNU Coding Standards to me and rather more invasive, but would anyone who has the book like to enlighten us?

Each one is justified fairly well. For the one or two I disagree with (that I've found so far) I can see the rationale, I'm really just used to doing it another way. I'm not, however, going to write out everything, because that's a lot of effort for little gain. If you have a particular question, I'm sure I can answer it.

So is there no concise explanation of any of them? If so, this must be quite tenuous. If someone could justify the bits where it's in conflict with Koha perltidyrc/past practice, that would suffice. [...]

...

The Koha current perltidyrc conflicts with -pbp in that it has -l=178 -ci=2 -bbt=0 -sfs -olq which disagree with the above.

Of these, I think that each is "wrong", not because it's different to pbp, but because it hinders read/writeability. And -l=178? You must be kidding. That's terrible in-and-of itself. If I were to disregard something because of where it came from, this would be enough to make me disregard the rest of your suggestions ;)

Oh niiiice, shoot the messenger because you can't justify it? When I made parameter suggestions, I think I was trying to codify current practice where it existed, as a starting point, but my memory is poor and I don't have any relevant notes to hand. Personally, I don't care what the maximum line length is, but I seem to recall that some at liblime and biblibre used pretty wide windows for coding, so there are some long lines in the code and I had patches rejected when I split them. [...]

...

So, why would it be worth changing the line length limits, indentation, outdentation, brace-tightness, and semicolon spacing to pbp?

Because: a) it's a standard that many perl editors understand, b) many perl programmers understand, c) it's less arbitrary than any other standard (as the reasoning is quite meticulously backed up), d) it's a pretty good standard, e) if you don't pick a standard then the code will continue to be quite ugly, and this is one we have now without years of quibbling over where braces should go, f) many people have access to the book and so can read the justifications if they wish.

But, a and b) they understand others too (GCS being the obvious alternative which would help open Koha up to non-perl and multi-language programmers); c and d) no evidence of that has been shown; e) is irrelevant because I'm not arguing against setting any standard; f) seems like proof by appeal to authority, also creating a divide between those who buy the ORA texts and those who do not. (The book is not in LibrariesWest and I'm not inclined to suggest my taxes are spent on it.) So at the moment, I think GCS with a few relaxations to avoid changing old Koha code seems like the most logical standard. Regards, -- MJ Ray (slef), member of www.software.coop, a for-more-than-profit co-op. Webmaster, Debian Developer, Past Koha RM, statistician, former lecturer. In My Opinion Only: see http://mjr.towers.org.uk/email.html Available for hire for various work through http://www.software.coop/

Colin Campbell wrote:

On 22/09/11 15:24, MJ Ray wrote:

...

I dislike that publisher in particular, but also I'm a democrat, so in general I'm against appointing Conway as a pope. This discussion should be about whether that book is correct, not who is the best person to follow.

The rationale for most of the formatting issues in pbp is that code should be consistent and that unless there is a good reason not to we should adopt the perl style guide ('perldoc perlstyle'). So most of the recommendations don't come from Damien or his evil publishers but have been distributed with perl for ages.

Past practices (the current perltidyrc) and making it easy for a broader audience than just perl hackers (-gcs) both seem like good reasons not to do it, but pbp is not the same as perlstyle (which is the perltidy defaults). [line length of 178]

...

Personally, I don't care what the maximum line length is, but I seem to recall that some at liblime and biblibre used pretty wide windows for coding, so there are some long lines in the code and I had patches rejected when I split them. I agree wholeheartedly with Robin's point, although you might have lots of screen real estate its surprising how many tools for looking at code work better with a shorter line length and the extra real estate is probably more useful for running more windows. Too often the real import of a line might just disappear of the right edge... Plus its a handy reminder that your code might be becoming far too deeply nested so I think most coding standards irrespective of language gravitate towards an 80 col standard.

Those are whole debates in themselves, like whether short-line-only tools are broken and whether it's healthy to restructure code mainly to reduce nesting, but I suggested 178 because coders were using it. If I've got these command right, they still seem to be: coop@koha:~/koha/unstable/src$ find . -type f -name '*.pl' | wc -l 966 coop@koha:~/koha/unstable/src$ grep -Erl '^.{79,999}' $(find . -type f -name '*.pl') | wc -l 937 coop@koha:~/koha/unstable/src$ grep -Erl '^.{79,178}' $(find . -type f -name '*.pl') | wc -l 937 So I think about 97% of .pl files would be changed by just that one setting change. I'd love to hear if many use 178-columns now, especially at biblibre and liblime because I think it came from one of them.

...

But, a and b) they understand others too (GCS being the obvious alternative which would help open Koha up to non-perl and multi-language programmers); As far as GCS is concerned perltidy's man page points out that "-gnu approximates to the Gnu Coding Standards (which do not apply to perl) as they are sometimes implemented" Not really a resounding vote in favour.

Well, they're written only for C, but it's a pretty trivial translation to most languages and they're less Perl-specific.

...

[...] proof by appeal to authority, also creating a divide between those who buy the ORA texts and those who do not. It's not about the book, almost all the formatting recommendations come from the documentation which has always come with perl. It also is a style which does not jar with the bulk of good code you'll find in CPAN and elsewhere.

It does jar with the bulk of Koha, much of which is also good code in other ways, isn't it? The Koha community never followed the perlstyle recommendations, they're not new and I'd like to think that our predecessor coders had their reasons for what they did (I don't know if Chris C knows why but I think it was on Josh's watch), so I feel there needs to be a decent reason for such a wide-reaching change.

For rationales of some of the recommendations and other good suggestions folk might want to look at http://onyxneon.com/books/modern_perl/ which is free in its electronic form.

I'll take a look at that. It may take some time. PDFs aren't fun.

One of the points made there is that following community norms unless theres a real reason to differ is good because the community builds tools which leverage those norms. perltidy is an example, I think if you compare the defaults with the pbp recommendations they are identical for most settings

perltidy's defaults are perlstyle, not PBP. Following community norms is a great idea! *Which* community norms? We've at least four norms to choose between here: Koha, GNU, Perlstyle and PBP. PBP seems the least accessibly documented of them all and I don't think any formatting can be brilliant enought to outweigh that barrier - and I don't think a restricted book like PBP is really a *community* norm either. Regards, -- MJ Ray (slef), member of www.software.coop, a for-more-than-profit co-op. http://koha-community.org supporter, web and LMS developer, statistician. In My Opinion Only: see http://mjr.towers.org.uk/email.html Available for hire for Koha work http://www.software.coop/products/koha

On 23/09/11 10:07, Alex Arnaud wrote:

I've not had time to read all in this topic, then sorry if you already told about my problem.

I got an error when going to the biblio_framework page in koha. "unexpected token (marc) [% grille marc %] at /home/users/koha/versions/community/C4/Templates.pm line 119."

[% grille marc %] is simply the french tranlation of [% frameworkcode %] in the english template. I've modified the EN Template to add parenthesis around 'framwork' and it has not been translated to 'grille marc'. good.

Then i think it would be better to do the same for each variables in each template but, strangely, in the same template, few lines later another [% frameworkcode %] has not been translated to [% grille marc %]. I don't know why ...

Its not a case of the bug reported as bug 6458? C. -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 845 557 5634 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com

Le samedi 22 octobre 2011 à 18:09 +0100, MJ Ray a écrit :

I'm not sure this ever came back to this list, but the meeting http://librarypolice.com/koha-meetings/2011/koha.2011-10-05-10.00.log.html#l... agreed to use perlstyle as the recommended perltidy config from now on.

ok.... But is there a perltidy option to be sure to abide by this "perlstyle" Is "perl-style" the same as pbp ? Is that the perltidyrc in our xt in src ?

I've updated http://wiki.koha-community.org/wiki/Coding_Guidelines#Perl but, returning to the original topic of this thread, http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML_Templates could still do with a bit of expansion and linking to some tt resources.

Hope that helps,

-- Henri-Damien LAURENT BibLibre

Le 24/10/2011 19:18, MJ Ray a écrit :

...

ok.... But is there a perltidy option to be sure to abide by this "perlstyle"

As I understand it, perltidy defaults to perlstyle, so just run it with no options or rc file.

Reminder: we've said during the last IRC monthly meeting that: * new code should be perlstyled * perltidying must not be done in a file mixed with other things * we don't perltidy everything for now, as it would result in git blame returning the perltidy submitter as author of everything. Side question : if someone has an idea to perltidy everything and not loose the blame, he's welcomed. I've checked git blame --help, and it seems we have a friend here : -w Ignore whitespace when comparing the parent’s version and the child’s to find where the lines came from. except it's just for whitespace, and perltidy often update more than space... QUESTION/DISCUSSION = I feel that just tidying new code will result in a mixing of style that will be uncomfortable. OTOH, tidying everything would resuld in git blame being unsusable. It mean both situations are unsatisfying. Does anyone have an idea to improve the situation ? -- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08

Paul Poulain schreef op di 25-10-2011 om 10:37 [+0200]:

...

* perltidying must not be done in a file mixed with other things

What does that mean? It means the new code must be perltidyied, but old code must not be

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Le 26/10/2011 00:36, Robin Sheat a écrit : perltidyed at the same time. ie: your patches must do one thing only This question is worrying me a little (just a little, really) I must say. we will speak of during HackFest I think. - -- Paul POULAIN http://www.biblibre.com Expert en Logiciels Libres pour l'info-doc Tel : (33) 4 91 81 35 08 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJOp9NvAAoJEK81SonuhyGoqaYIAKoM+mFRJqAG4Yv2c5UlXoLe GGPZyAaIdYhUkjtm14Af/saJ9sZq8daJNoG3Gbi3umdIEULcZfIUKVRBfjlE5NN3 DEfuqn8pQqhHczE34ClsxSikZlVy7/KaeiJDQsLXd12lpezD5xGCZXya0k8LIpIj Vuh769NQTrv3MOFtsCj6NUpxDC5pdGP7rPayU529Fo4+k6NxyY4CoSxpjDTATBOO x3HXuA65ddpIYS8HxxGai4sNmsQfAlY3CHT0FHdGCmhYSXG0FidumN3dhn1MJs54 0DCJlPHr4reNmN/GtAKPsNnEoRp5lTK0TrvURI6VRjnek3RWzTeVilpTrjrCL1A= =acr2 -----END PGP SIGNATURE-----

Paul wrote:

At 03:24 PM 9/22/2011 +0100, MJ Ray wrote: [snip]

...

So at the moment, I think GCS with a few relaxations to avoid changing old Koha code seems like the most logical standard.

Could you please point me to a write-up about "GCS" -- my googling abilities are coming up short. Thanks,

I should have expanded the abbreviation on the first use in the post, but look back up this discussion and you'll see it's GNU Coding Standards, which are at http://www.gnu.org/prep/standards/ and the most relevant section probably starts at http://www.gnu.org/prep/standards/html_node/Formatting.html Do you like Google or not? If you like it, I seem to recall they don't like it as a verb. If not, use a better search engine. ;-) Hope that informs, -- MJ Ray (slef), member of www.software.coop, a for-more-than-profit co-op. Webmaster, Debian Developer, Past Koha RM, statistician, former lecturer. In My Opinion Only: see http://mjr.towers.org.uk/email.html Available for hire for various work through http://www.software.coop/