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.
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.
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:
> 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 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.
On Mon, Aug 6, 2012 at 3:16 PM, piotr_cz <pkoniec...@hotmail.com> wrote:
> 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
> 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,
> > 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.
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.
> 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.
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:
> > 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.
> 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/librari...)
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.
On Tue, Aug 7, 2012 at 1:31 PM, piotr_cz <pkoniec...@hotmail.com> wrote:
> 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:
> > > 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.
> > 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/librari...) > 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.
> On Tue, Aug 7, 2012 at 1:31 PM, piotr_cz <pkoniec...@hotmail.com> wrote:
>> 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:
>> > > 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.
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)...
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 <rc404m...@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/librari...) 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.
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?
On Tue, Aug 7, 2012 at 8:31 PM, Rouven Weßling <m...@rouvenwessling.de> wrote:
> 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 <rc404m...@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/medi a/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.
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.
On Tuesday, 7 August 2012 11:23:26 UTC-4, kavith Thiranga wrote:
> 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.
> On Tue, Aug 7, 2012 at 8:31 PM, Rouven Weßling <m...@rouvenwessling.de>wrote:
>> 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 <rc404m...@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/medi a/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.
On Tuesday, 7 August 2012 14:14:51 UTC-4, Ian wrote:
> 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
> On Tuesday, 7 August 2012 11:23:26 UTC-4, kavith Thiranga wrote:
>> 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.
>> On Tue, Aug 7, 2012 at 8:31 PM, Rouven Weßling <m...@rouvenwessling.de>wrote:
>>> 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 <rc404m...@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/medi a/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.
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.
On Tuesday, August 7, 2012, piotr_cz wrote:
> 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 <javascript:;>> 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:
> > > 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.
On Tuesday, 7 August 2012 16:37:55 UTC-4, Samuel Moffatt wrote:
> 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.
> On Tuesday, August 7, 2012, piotr_cz wrote:
>> 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:
>> > > 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.
"-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.
On Tuesday, August 7, 2012 12:01:08 PM UTC-3, Rouven Weßling wrote:
> 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 <javascript:>> > 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/medi a/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.
> 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
> On Tuesday, August 7, 2012 12:01:08 PM UTC-3, Rouven Weßling wrote:
>> 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/medi a/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.
On Tuesday, August 14, 2012 12:33:23 PM UTC-4, Adam Stephen Docherty wrote:
> 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
> On Tuesday, August 7, 2012 12:01:08 PM UTC-3, Rouven Weßling wrote:
>> 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/medi a/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.
On 14.08.2012, at 18:33, Adam Stephen Docherty <adam.doche...@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.war...@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)
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.
On Tue, Aug 14, 2012 at 10:29 AM, Rouven Weßling <m...@rouvenwessling.de> wrote:
> On 14.08.2012, at 18:33, Adam Stephen Docherty <adam.doche...@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.war...@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)
On 14.08.2012, at 19:43, Mark Dexter <dextercow...@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.
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.
On Tuesday, August 14, 2012 1:50:52 PM UTC-4, Rouven Weßling wrote:
> On 14.08.2012, at 19:43, Mark Dexter <dexter...@gmail.com <javascript:>> > 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.
+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.
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?
On Wed, Aug 15, 2012 at 4:51 AM, Amy Stephen <amystep...@gmail.com> wrote:
> +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.
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.
On Wed, Aug 15, 2012 at 2:01 AM, kavith Thiranga <rc404m...@gmail.com> wrote:
> 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.
> On Wed, Aug 15, 2012 at 4:51 AM, Amy Stephen <amystep...@gmail.com> wrote:
>> +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.
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.
On Wed, Aug 15, 2012 at 9:31 PM, Sam Moffatt <pasa...@gmail.com> wrote:
> 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.
> On Wed, Aug 15, 2012 at 2:01 AM, kavith Thiranga <rc404m...@gmail.com>
> wrote:
> > 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.
> > On Wed, Aug 15, 2012 at 4:51 AM, Amy Stephen <amystep...@gmail.com>
> wrote:
> >> +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.