CodingStandardsDiscussions

Regarding Coding Conventions

Are we really fixated on the camel case style for function and variable names (within code)? Most Perl projects and development houses I'm familiar with use underscores_to_separate_words_in_variables, It is common Perl style.

-- SeanMcCarthy

Not fixated, no. But what value comes from changing? I see absolutely no value in mixing the styles (quite the reverse).

-- CrawfordCurrie - 17 Nov 2008 - 08:15

Good point, mixing would be completely awful. I was thinking of any new modules/plugins/contribs, anything that is a clean start could be done the proper way

-- SeanMcCarthy - 17 Nov 2008 - 12:19

surely, new modules should also be consistent with existing modules.

-- SvenDowideit - 21 Nov 2008 - 12:17

It's probably not worth too much argument, I guess my point is CamelCase is a style more synonymous with C++/Java/C#. Perl programmers jumping on board are more likely to be familiar with the usual Perl style. IMHO the Perl style is more readable, and it would be nice to look like a Perl project. /pendantry

-- SeanMcCarthy - 24 Nov 2008 - 01:42

In 2012 we implemented a checkin trigger to verify perltidiness of all checked-in code. To save time and effort the trigger was applied to all code, including extensions. This raised the hackles of one coder. The email discussion follows:

> Now it simply refuses to accept the checkin.
Yes, that's its only feature.

> > Who said we need this?
Many users, including some of the most active coders.

> > Please remove this new check from svn.foswiki.org immediately.
No.

> > > Just to make clear:
> > >
> > >* Enforcing certain code formatting that way is OKAY for foswiki core plus plugins part of it.
> > >* Enforcing the same code formatting rules for the rest of the code base is NOT OKAY.
> > >Thats how we dealt with it for ages basically.

> > There has been no discussion what so ever to have this feature, nor
> > anybody that I know that asked for it.
Maybe nobody that you know, but I know several persons who asked for it,and I've raised the point several times over the years about it.

> > And at least I would never have agreed on it.
Feel free to use another service to host your code then. Or tell me howto filter out your beloved plugins. But don't tell me I should do onething, and then complain because the list you gave me doesn't containall your plugins.

In order to be a bit more constructive, we might submit this to a vote.I never really understood what you have against the default perltidy,and I've offered you to provide an alternate perltidyrc on a per pluginbasis (something like if there is a perltidyrc in the plugin'sdirectory, use it).

(I have deliberately not included names above, as this discussion was becoming unpleasantly heated).

My response is:

Let's not make a mountain out of a molehill here.

• CodingStandards makes no distinction between core code and any other code.
• I'm no fan of perltidy, but I recognise the need to have standards to make it easy for people to read code, and respecttheir desire to ready my code. So I am happy for this trigger to remain in place for all code.
• The reason for imposing presentation standards is so that anyone can read code without having to reset theirbrain for every new presentation style (read, not write; this has nothing to do with who can change code, andeverything to do with making it accessible). Code is documentation.
• Everyone should already be pertidying code before checkin. We have had perltidy with no parameters as ade facto standard for a long time now.
• Any external modules (e.g. CPAN) should not be tidied, however, as no Foswiki person has control over their sources.
• Standards exist for the benefit of everyone. Consider them in a positive light.
• This topic has has been open as a vehicle for discussion for many years.

-- CrawfordCurrie - 07 May 2012

I am with Michael here.

Who are to tell someone who has contributed a plugin that they MUST comply with a specific Perl Tidy set of rules?

We should be thankful that people contribute their plugins instead of just keeping then private.

Such an enforced rule will create more bad than good.

Another thing is, I am against it even on core and default plugins.

Perl Tidy is good as principle. But enforcing it is too strict.

Sometimes, making one or two long lines or making the syntax slightly different makes code more readable for humans. Enforcing it in SVN checkin means that you do not have this freedom.

I often shake my head on what Perl Tidy destroys of readability

We should encourage these rules. Not enforce them braindead. And for sure not on plugins no matter what the policy is.

-- KennethLavrsen - 07 May 2012

