leafo / lessphp

LESS compiler written in PHP
http://leafo.net/lessphp
Other
2.2k stars 527 forks source link

parse error: failed at `@props: ~`"@{arguments}".replace(/[\[\]]|\,\sX/g, '')` #298

Closed krnlde closed 12 years ago

krnlde commented 12 years ago

I pulled the latest Bootstrap Branch 2.1.1-wip and tried to compile it getting this error:

Internal Server Error: exception 'Exception' with message 'parse error: failed at `@props: ~`"@{arguments}".replace(/[\[\]]|\,\sX/g, '')`; `common/assets/bootstrap/less/mixins.less on line 253' in ...\lib\vendor\Lessphp\lessc.inc.php:3144

This is the lines 250 - 257 in the latest mixins.less:

// Drop shadows
.box-shadow(@shadowA, @shadowB:X, ...){
  // Multiple shadow solution from http://toekneestuck.com/blog/2012/05/15/less-css-arguments-variable/
  @props: ~`"@{arguments}".replace(/[\[\]]|\,\sX/g, '')`;
  -webkit-box-shadow: @props;
     -moz-box-shadow: @props;
          box-shadow: @props;
}

I guess the problem is that there's currently no JS support in lessphp.

leafo commented 12 years ago

I'm discussing it here: https://github.com/cloudhead/less.js/issues/939

Feel free to add your input.

krnlde commented 12 years ago

phew! Cool stuff.

michsk commented 12 years ago

Just subscribing since i'm having this aswell (guess everyone wille who wants to use the latest versions)

RWOverdijk commented 12 years ago

What's the progress on this? Also is there a temporary workaround? I'm sort of stuck on this.

ds125v commented 12 years ago

I believe they've reverted this change in the next version of Bootstrap upstream (2.1.2). If you can't upgrade you could backport the changes.

RWOverdijk commented 12 years ago

I've checkoud it out (wip) and it works fine. Thanks.

On Wed, Oct 3, 2012 at 10:16 AM, David Scotson notifications@github.comwrote:

I believe they've reverted this change in the next version of Bootstrap upstream (2.1.2). If you can't upgrade you could backport the changes.

— Reply to this email directly or view it on GitHubhttps://github.com/leafo/lessphp/issues/298#issuecomment-9098899.

brice commented 12 years ago

Still open? What could we do? I currently use bootstrap-2.1.1

RWOverdijk commented 12 years ago

Not use lessphp. I've gone through the code and given up on it. Others might find it useful, but I think it's horrible. Just use lessc, works like a charm.

On Mon, Oct 22, 2012 at 3:30 PM, brice notifications@github.com wrote:

Still open? What could we do? I currently use bootstrap-2.1.1

— Reply to this email directly or view it on GitHubhttps://github.com/leafo/lessphp/issues/298#issuecomment-9663594.

ezp-matt commented 12 years ago

This issue is fixed in Bootstrap's latest release version, so this should be closed.

leafo commented 12 years ago

Yup. Thanks.

@RWOverdijk What's so horrible about the code?

RWOverdijk commented 12 years ago

@leafo The formatting is not what I'm used to, so that argument is invalid (yet I wanted to let you know that I actually like PSR*, perhaps you'd like to use it). There are some comments in there with commented out code which makes me doubt the reliability of the code (as it looks unfinished). I've seen a lot of more procedural-like written code in your files. My problem with that is the simple fact that it's not readable like that. Perhaps try splitting it up more. Also, I got different results when using lessc and lessphp (I trust the former to be correct). And finally, using php for less is just silly. lessc performs a LOT better. Don't get me wrong, I do like the fact that you've written this. A lot of other people find it useful :)

I don't like that this issue is closed. Embedding javascript is a feature in lesscss, and you're just ignoring it.

valorin commented 12 years ago

@RWOverdijk So you are basically saying that you think porting LESS to PHP is silly and we that should all just use the official node.js implementation... Did you miss the point of this being a PHP port? If you don't want to use it, then we aren't forcing you to use it. Don't come here and be rude for the sake of being rude!

@leafo, I think what you are doing with lessphp is fantastic. Thank you for your efforts! :)

RWOverdijk commented 12 years ago

@valorin Yes, that's basically what I'm saying. And yes, I probably do miss the point (as I don't see the point, at all). I was going to use it, so others wouldn't have to run node (if that's what you're getting at). But I think the smarter choice is in fact the official implementation. I'm not here to be rude. I wasn't rude, at all, until I looked at the code. And even then I wasn't being rude, just giving my opinion as leafo asked me.

leafo commented 12 years ago

This issue is closed because I am not implementing JavaScript interpolation for the reasons described here: https://github.com/cloudhead/less.js/issues/939

lessphp exists for portability. There are a lot of systems, especially PHP only ones, where it's much easier to integrate a PHP compiler instead of a node.js one. The authors of tools should be making it easier for developers to do things. Sure lessjs is faster but if it's difficult for a PHP developer to set up then they are probably just not going to use it. Just by this project existing I've let thousands of developers use less in their projects where they normally wouldn't have. I'd say that's a good thing.

As for the comments in the code, I'm assuming I left those there because they are potentially breaking changes and I wanted to see if any regressions would pop up. Either that or I just left it there by accident.

Also in terms of the readability of the code, I think I've written a pretty readable parser in PHP. The code is going to be complicated, there is no way around it. I think I've mitigated it pretty well. The chainable match methods with return values in reference arguments gives the best performance I've found and quite good readability. The whole project being in a single file does not bother me. I'm not out to change things just because of someone else's beliefs on the way code should be structured. If it's actually causing a problem then I will change it.

As for the differences, well it's hard to keep up. I have a handful of projects I work on. I'll gladly accept any help!

Anyway, thanks for the input. I do appreciate it. (and I don't think you're being rude)

RWOverdijk commented 12 years ago

It's difficult to reply to this. It's a nice response from your side and it's a satisfying one, too. I have nothing to add to it. I may not agree with the coding style, but seeing how it's useful for others I should just shut up. I've been working on some pretty large projects where I have to deal with high volumes of visitors. This may be the cause of my complaining, as I sometimes forget that most projects are different from mine. Thank you for your response and happy coding.

ezp-matt commented 12 years ago

@RWOverdijk the traffic volume of your project shouldn't really be relevant, because if you're not pre-processing and caching something like this, you're doing it wrong. That comment just makes you sound condescending.

There are multiple significant benefits to not relying on an external application in your project, such as ease of application deployment, platform agnosticism, better error handling, security (ability for sandboxing/chrooting, not relying on exec()), and flexibility that lessc doesn't provide (e.g. lessc can't read input from stdin, which I've found to be limiting when I don't want to write the input to a file first). If you haven't has to work around any of these issues in your "large projects", great, you're not the intended use-case for a library such as this. But don't belittle the work of those who do find it useful/necessary.

lessphp's coding is fine (albiet a little light on commenting/docblocks). PSR* aren't actual standards, they're merely recommendations that some self-appointed PHP developers decided to come up with. A lot of developers are openly contesting these "standards" anyhow (e.g. the Kohana Framework developers have openly stated that they have no intentions of ever adopting PSR-1/PSR-2). A different coding style has little impact on interoperability, and certainly isn't reason to declare someone's project as "horrible". I suggest you use a bit more tact in the future.

RWOverdijk commented 12 years ago

@matt-dns I'm speaking about projects where you go with live asset creation (one hit per file). This is a fine solution and makes pushing changes live a lot easier. But with large volumes of traffic, and no locking, this could cause some problems. Otherwise you'd use a cache warmup. I'm not trying to sound condescending, the projects aren't even mine, I just work on them :) So sorry if it sounded condescending, I just meant to explain something.

That's fine and all, I already said that. I personally do not like depending on php all that much. That doesn't mean that you're not allowed to. All of your arguments are fair. I do think, that with the right knowledge of these things, you could be able to set up something with better performance and scalability. That doesn't mean that using PHP has NO benefits, I do realize that. I didn't mean to stab people with my comments. I do admin that my comments were unfair and not very nice. In my defence, they were written while I was in a bad mood. (Yes, I know, I should then just stay away from github.).

True. But standards are attempts to make open source development easier. Abstracting your code through multiple files can greatly improve readability. But, if your intention is to just use it, it doesn't really matter. Also, I'd like to point out that I never said that the formatting is a real issue.

The formatting is not what I'm used to, so that argument is invalid (yet I wanted to let you know that I actually like PSR*, perhaps you'd like to use it).

Here, I'm already saying that this is a non-argument. It's just personal preference. I should perhaps use a bit more tact in the future, you're right. But I didn't call the code "terrible" because of the missing standards that I'm used to. You just make it sound like that.

Thank you for your comment, it was very useful.