Checking this great module with Coder returns some wrnings (nothing serious in fact, but still not nice to see...).

So, to make the code compliant with "Drupal coding standards", I've applied a few minor changes, as in the diff attached. Maybe it would be good to consider implementing it eventually...

CommentFileSizeAuthor
#5 css.code-cleanup.2.diff3.61 KBluti
css.code-cleanup.diff3.61 KBluti

Comments

fax8’s picture

Thanks for your patch LUTi.. anyway, it seems to me that, according to http://drupal.org/node/318#concat , the correct way to code string concatenation is using a space before and after the dot. You used one space only after the dot.

Please, modify the patch according to the above and I'll happily commit it to CVS.

Thanks

luti’s picture

fax8,

I prefer to use spaces on both sides as well. But, I am not sure about "Drupal standards". I've done only what is necessary to satisfy coder...

fax8’s picture

Quoting from http://drupal.org/coding-standards#concat

Always use a space between the dot and the concatenated parts to improve readability.

So, if Coder accepts dots with spaces only after the dots.. well that's something wrong with coder IMHO.
I'm going to report this to the Coder developers.

Meanwhile if you could provide a correct patch I'll be happy to commit it. Thanks!

fax8’s picture

Bug to Coder submitted at http://drupal.org/node/788278

luti’s picture

StatusFileSize
new3.61 KB

fax8,

as said, for Drupal 6 I am not sure what is the "coding standard". If I remember well, not a space on both sides...

In any case, here is a patch with spaces on both sides - since you insist I have to provide it, I've just edited the old diff file; it was a matter of approx. 10 seconds... ;-)

fax8’s picture

Status: Active » Fixed

I read some bug reports on string concatenation. Looks like in D6 is ok to use both A . B and A. B .. while in D7 the only accepted syntax will be A . B .. For more details see http://drupal.org/node/246568 and http://drupal.org/node/245115

Anyway, I prefere using A . B ... It's also the next D7 standard so it better for me.

Committed to CVS .. it will come in next stable version.

Thanks LUTi for your help,

Fabio Varesano

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.