Copying what I wrote on Tasks.Item11808, as it answers also one of Kennethconcern. The idea is not to have to comply with a specific set of rules WEchoose, it's to be coherent within their own plugin:

I've asked Micha about what was best for him to implement so he could workproperly, and we could have a global perltidy checker. He told me to respectExtensions.CoordinateWithAuthor, which I did.

Now I offer again what I offered before:

Would it be ok to implement a perltidyrc, which when set at the top level of aplugin, would be used to test for perltidyness? Also accepting an OFF flag?

So, to give some examples, you commit a file calledStringifierContrib/perltidyrc, which contains what you posted above, and thepre-commit.pl hook would use it for all code within StringifierContrib.

Or you commit a CaptchaContrib /perltidyrc which contains just OFF, and noperltidy checking will be done for all code within CaptchaContrib.

This would prevail over CoordinateWithAuthor, which we would still keep in place.

Problem comes for plugin which do not have a topic on foswiki.org, likeBibtexPlugin, or CaptchaPlugin. CoordinateWithAuthor cannot apply, so we have toresort to the perltidyrc.

-- OlivierRaginel - 07 May 2012

If it is Opt in instead of Opt out so the plugin author can tun it on if he so want to (EmptyPlugin with on) then it is something that is not patronising but instead a service. I still find the enforcement bad because Perltidy is a software and not a human and I still claim that in some instances it ruins readability of a few codelines.

Can you disable the check for a few lines when readability is a concern?

-- KennethLavrsen - 07 May 2012

I don't get it.

Some developers that spend alot of time merging code would like the things they work on regularly to have identical style, so that commits and merges are only relevant code.

Some developers don't want it.

So what we want, is an opt in marker in each file.

ie:

#COMPLIES with Foswiki style

we're talking about a value add service, not a policeman.

(and if create-extension && build.pl can have a default that I can set to yes for all contribs i create, i'd be happy - I know i'm forgetful, but merge consistency makes my life easier)

-- SvenDowideit - 07 May 2012

I actually do like perltidy, it pains me greatly when I use a language that doesn't have an equivalent. So I think I'm one of the developers who wanted a perltidy-on-commit feature. But I don't recall wanting it for every extension by default.

So I agree that tidying by default is jarring to our developer culture: inclusive, free, do-ocracy.

Speaking of do-ocracy, I understand that Crawford wrote what had time to write, and I appreciate that. But we need to consider the impact on our casual contributors. We don't have enough contributors. Contributing should be easy. I don't think this initial attempt will help us recruit or retain contributors.

Aside: I had a recent adventure with CPAN:MooseX::Method::Signatures - a terrific syntax for powerful method signatures in the Moose OO system. But perltidy completely explodes on this code.

IRC #perl punks chastised me into oblivion for even daring to be so incompetent as to want perltidy at all; then after I gently hammered the point home that this is an utterly ridiculous position to take, it was implied that I'm still an idiot for not 'just fixing' perltidy to cope with CPAN:Method::Signatures syntax.

Anyway, I did look into fixing perltidy.

Surprisingly, it doesn't:

• Use PPI. It pre-dates it by many years. A perl parser, written in perl! Slightly terrifying.
• No in-line directives to control tidy behaviour on a per-file basis (so I'm not sure SvenDowideit 's suggestion can work very easily).
• .perltidyrc files are only considered if they exist in the same directory as the tidy'd file. It won't walk the parent directories like with .gitignore.

-- PaulHarvey - 08 May 2012 - 03:38

er - of course it works - we use a script that calls perltidy. it gets a list of files to tidy by grepping for COMPLIES with foswiki style

-- SvenDowideit - 08 May 2012

Ah, of course. Don't mind me then

-- PaulHarvey - 08 May 2012 - 04:36

I would prefer an opt-out, rather than an opt-in. Sheer laziness on my part - I want all my extensions to be opted-in, but don't want to have to go through instrumenting them all, and it's slightly easier to write the checker script that way.

I suspect we could all be happy with a few simple rules:

