Unit tests are failing. Thus you probably broke something.

Cool. But I bet it's because the unit tests are wrong.

just curious:
can we compile less > css when call JHtml::_('stylesheet', 'path/to/stylesheet.less') and include stylesheet.css in result ... is there any test what better/faster less.js or compilation on the server side ?

can we compile less > css when call JHtml::_('stylesheet', 'path/to/stylesheet.less') and include stylesheet.css in result ... is there any test what better/faster less.js or compilation on the server side ?

You sure don't want to compile the LESS file on each pageload without at least some caching. That would be very stupid and we should not support that
In debug mode, we don't really care for performance, and I think using less.js makes sense there as you can debug the most in that case.

Cool. But I bet it's because the unit tests are wrong.

That may be the case. It still needs to be fixed

The unit tests stupidly rely on config settings being changed at runtime rather than the JDEBUG constant. So I guess I'll change that part back...

I find it rather silly that the test fail if JDEBUG is used. If that's the case, what is the point of this constant? Also, the particular test that was failing (JHtmlTest::testStylesheet) has another big problem. It creates files and directories and then removes them but, since it doesn't use a tearDown function to do it, it will leave a mess if any assertion fails.

It has little to do with this PR so I'm not going to fix that here.

@Bakual I mean in debug mode for both cases
but ok

@Fedik definitely no. Less compilation is should be part of a build process or, even better, done when saving a file. If you use any decent IDE, there should be some plugin or option to compile and minify/uglify your less files every time you save them.

In SublimeText I use https://github.com/timdouglas/sublime-less2css

I find it rather silly that the test fail if JDEBUG is used. If that's the case, what is the point of this constant?

In unit tests, not the whole application is instantiated. Thus the constants may not be available.

That's not the case here. bootstrap.php sets up this constant and any others we might need. It sets it as false but the test function seems to want to test "what if it were true?" Which is fine but, if every place in which debug is relevant requires testing both then having a constant for that is kind of pointless in the first place.

just an FYI: You can't use JDEBUG alone. JDEBUG is not defined if debugging is not enabled, so it would throw notices. You can only check like defined('JDEBUG').

@Hackwar I don't think that's true actually. If you look in includes/framework.php, it's being defined as whatever $config->debug is whether or not $config->debug is true. But, for several reasons, it seems like there's never a case where it's not better to check the global config instead of JDEBUG. Is there any reason to keep this constant around? Maybe its use should be deprecated.

@okonomiyaki3000 yes, I was false at that time. JDEBUG is actually set to the value in global config. JDEBUG is a valid constant, since you might have apps that don't define this in the configuration file in this way. Besides that, it is a lot quicker than looking it up in the app configuration all the time.

In a more proper structure, a lot of code wouldn't have direct access to the app config like we enable today via JFactory. Views, layout files, and modules would all fall into this. So the constant isn't so bad.

I really like it for convenience but it creates a problem for testing. If you have a function that needs to be tested with and without debug enabled, you can't do that if it uses the constant unless you run the test twice with different settings each time.

Testability and constants never really go well together when the constants can change behavior (in which case, it isn't really all that constant). Definitely something that needs addressed long term.

I guess this should have the new feature label, right?

Done ;)

Under the licence terms of the added javascript file the licence file must be included

It requires you to:
Include a copy of the license in any redistribution you may make that includes Less.js
Provide clear attribution to The Less Core Team for any distributions that include Less.js

In addition it is licensed as Apache 2 which is not compatible with GPL2 http://www.gnu.org/licenses/license-list.en.html#apache2

What about instead of instead of loading less.js,
if .less extension is detected:

we use:

$less = new JLess();
$less->checkedCompile(...);

because JLess extends lessc, which has:

    // compile only if changed input has changed or output doesn't exist
    public function checkedCompile($in, $out) {
        if (!is_file($out) || filemtime($in) > filemtime($out)) {
            $this->compileFile($in, $out);
            return true;
        }
        return false;
    }

That defeats the objective (I assume)of this PR which is to be able to test
a less stylesheet before compiling it

On 18 December 2015 at 09:31, Georgios Papadakis notifications@github.com
wrote:

What about instead of instead of loading less.js,
if .less extension is detected:

we use:

$less = new JLess();
$less->checkedCompile(...);

because JLess extends lessc, which has:

// compile only if changed input has changed or output doesn't exist
public function checkedCompile($in, $out) {
    if (!is_file($out) || filemtime($in) > filemtime($out)) {
        $this->compileFile($in, $out);
        return true;
    }
    return false;
}

—
Reply to this email directly or view it on GitHub
#5267 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@brianteeman
Right, i missed that

then to avoid modifying the current CSS files,

it could create the compiled CSS files inside

• /tmp folder,
• or in subfolder /test/ inside the given path

Personally I see no need for this functionality in the core
On 18 Dec 2015 9:44 am, "Georgios Papadakis" notifications@github.com
wrote:

then to avoid modifying the current CSS files,

it could create the compiled CSS files inside

• /tmp folder,
• or in subfolder /test/ inside the given path

—
Reply to this email directly or view it on GitHub
#5267 (comment).

I think I'll look into how less support could be provided via a plugin but it will still require some parts of Joomla to be a bit more flexible if it's to work as I think it should.

Can I close this here?

On 18 December 2015 at 10:59, Elijah Madden notifications@github.com
wrote:

I think I'll look into how less support could be provided via a plugin but
it will still require some parts of Joomla to be a bit more flexible if
it's to work as I think it should.

—
Reply to this email directly or view it on GitHub
#5267 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Sure, why not?