GSOC - API to compress and compile Javascript and CSS - Time for your feed back

[kavith Thiranga]

Hi,

First of all here is the link to media package available in my git hub repo.

And you can read this blog post of mine to get a rough idea on how to using this API etc.

I would like to know your feedback soon. I have another 15 days to suggested pencils down. So I can try for the modifications early. Please refer to above mentioned blog post for more info on using this API.

I am currently working on a plugin to enable this functionality on the CMS. I hope you will find some time to review this and give me some feedback.

Thank you.

[piotr_cz]

I haven't tested the class, just peeked at the code and I'm really
happy that such package will be available in the core!
I really like how you prepared the classes, methods are fitting other
packages and it's really to pick up what methods are supposed to do.


Some notes:

I think, the convention now is to use JLoader instead of jimport (see
http://developer.joomla.org/manual/ch01s04.html#chap-Joomla_Platform_Manual-JLoader)

I'd replace the file comments fom '// File: ' to '/** File: ' in
combiner/js.php for two reasons:
- same convention as in css files (there I'd change '/*' to '/**')
- Usually double slash type comments are reserved for inline comments
inside the code, '/**' is used in beginning of file and new classes
(see http://developer.joomla.org/standards/ch03s04s02.html).


Good luck with CMS integration and thanks for sharing the progress!

On Aug 5, 4:39 am, kavith Thiranga <rc404m...@gmail.com> wrote:
> Hi,
>

> First of all here<https://github.com/kaviththiranga/joomla-platform/tree/JMedia/librari...>is the link to media package available in my git hub repo.
>
> And you can read this<http://kaviththiranga.blogspot.com/2012/07/gsoc-after-mid-term-evalua...>blog post of mine to get a rough idea on how to using this API etc.

[kavith Thiranga]

Hi piotr_cz,

Thank you very much for your review. I would like to mention that my mentors gave a big help in preparation of classes.
I think that your suggestions for the comment tags make sense. I will modify the combiners to add file comments in the way you have mentioned. And I'll refactor the usage of jimport too.

Thank you

--
Kavith Thiranga Lokuhewage,
Undergraduate,
BSc (Hons) Software Engineering,
Staffordshire University, UK.

[Janich]

Would it be better to get the mbstring functions out of the actual compressors and test it in the upper layer?

I cant really see the solution, but from experience, there are still hosts who doesnt have it.

[piotr_cz]

I don't have experience here, but you could take a look at JString
package which does some magic in the mbstring area. Or check if
mbstring is included in PHP 5.2+ (minimum requirement for Platform
12.1).

I've been thinking about the name of the package. When I read JMedia,
multimedia (images, video, sound) come to my mind. I know, js and css
files are stored in joomla/media folder but maybe something like
JAssestspacker, JPackager, JMinifier or JCompress (used in your
implementation plan) would better describe your API package.

In your recent blogpost I haven't found anything about evaluation of
use with other libraries (YUI compressor, Google closure). If these
were not possible to integrate, what were the reasons?

On Aug 7, 9:37 am, Janich <jan...@gmail.com> wrote:
> Would it be better to get the mbstring functions out of the actual
> compressors and test it in the upper layer?
> I cant really see the solution, but from experience, there are still hosts
> who doesnt have it.
>
>
>
>
>
>
>
> On Sunday, 5 August 2012 04:39:52 UTC+2, kavith Thiranga wrote:
>
> > Hi,
>

> > First of all here<https://github.com/kaviththiranga/joomla-platform/tree/JMedia/librari...>is the link to media package available in my git hub repo.
>
> > And you can read this<http://kaviththiranga.blogspot.com/2012/07/gsoc-after-mid-term-evalua...>blog post of mine to get a rough idea on how to using this API etc.

[kavith Thiranga]

Hi,

> Would it be better to get the mbstring functions out of the actual
> compressors and test it in the upper layer?

I haven't used any mbstring function inside the methods that do the compressing. But I actually have something in mind about overloading normal string functions with mbstring functions if user wants it.
Personally, I use UTF-8 encoding for javascripts to include my local language strings, So it may be good thing to support javascripts which are multibyte encoded. Otherwise those scripts will be ended up with crappy strings, after compression.

A solution will be to add a method to js compressor that will change php ini value ''mbstring.func_overload" to 2 (See http://php.net/manual/en/mbstring.overload.php) whenever user wants to compress a mb encoded file. May be we can add a compressor option also. I have just added a method to change mb internal encoding to 8 bit (see https://github.com/kaviththiranga/joomla-platform/blob/JMedia/libraries/joomla/media/compressor/js.php#L322) if php mbstrings functions are already enabled and overloading normal string functions with mbstring functions is also enabled. See the _changeCharEncoding() method. It actually does nothing to affect the compression unless user manually configures php.

So I think it will not be an issue.

regarding the name : Yep, my personal idea also was to use JCompress name. But with the suggestions from community, I decided to use JMedia, thinking of further improvements(like more compressors) which can be done. But it must not be the final name. We will see the feedback from others regarding this and I'll do the modifications if majority suggests that.

regarding the integration of available libraries : Actually the original idea was to implement 100% joomla-platform library that will handle compression and combining. So I have tried my best to do that. I have to mention that this library does not obfuscate javascripts. For those advanced things, we can integrate an available library. Since I had only a limited time, I just omitted obfuscating part . But I am hoping to continue to work on that after GSOC.

Thanks for you feedback.

[Janich]

I think the code looks good and seems to follow the normal Joomla convention, which means it should be rather easy to attach more functions later.

Also I agree on the name. JMedia sounds more like file/media functions to me. I'd really give a +1 on almost every other name not mentioning the 'media' (fx JAssestspacker, JPackager, JMinifier or JCompress)...

[Rouven Weßling]

I'm not completely trough but this looks like it will work great. I also have a few comments (or almost nitpicks):

-New packages should completely rely on the autoloader. I think your naming convention and file structure are correct for that but you're still using jimport(). Why?
-We don't use package exceptions (JMediaException) anymore and instead use SPL exceptions (RuntimeException, InvalidArgumentException, etc.)
-We try to avoid using JFile, JFolder, etc. when native PHP code exists to do the same thing. Forexample instead of using JFolder::files a DirectoryIterator can be used. An example can be found in JSession::getStores.

On 07.08.2012, at 11:29, kavith Thiranga <rc40...@gmail.com> wrote:

> A solution will be to add a method to js compressor that will change php ini value ''mbstring.func_overload" to 2 (See http://php.net/manual/en/mbstring.overload.php) whenever user wants to compress a mb encoded file. May be we can add a compressor option also. I have just added a method to change mb internal encoding to 8 bit (seehttps://github.com/kaviththiranga/joomla-platform/blob/JMedia/libraries/joomla/media/compressor/js.php#L322) if php mbstrings functions are already enabled and overloading normal string functions with mbstring functions is also enabled. See the _changeCharEncoding() method. It actually does nothing to affect the compression unless user manually configures php.

Not a good idea to encourage overloading since we generally require the exact opposite (that they'Re not overloaded). This is also for performance reason since the mb_* functions are quite a bit slower.

Also we haven't required the mbextension (yet), so whenever possible JString or if the method isn't present there phputf8 should be used as an abstraction.

Rouven

[kavith Thiranga]

Hi Rouven,

I am going to refactor the usage of jimport(). Even though JMediaException exists, it is not used, instead I have used RuntimeException, I will remove that file from the repository. I was using it for the prototype implementation.

Regarding the JFile and JFolder : I was not aware of that JFile and JFolder are to be avoided. I think I will be able to refactor those lines as soon as I am done with CMS part. Thank you for your review regarding that.

Regarding the mbstring overloading - Yep, the reason I didn't include the overloading in the js compressor was that. I was just confused to get a decision on that.

What is your idea regarding the name of the package?

Thank you for your valuable feedback.

[Ian]

I would probably lean towards calling the class JMediaCollection.  Then you'd have two subclasses - JMediaCollectionCss and JMediaCollectionJs.  These would be similar to the JMediaCombiner classes that you already have.  I would either have a compressor object passed in via a constructor parameter or via $options.

Then you'd use the class as:

$compressor = new JMediaCompressorJs();

$collection = new JMediaCollectionCss($compressor, $options);

or

$collection = new JMediaCollectionCss(array('compressor' => $compressor));   // this would allow you to support multiple compression schemes as well

You'd check if the compressor was an object and if not fall back to a default.

Then you'd have $collection->addFiles($files) which would be used to add all the files.

Then when you were done you might do $css = $collection->getCombined() which would somehow get a signature of all the file names included, check if the file had already been generated, if not, generate it, and then return the path to the generated file.

I don't really see a use case for getCombiners nor combineFiles nor isSupported.  I'm also not a huge fan of getInstance type methods that store objects, but that's probably more personal preference.

Anyway, some thoughts.

Ian

[Ian]

Also, as an additional note, in case it hasn't already been mentioned, JText shouldn't be used to generate exception messages.

Ian

[Sam Moffatt]

Some distros will unbundle modules into their own packages. Red Hat (by extension CentOS) are known for doing this with the XML libraries. Mbstring is likely also in this bucket. 

--
Sam Moffatt
http://pasamio.id.au

[Ian]

mb_string is a separate package on CentOS.

Ian

[Adam Stephen Docherty]

Hi Rouven, I see you made the following comment:

"-We try to avoid using JFile, JFolder, etc. when native PHP code exists to do the same thing. Forexample instead of using JFolder::files a DirectoryIterator can be used. An example can be found in JSession::getStores. "

And I was wondering if you could offer more clarification. I have always thought that using the J! framework libraries / methods was the accepted and to only fall back onto PHP native methods when J! didn't offer an abstract. In regards to JFile, JFolder, etc I thought that this was used to resolve issues with the FTP layer and stuff.

Thank you for any further insight that can be offered on this.

Kind regards,

Adam

[JSamir]

+1 Plz

[Elin Waring]

Are directory iterators supported for all versions of php that the platform and large applications on the platform support?

Elin

[Rouven Weßling]

On 14.08.2012, at 18:33, Adam Stephen Docherty <adam.d...@gmail.com> wrote:

> "-We try to avoid using JFile, JFolder, etc. when native PHP code exists to do the same thing. Forexample instead of using JFolder::files a DirectoryIterator can be used. An example can be found in JSession::getStores. "
>
> And I was wondering if you could offer more clarification. I have always thought that using the J! framework libraries / methods was the accepted and to only fall back onto PHP native methods when J! didn't offer an abstract. In regards to JFile, JFolder, etc I thought that this was used to resolve issues with the FTP layer and stuff.
>
> Thank you for any further insight that can be offered on this.

indeed we use JFile and JFolder for the FTP abstraction. However that's not always needed, the best example is probably the DirectoryIterator which uses quite a bit less memory compared to JFolder::files().

In the recent past we've tried to uncouple the platform packages as far as possible and the non-autoloadable fileystem package is always a prime candidate for that. Now for your work you won't be able to completely remove dependency on these classes when you wanna continue supporting FTP but as I said some things can be replaced.

On 14.08.2012, at 19:22, Elin Waring <elin....@gmail.com> wrote:

> Are directory iterators supported for all versions of php that the platform and large applications on the platform support?

DirectoryIterator was added with PHP 5 and we actually use it in a few places in Joomla 2.5 (JLoader for example)

Best regards
Rouven

[Mark Dexter]

I'm not sure I understand this. If we provide an abstraction layer for
something (like JFile) shouldn't we tell people to use it? If there
are optimizations people can make, could we make them in the
abstraction layer? Are tiny improvements in memory usage or speed
worth the confusion about when to use a platform function and when to
use a native function? If we want to replace a platform class with a
native PHP class, should we deprecate it and stop using it in the CMS?

One of the main goals of having a platform is to simplify development.
If we have a platform class that overlaps with a native class, that
seems to add confusion, not simplification.

Mark

[Rouven Weßling]

On 14.08.2012, at 19:43, Mark Dexter <dexter...@gmail.com> wrote:

> I'm not sure I understand this. If we provide an abstraction layer for
> something (like JFile) shouldn't we tell people to use it? If there
> are optimizations people can make, could we make them in the
> abstraction layer? Are tiny improvements in memory usage or speed
> worth the confusion about when to use a platform function and when to
> use a native function? If we want to replace a platform class with a
> native PHP class, should we deprecate it and stop using it in the CMS?
>
> One of the main goals of having a platform is to simplify development.
> If we have a platform class that overlaps with a native class, that
> seems to add confusion, not simplification.

JFile and JFolder date back to PHP4 days in which some of these things haven't existed. DirectoryIterator is - at least in my opinion - much nicer than JFolder::files()/JFolder::folders(). Indeed I'd be very much in favor of deprecating it. However these two methods do a lot. Just look at their signature. We'd have to make sure all of that is covered by PHP itself. So far we've only been replacing the simple - but very common - use case.

One of the main reasons we've moving away from these where possible - besides speed and memory - is that we wanna make the platform packages useable independent of each other. For example until recently the database package had a dependency on the filesystem package which didn't make much sense.

Best regards
Rouven

[Elin Waring]

Reducing dependencies is a worthy goal, but as Mark said the purpose of a platform is to make development easier. I think you should say there is a goal of reducing unnecessary and inappropriate dependencies (such as JUser and JSession and most importantly entanglement with CMS concepts), since in very many cases the way the pieces of the platform work together are what make it powerful.   As is clear there are many good reasons for using JFile and JFolder so we should avoid making blanket statements that their use "is discouraged" but instead say "use for purpose x is discouraged." A note in the docblocks indicating specific cases where using a php native alternative is preferable would be very helpful. 

Elin

Elin 

[Amy Stephen]

+1 Rouven - the platform should step aside when PHP catches up and there is a lot in the platform that existed to help PHP 4 do what the current minimum release is able to do on it's own. It builds credibility with experienced PHP devs when they know those maintaining the platform defer to PHP and augment its value.

JFile and JFolder date back to PHP4 days in which some of these things haven't existed. DirectoryIterator is - at least in my opinion - much nicer than JFolder::files()/JFolder::folders(). Indeed I'd be very much in favor of deprecating it. However these two methods do a lot. Just look at their signature. We'd have to make sure all of that is covered by PHP itself. So far we've only been replacing the simple - but very common - use case.

One of the main reasons we've moving away from these where possible - besides speed and memory - is that we wanna make the platform packages useable independent of each other. For example until recently the database package had a dependency on the filesystem package which didn't make much sense.

I can't over-emphasize the value of what you are saying to those of us trying to use the platform to build applications.
 

Best regards
Rouven

[kavith Thiranga]

Hi All,

Seems we have multiple suggestions here. I am happy to hear more feedback from you, So I can make modifications to the code in where I have used JFile and JFolder methods, If it is an important thing. I have to stop coding in 5 days. But that won't be an issue hence I am hoping to work on this further.

And further, can you suggest an alternate way to generate exception messages without using JText?

Thanks all of  you for your valuable reviews.

[Sam Moffatt]

You don't have to stop coding, you just have to have a copy of what
you've done for evaluation purposes. The only thing stopping you from
working is your own desire to do so.

Cheers,

Sam Moffatt
http://pasamio.id.au

[kavith Thiranga]

Hi Sam,

Yes of course, I am willing to continue with this. The only thing I was worried is that my target was to submit a community satisfied work for the final evaluations.

Thank you.

[Adam Stephen Docherty]

Rouven,

Thank you for taking the time to clarify. While the native php
directoryiterator class is very comprehensive I believe I will still
use the Joomla JFile, JDirectory helpers as this seems the safest
option for the time being... or is it? hmmm on that note, Do you know
if these classes will be depreciated in favour of using native PHP
classes in the near future?

Anyway my apologies, I seemed to have derailed this thread and I do
not want to detract from the great work Kavith is doing or distract
others from assisting him so I shall be quiet on this now.

Once again thanks!

Kind regards,

Adam

On Aug 14, 1:33 pm, Adam Stephen Docherty <adam.doche...@gmail.com>
wrote:

> > On 07.08.2012, at 11:29, kavith Thiranga <rc40...@gmail.com <javascript:>>

[Rouven Weßling]

Hi Adam,

On 16.08.2012, at 03:08, Adam Stephen Docherty <adam.d...@gmail.com> wrote:

> Rouven,
>
> Thank you for taking the time to clarify. While the native php
> directoryiterator class is very comprehensive I believe I will still
> use the Joomla JFile, JDirectory helpers as this seems the safest
> option for the time being... or is it? hmmm on that note, Do you know
> if these classes will be depreciated in favour of using native PHP
> classes in the near future?

I don't see us deprecating those classes anytime soon, however certain methods (JFile::read, JFolder::files and JFolder::folders are good candidates in my book) may be deprecated soon. First however we'll have to check if all options offered by those methods are covered by PHP or how we can expand on those.

Best regards
Rouven