1. All code under core must be perltidy (that means tools, bin, lib are all affected) with the single exception of core/lib/CPAN
2. All shipped extensions must be treated the same as core and must be perltidy
3. Extensions can opt-out of tidiness by adding # no perltidyto the .pm
   • I would rather have extensions out-out on an extension basis, and not a file-by-file basis, as I explained above, because I find it much more convenient (and flexible, as they could request another perltidy configuration instead of simply opting out) -- OlivierRaginel - 08 May 2012
   • I like this - a single file e.g. TIDIED in the source tree above the source module would suffice for an opt-in - C.
4. The concept of "tidiness" will be extended to source Javascript and CSS if and when we can find suitable formatters

If the weight of opinion favours an opt-in, then rule 3 is simply "Extensions can opt-in to tidiness by adding # perltidy to the .pm

On the assumption that core and shipped extensions will be tidied, what is your preference for handing other (non-core) extensions?

-- CrawfordCurrie - 08 May 2012-- OlivierRaginel - 08 May 2012

Let's first find out which alternatives we have in principle before going into the details of one of those options. We have:

• (a) leave things as they were: don't check code quality at check-in time; don't block contributions by no means; formatting and perl critiques are performed by the developers themselves
• (b) enforce code quality measures globally: block any check-in to foswiki's central software repository when quality measures aren't met; quality measures are discussed, maintained and reviewed on foswiki.org.
• (c) like (b) but only for foswiki's core code and associated plugins, minus any 3rd party modules no matter where they are located (not only under lib/CPAN).

Note that I find it mandatory to remove the current check-in handler that have been installed previously and are now blocking business, until we have made a well founded decision.

-- MichaelDaum - 08 May 2012

