[#6391] - Converted generatecss to use oyejorge/less.php
- Closed
- 15 Mar 2015
- Medium
- Build: staging
- # 6391
- Diff
- nonumber:less.compiler
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
This adds the oyejorge/less.php (https://github.com/oyejorge/less.php) to Joomla and uses that in the generatecss.php.
oyejorge vs leafo
In my view the oyejorge LESS compiler does a better job at compiling LESS than the current leafo one.
Some things oyejorge does better:
- Comments are added in the right place (leafo adds all comments from included files at the top)
- Better handling of with calculations (percentages).
- Better handling of order of declarations.
- Keeps (adds) saces after comma's in rgb() and other function values (as it should be)
Example of output of the 2 (without colour compression):
leafo:
color: #fff;
text-shadow: 0 -1px 0 rgba(0,0,0,0.25);
oyejorge:
color: #ffffff;
text-shadow: 0 -1px 0 rgba(0, 0, 0, 0.25);
CSS minification
This PR also adds minification of the css files in the list in the generatecss.php.
Once this PR is ok'ed and merged, I can do the PRs to include the .min.css files in the templates and ather areas instead of the normal .css files.
joomla-cms-bot
- | Labels |
Added:
?
|
||
Nice! Just two points here:
1. Templates hathor and beez as well as the bootstrap files don’t need to be minified. The reason for the templates is that even if we minify those css files there are plenty more added as uncompressed, so we either refactor all those or we just drop them. (I have also a feeling that these two templates might eventually take the path of web links and decoupled in the next version = 3.5)
2. Adding one more way for doing the sameness is not exactly the best approach, although the arguments for the comments and all the rest stuff is something I would also agree!
Maybe we should merge these two PR’s (#6383 and #6391) as it would be easier for people to follow and test...
oyejorge is licenced as apache2 which is not compatible with gpl version 2
DOH! Did not see that.
Well that's a show stopper then!
@dgt41 I think all css files that are generated from LESS should get minified too. It is no extra effort and makes the page load faster, even if it is for old and barely used templates.
I see no reason to not use minified files.
This might need to pass by the lawyers, but we already have Bootstrap 2
(under Apache 2 license) shipped with the CMS.
On Wed, Mar 11, 2015 at 10:22 AM, Brian Teeman notifications@github.com
wrote:
oyejorge is licenced as apache2 which is not compatible with gpl version 2
https://www.gnu.org/philosophy/license-list.html#apache2On 11 March 2015 at 14:08, Peter van Westen notifications@github.com
wrote:This adds the oyejorge/less.php (https://github.com/oyejorge/less.php)
to
Joomla and uses that in the generatecss.php.
oyejorge vs leafoIn my view the oyejorge LESS compiler does a better job at compiling LESS
than the current leafo one.Some things oyejorge does better:
- Comments are added in the right place (leafo adds all comments from included files at the top)
- Better handling of with calculations (percentages).
- Better handling of order of declarations.
- Keeps (adds) saces after comma's in rgb() and other function values (as it should be)
Example of output of the 2 (without colour compression):
leafo:color: #fff;
text-shadow: 0 -1px 0 rgba(0,0,0,0.25);oyejorge:
color: #ffffff;
text-shadow: 0 -1px 0 rgba(0, 0, 0, 0.25);CSS minification
This PR also adds minification of the css files in the list in the
generatecss.php.Once this PR is ok'ed and merged, I can do the PRs to include the
.min.cssfiles in the templates and ather areas instead of the normal .css files.
You can view, comment on, or merge this pull request online at:
#6391
Commit Summary
- Converted generatecss to use oyejorge/less.php
- Fixed typo
File Changes
- M build/generatecss.php https://github.com/joomla/joomla-cms/pull/6391/files#diff-0 (55)
- M libraries/vendor/composer/autoload_classmap.php https://github.com/joomla/joomla-cms/pull/6391/files#diff-1 (1)
- A libraries/vendor/oyejorge/Cache.php https://github.com/joomla/joomla-cms/pull/6391/files#diff-2 (241)
- A libraries/vendor/oyejorge/Less.php https://github.com/joomla/joomla-cms/pull/6391/files#diff-3 (10270)
- A libraries/vendor/oyejorge/Version.php https://github.com/joomla/joomla-cms/pull/6391/files#diff-4 (15)
Patch Links:
- https://github.com/joomla/joomla-cms/pull/6391.patch
- https://github.com/joomla/joomla-cms/pull/6391.diff
—
Reply to this email directly or view it on GitHub
#6391.Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/—
Reply to this email directly or view it on GitHub
#6391 (comment).
I've asked the creators (via https://twitter.com/gpeasy) if the license can be changed.
This might need to pass by the lawyers, but we already have Bootstrap 2
(under Apache 2 license) shipped with the CMS.
Hmmm, what to say/think about that???
@nonumber don’t get me wrong, all I meant is that if those two templates eventually end up in their own repo then we would have to remove them from the array and also make another individual generatecss.php for each of them. So all I am saying is let’s not do this twice, let’s apply this for isis and protostar and act for the others depending on the outcome of the decoupling conversation...
What version of less is this based on. My biggest gripe at the moment is that the current LESS PHP compiler doesn't deal with import (reference) cause it's massively out of date :P
zero-24
- | Rel_Number | ⇒ | 6378 | |
| Relation Type | ⇒ | Pull Request for |
| Category | ⇒ | Repository |
Setting to needs review until @nonumber gets a reply from author
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6391.
| Status | Pending | ⇒ | Needs Review |
How do we go about the fact that Bootstrap is licenced under Apache License v2.0 too?
If I remember correctly this was looked into and the decision was made that
this is css and not php and so rules are different. I am NOT saying I agree
On 14 March 2015 at 11:00, Peter van Westen notifications@github.com
wrote:
How do we go about the fact that Bootstrap is licenced under Apache
License v2.0 too?—
Reply to this email directly or view it on GitHub
#6391 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
So would it be ok to only include this in the build folder. And only use it for the generatecss.php?
Then it won't be distributed in the official releases...
Possibly - but above my pay grade
On 14 March 2015 at 11:10, Peter van Westen notifications@github.com
wrote:
So would it be ok to only include this in the build folder. And only use
it for the generatecss.php?
Then it won't be distributed in the official releases...—
Reply to this email directly or view it on GitHub
#6391 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Changing licenses seems to be a no-go, as the less.php compiler wants to follow the licensing of the less.js, which is Apache v2, and not planning to change.
So I moved the less compiler php files to the build folder.
There should be no licensing issues with this, as we are only using it to generate files which we distribute. We are not distributing the php files themselves as part of the package.
The generated css/less files do not inherit the Apache license.
I would need to check with the legal team, but I think that simply moving it to the build folder is unlikely to be sufficient to avoid a licensing conflict since we are still distributing the conflicting code by hosting it on Github. However, the issue could be avoided if they simply dual-licensed their code under Apache2 and GPL2.
phproberto
-
Following internal discussion we have concluded that it is not going to be possible to accept this PR due to the license conflict. It can always be re-visited if oyejorge is willing to dual-license his code. Thanks for putting the PR forward for consideration.
Closing due to the licence clash.
| Status | Needs Review | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-03-15 19:59:40 |
Ok, I'll be creating a new php to add the minifying to the generatecss.php using the current lessphp.
I'll also tweak it a bit so that the output is better.
Less PHP isn't actually the latest version of less. We are missing things like require (reference) and as he said comments are also added in completely the wrong place with the current compiler
I'm not against having minified files. So it's valid by me - improving the current less compiler would take some time. And that's a nice shorter term improvement
oyejorge is licenced as apache2 which is not compatible with gpl version 2
https://www.gnu.org/philosophy/license-list.html#apache2
On 11 March 2015 at 14:08, Peter van Westen notifications@github.com
wrote:
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/