oyejorge / less.php

less.js ported to PHP.
http://lessphp.typesettercms.com
Apache License 2.0
656 stars 2 forks source link

Porting from leafo/lessphp; fail on `!important` in mixin #335

Closed vanderlee closed 7 years ago

vanderlee commented 7 years ago

Trying to move from leafo/lessphp and encountered just one inconsistency.

The following fragment (simplified for readability) compiles in leafo, not in oyejorge:

.box-shadow (@string) {
    box-shadow:         @string;
}
div {
    .box-shadow(0 4px 16px rgba(0,0,0,.5) !important);
}

If I remove the !important, it compiles, but obviously without the !important that I need. If I move it after the mixin call (like so: .box-shadow(0 4px 16px rgba(0,0,0,.5)) !important;), it works as intended.

So it's fixed pretty easily, but still; an inconsistency which may make porting a bit more difficult and which might be fixed in less.php itself or in the lessc.inc.php file.

Using v1.7.0.10

seven-phases-max commented 7 years ago

I suppose you have to use ~'!important' there. The same issue is in the reference less.js compiler (obviously the problem is in the exceptional ! symbol not fitting into the "ordinal" value syntax).

vanderlee commented 7 years ago

Compatibility with less.js would have preference over compatibility with leago/lessphp. Is this an issue worth reporting to less.js; the "ordinal" !important syntax seems reasonable to me, or should this be considered expected behaviour?

seven-phases-max commented 7 years ago

Is this an issue worth reporting to less.js;

To be honest, it would make sense only if you can do a PR with a corresponding fix. Curiously there was already the same error later fixed and then broken again.

vanderlee commented 7 years ago

The way I read it, they've made a conscious decission to put !important outside the parenthesis, and with good reasons. I think a fix in lessc.inc.php should be pretty easy (str_replace('!important), ') !important', $css)` should do), and fixes the compatibility issue. I'll see if I can add this fix and unittest for it soon.