I think we agreed that (b) was a bad idea, and we need to find some rule to define the delta. It think your phrasing is misleading, as there is no opt-in / opt-out in neither (b) nor (c). (a), I hope, should be ruled out because it impacts people who are doing a very important but boring job: merging things across. I remember Kenneth grumbling against people who perltidy'ed, and now George doing the same about people who don't. It should be all or nothing, but you can't prevent people for perltidy'ing (I'm talking about core code).

So I agree, the table should reflect (a) and (c), and (c) being either opt-in or opt-out (we need some way to define what extensions should be perltidyed or not).

I've amended the table to add a "Do nothing" table, which is your (a) solution.

-- OlivierRaginel - 08 May 2012

"Do nothing" is not an acceptable option IMHO. The point of this check was to help developers to achieve quality standards that they should be striving for. Simply giving up and allowing any old crap into the repository is not a good idea.

-- CrawfordCurrie - 08 May 2012

Nor am I leaning towards a "do nothing" choice. That's misleading and not what I've tried to outline. Both (a) and (b) are the two general extremes. (c) is what makes most sense as far as I can see and from where we stand with the current way of handling the affairs.It basically is easing and formalizing the way we would like to work but did so more or less manually.

That's where I see we all agree: more convenience - less hassle - better code quality.

If we agreed on going via (c) then I fail to see the point of opt-in vs opt-out. Can somebody explain the differences and value of both, given that it shall cover foswiki core only anyway?

-- MichaelDaum - 08 May 2012

The point of opt-in/opt-out is that most professional developers will want to use the tools for checking when the commit, and that obviously includes extension authors.

I checked in an interim solution that uses the existence of a file TIDY in the directory tree above the source file to determine whether to enforce tidiness or not. I added TIDY files at the root of the core and core plugins, to support the standards in those modules.

-- CrawfordCurrie - 08 May 2012

I will tune this a bit so it checks the existence AND content of the TIDY file, and we can agree to something along those lines:

• File does not exist -- depends of the vote. Either tidy everything (opt-out option), or tidy nothing (opt-in option)
• File exists and is empty -- tidy everything
• File exists and contains only the word OFF -- tidy nothing
• File exists and contains (among other things) a line: perl OFF -- perltidy nothing
• File exists and contains (among other things) a line: perl -q -l=0 -i=2 -pt=2 -nsfs -ce -novalign -- perltidy using those options (these are Micha's favorite)

We could add some exception to prevent some files from being touched, but I leave that for later.

As Micha and Crawford seemed to agree it's a good idea, I'll start implementing something along those lines.

This does not really prevent people from voting for opt-in or opt-out above, as we still have to decide about the default behavior

-- OlivierRaginel - 08 May 2012

I am going to vote "opt out" as well. However Micha makes a good point above, that we need to exclude 3rd party libraries, not just CPAN,especially if we start to tidy Javascript and CSS. Do we want to be tidying the moxicode distribution of TMCE for example?Or the code we get from JQuery or other external sources?So inclusion of a TIDY file at the root of an extension is probably not granular enough.For TMCE we would want all Foswiki authored code to be processed, but not the external libraries.

I was going to suggest a tidy marker in the MANIFEST, but that won't work in the cases where we ship a compressed file in the manifest, but would need to tidy or not tidy the uncompressed version.

Oh...and is perlcritic a consideration still as well?

-- GeorgeClark - 08 May 2012

Yes, perlcritic is a consideration as well. The way Crawford implemented it, it's hierarchical. Meaning you can do that:

• JQueryPlugin /TIDY => empty so it tidies everything
• JQueryPlugin /pub/System/JQueryPlugin/TIDY => OFF, so nothing under this directory gets tidied.

I'm testing my code which does that, and it should just work

-- OlivierRaginel - 08 May 2012

Just to clarify my views. I prefer opt-in at least in the first place but can live with having at opt possibility

I intend to use it myself. I am OK with perltidy by principle

I am against enforcing it because there is sometimes code where a few lines become unreadable if altered.

It would be nice if perltidy allows to disable the check in a critical section so that maybe 1000 lines are tidied but the 3 critical lines are not. Maybe there is such feature in tidy? I do not expect us to make it. Just allow the use if it is there.

The grumpling I once did was related to people tidying large untidied files AND making a code change in same checkin. That made it very difficult to diff the changes when searching for when bugs occured and when merging. Once tidy is used it is not a problem to run perltidy when you have coded before checkin. What I asked people to do back then was to check in the tidy in a separate checkin.

The reason why some plugin authors will not want us to tidy their plugins is because they may have internal branches they are testing or variations that are local hacks. If we tidy their plugins on subversion, we make it difficult for them to update their code on subversion because their code can be altered significantly if they never ran tidy or use their own coding dialekt. If a plugin is an ask first plugin and in practical only updated by then I do not see the issue in letting them be untidy. That is why opt-in the more respectful.

It is with the core and the default plugins and the popular Feel Free plugins the project wins by checking for tidy code. It is also only core and defaults that exists in more branches. The private plugins only exists in trunk so no problem with merging from branch to branch.

Could the commit script warn instead of bloking checkin for none opted-in code? Then you could tidy up if you forgot and recommit?

-- KennethLavrsen - 08 May 2012

>> It would be nice if perltidy allows to disable the check in a critical section

Yes, and there is. It uses "comment markers" and is in the perltidy documentation. We need to add it to the dev. docs.

#<<<  do not let perltidy touch this
    my @list = (1,
                1, 1,
                1, 2, 1,
                1, 3, 3, 1,
                1, 4, 6, 4, 1,);
 #>>>

-- GeorgeClark - 09 May 2012

Cool! Thanks George, I didn't know about that (despite having read the perltidy documentation at least 5 times)

On a purely mechanical point, we do need to document this quite carefully for developers.

Formatting Code

It important when working with other people to make sure source code is consistently formatted. To help with this, the source code repository automatically checks that files have been consistently formatted before allowing them to be checked in. Any code belonging to the Foswiki core, or any of the extensions shipped by default with the core, is required to be correctly formatted or the checkin will be rejected. Other extensions may be 'flagged' as requiring checking by placing a file called TIDY in the directory tree containing the sources. Similar to Apache's .htaccess, the existence of this file affects all sources in the file tree in and below the directory where it was found.

• TIDY does not exist -- check nothing (code can be formatted however you want)
• TIDY exists and is empty -- check everything - all source files must be formatted according to the standard
• TIDY exists and contains only the word OFF or contains (possibly among other things) a line: <language> OFF -- disables checking <language> files for this subtree e.g. perl OFF
• TIDY exists and contains (possibly among other things) a line such as: perl -q -l=0 -i=2 -pt=2 -nsfs -ce -novalign -- check using these formatting options instead of the standard - not recommended (please use the defaults)

If you use the BuildContrib (highly recommended) then you can use perl build.pl tidy to run all available formatters on your code.

Formatting perl code

The Foswiki standard for formatting perl source files is to run the standard perltidy command without any formatting options

perltidy -b <filename>.pm

Note that individual perl source files can contain special beginning and ending comment markers around code to be passed to the output without formatting. The beginning marker is #<<< and the ending marker is #>>>

Formatting Javascript Code

At this point in time Javascript sources are not checked. However in the future we are likely to use js-beautify for this (the engine behind http://jsbeautifier.org)

Formatting CSS

At this point in time CSS sources are not checked.

-- CrawfordCurrie - 09 May 2012

I'm torn on this. What opt-in vs opt-out really is, is a declaration that perltidy becomes a dependency for Foswiki development.

The engineer in me is cool with that, but the fostering-contributions-from-the-wider-community part of me thinks this might result in some random plugins becoming even less likely to end up in svn.

And the thought of an opt-out dependency on js-beautify which doesn't seem to be widely packaged, sounds obnoxious.

I've tried to add my vote below, but my conclusions change every 30 minutes. So I think I'll refrain from this one.

-- PaulHarvey - 11 May 2012 - 00:03

Shock! Horror! Probe! After listening to the discussion. careful thought and consideration of the mechanism we have for opt-in, I'm changing my vote. The reasons for this change of heart?

• opt-in is so easy that it's no strain on the brain, and should be the default for any new extension.
• opt-out presents too much of a barrier to contribution.
• use of opt-in is a useful measure of code quality (along with unit test coverage, and perlcritic etc) but many people just don't believe in that approach, preferring personal and absolute control (i.e. benevolent dictator).
  • I still think those measures - opt-in, perlcritic etc - should be used in building a rating for extensions, but we should not try to force quality any other way.
  • Even those extensions that are not opted in should receive feedback during checkin - "perlcritic had this to say about your code"
    • If anybody finds a way to do that in Subversion, I'd love to implement it. But from what I tried, it seems it only displays anything if it rejects the commit (when I disabled perltidy for Micha, I replaced the fail with a warn, but I haven't seen any warning, and I doubt Micha saw anything either) -- OlivierRaginel 12 May 2012

This doesn't change my view that core and core extensions are automatically opted-in. Also all extensions I work on will be opted in.

Paul, I added your abstention below.

-- CrawfordCurrie - 11 May 2012

At this point I'm now totally confused. What are we opting in or out of. By op-in, are we saying that we will not tidy unless the developer manually creates the TIDY file? Or when running build-contrib, they answer a question - do you want to tidy, with a default of not tidying, and build-contrib creates the files? Does "opt-in" say that we remove all of the TIDY files that have been checked in and let the individual developers create them if desired. Or am I missing something.

Does the following describe what we are voting on?

Opt-in

Create TIDY files for only the extensions you are maintaining? Up to each developer to create when desired. If developer does nothing, nothing gets tidied.

Opt-out

Create TIDY files for every extension. Up to developer to delete or modify as desired.

• Not really. What we vote here is what will the program do then there is no TIDY file: skip or use default perltidyrc? I think what we have now is the best and we should leave it like that. -- OlivierRaginel 12 May 2012

For core and core extensions, do we even have an opt-out option? (I think not. ... core extensions are always tidied, there is no Opt(ion) )

• No, core and core extensions have no way out, but they're treated as any other extension. People will just get shout at if they remove the TIDY files -- OlivierRaginel 12 May 2012

And one more concern. Rather than using filename TIDY to contain settings, should we use .perltidyrc? I'm concerned that if I'm maintaining someone else's extension, I need to remember to examine and apply the settings in the TIDY file before commit. Otherwise I'll be incorrectly tidying on occasion. Or needing to remember which directory I'm in, and using -pro=TIDY I suspect that settings in the TIDY file may be a recipe for chaos. Also, I'm usually testing/developing in a symlinked checkout, and will do edit/tidy cycles from lib/Foswik directory. After this I can't do that any more. To tidy correctly, need to cd to the actual extension repo directory before doing the tidy. Or cd to the Extension (or core) directory and running the build tidy tool.

• The TIDY file instead of .perltidyrc is for one reason: Crawford's aim not to limit at perltidy. Then we could use JS beautifier for javascript code, something for CSS, and maybe even some perlcritic, and some other fancy checker. And all would be controlled by the TIDY file, as explained above (eg: perl ON, JS OFF, CSS ON). -- OlivierRaginel 12 May 2012

By trying to accommodate objections from one, I suspect we are creating a quagmire.

-- Confused - 11 May 2012

Why opt in instead of opt out? Let us take that that first.

• Respect - do not patronize other people. If someone contributes an extension be thankful. As Crawford and actually also Paul says. We will lose future extensions if we force this through. Is it really worth it?
• It is no issue to opt-in. It is a poor excuse is someone says it is too difficult to opt in the easy way it is done now
• We should not care. If someone check in an extension and refuse to tidy it and insist on being the one and only that maintain the code, why should we care then?

To clarify what we try to decide here

• There is concensus that core code and default apps are tidy. No opt in or out needed.
• Default plugins are tidy. No opting out.
• New plugins created based on EmptyPlugin are by default tidy. So you can say that an author needs to opt out in reality if he starts from scratch
• Existing plugins are either opt-in or opt-out. And this is where I say that forcing people to opt out on their existing plugins is pretty arrogant and adds little value

Most plugins developers are the regular developers and most have already expressed that they are OK with tidy abd will opt in.

I will personally opt-in the plugins I am owner of as the little concern I had with tidy is resolved now I know I can disable the check in a small section if I desire. And I am OK if someone else already now adds the TIDY file to my plugins.

-- KennethLavrsen - 12 May 2012

I mis-understood the part that core extensions are automatically opted-in. So I am changing my vote to opt-in.

-- ArthurClemens - 12 May 2012

Thanks Kenneth, that clarifies things for me, and I'll also vote opt-in.

One more question, for the extensions that don't have a current active "author" in Foswiki. For example, extensions that were migrated by us, it's okay for us to "opt them in"? For example, FindElsewherePlugin? So the extensions that we should not "volunteer" to opt-in would be anything with Coordinate with Author set, as well as anything that appears to have an active author as part of the Foswiki community, even if they are "free to modify".

-- GeorgeClark - 12 May 2012

---

PLEASE KEEP THIS AT THE END!

So, what is the weight of opinion? If you agree that the decision is between opt-in and opt-out, please complete the table below. If your name isn't in the table and you are a contributor, please add it.

Who	Opt-in	Opt-out	Do nothing
CrawfordCurrie	X
OlivierRaginel		X
GeorgeClark	X		keep it simple please.
KennethLavrsen	X
MichaelDaum	X
ArthurClemens	X
FranzJosefGigler			just code
PaulHarvey			don't make me think

I think the vote is moot now. I seem to be the only one still in favor of Opt-in, and to be honest, I don't really care anymore. My goal has been reached: we've done something, and made most developers aware of the issues and the tools we built to help them. If anybody wants his extension to be automagically checked upon commit, all he has to do is create the TIDY file at the top level.

As it is hierarchical, one can easily disable TIDY in some folders (not that anything containing /lib/CPAN is automatically skipped).

And if someone wants to ensure it is NOT tidyed, he can create the TIDY file with OFF, or perl OFF as content.

Finally, if someone wants his code to be perltidyed with some special options, he can use: perl -q -l=0 -i=2 -pt=2 -nsfs -ce -novalign

As Crawford nicely summarized above. All that needs to be done is the BuildContrib target to tidy files. I'll work on that next week.

-- OlivierRaginel - 12 May 2012

I put Crawford's documentation into a topic called TIDY. I also updated:

• PerlTidy
• CodingStandards
• SvnRepository#RulesForCheckins

And I made the rejected commit message in core/tools/develop/pre-commit.pl more helpful.

-- PaulHarvey - 26 Sep 2012