Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to less.php library #7654

Closed
tzi opened this issue Apr 9, 2015 · 8 comments
Closed

Migrate to less.php library #7654

tzi opened this issue Apr 9, 2015 · 8 comments
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.

Comments

@tzi
Copy link
Member

tzi commented Apr 9, 2015

Hi piwik team!

We currently use the leafo/lessphp library that is not active since a long time (16 commits in the last year).
We should perhaps consider migrate to oyejorge/less.php. It seems more up to date with the less.js version, have less bugs and is more active.

This could prevent issue like #6103 and could ease theming.

Cheers,
Thomas.

@diosmosis
Copy link
Member

Just a note: We'd need to make sure WhiteLabel works with less.php; it uses some behavior of lessphp that may not be supported by less.php.

@tzi
Copy link
Member Author

tzi commented Apr 10, 2015

Sure, we have to be sure WhiteLabel works perfectly.

I think it will help us with the theming.
Because lessphp have a weird bug that messing with variable override.
But, it seems to be fixed with the v0.5.0!

Do you think to a specific behaviour that we needed, that could be missing in less.php?

Cheers,
Thomas.

@diosmosis
Copy link
Member

Well, WhiteLabel might depend on that weird bug. It's been a while since I looked at the code, but I think it depends on an import being done before processing everything else, even if the import is at the end of the file. Or something like that. Whatever it was seemed like it would be considered a bug in other solutions. Anyway, it's just a note. Before merging a change with less.php we have to make sure the WhiteLabel build passes, at least on a branch.

@mnapoli
Copy link
Contributor

mnapoli commented May 28, 2015

On this topic, here is an issue that was created (#8004):

If I use &:extend i get parser error. Is it possible to use another parser or solve this problem please.

This is a big bug it seems, it can't even compile Bootstrap 3: https://github.com/leafo/lessphp/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+extend

@mnapoli
Copy link
Contributor

mnapoli commented May 28, 2015

Regarding WhiteLabel, there is this comment in the Generator class:

Note: We always have to generate two less files. One "theme.less" file that imports the actually generated less file containing all the variables (less_variables.less). As I found out the variables will be overwritten if we write them directly into the "theme.less" file as an imported less file would overwrite them although we define them at the very end.

We have this problem on master currently so I updated to v0.5.0 which fixes the problem (done in #8015, should be fixed soon). Given the bug, and the workaround that is used in WhiteLabel, it doesn't seem like it will break (i.e. the workaround is not necessary anymore, but it shouldn't break from my understanding).

But in any case we could definitely try moving to less.php (lessphp seems in a really sad state). And given the bug we are all talking about, maybe it won't be that hard because WhiteLabel shouldn't break (note the "shouldn't" ;) ).

@mattab
Copy link
Member

mattab commented May 29, 2015

the workaround is not necessary anymore

since you upgraded the less.php to the latest and this fixes the bug, maybe we could remove this workaround from our codebase? (now that it's not needed)

maybe it won't be that hard because WhiteLabel shouldn't break (note the "shouldn't" ;) ).

btw we will check White Label works well in 2.14.0 in #8006

@diosmosis
Copy link
Member

since you upgraded the less.php

small note: we use 'lessphp', not 'less.php'. just in case you are thinking about closing this issue ;)

@mattab mattab added the Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. label Jul 14, 2015
@mattab mattab added this to the Long term milestone Jul 14, 2015
@mattab mattab modified the milestones: Long term, Mid term Dec 23, 2015
@mattab mattab modified the milestones: Long term, Mid term Dec 5, 2016
@sgiehl
Copy link
Member

sgiehl commented May 12, 2021

We have meanwhile switch to using wikimedia/less.php

@sgiehl sgiehl closed this as completed May 12, 2021
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

5 participants