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

update less.js to 2.4.0 #79

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

update less.js to 2.4.0 #79

wants to merge 1 commit into from

Conversation

ftiasch
Copy link

@ftiasch ftiasch commented Mar 16, 2015

close #77

@cowboyd
Copy link
Owner

cowboyd commented Mar 17, 2015

It looks like all the testcases are failing. Wer you able to get them running on your machine?

@ftiasch
Copy link
Author

ftiasch commented Mar 17, 2015

@cowboyd Yes. commonjs.rb should also be updated to get this work.

I also posed a pull request on the commonjs.rb repo.

@cowboyd
Copy link
Owner

cowboyd commented Mar 17, 2015

That PR, cowboyd/commonjs.rb#17 also fails, and is not safe to merge

@ftiasch
Copy link
Author

ftiasch commented Mar 18, 2015

well, let me have an eye on both commits ...

2015-03-18 3:11 GMT+08:00 Charles Lowell notifications@github.com:

That PR, cowboyd/commonjs.rb#17
cowboyd/commonjs.rb#17 also fails, and is not
safe to merge


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

@maxd
Copy link

maxd commented Apr 3, 2015

@ftiasch Have you any ideas how to fix CI problems in this and related PR?

@ftiasch
Copy link
Author

ftiasch commented Apr 3, 2015

Sorry, I haven't. I got so busy these days to have no time the keep track
on it. :(

2015-04-03 16:20 GMT+08:00 Maxim Dobryakov notifications@github.com:

@ftiasch https://github.com/ftiasch Have you any ideas how to fix CI
problems in this and related PR?


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

@simi
Copy link

simi commented Apr 27, 2015

I've spend few hours on this today. I'll try to continue soon. I'm just not sure if we need to keep usage of less.Parser or we can move directly to simpler less.render.

@ftiasch
Copy link
Author

ftiasch commented Apr 28, 2015

The lessjs document says in new version (>= 2.0) we should use less.render
instead.

2015-04-28 5:18 GMT+08:00 Josef Šimánek notifications@github.com:

I've spend few hours on this today. I'll try to continue soon. I'm just
not sure if we need to keep usage of less.Parser or we can move directly
to simpler less.render.


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

@simi
Copy link

simi commented Apr 28, 2015

Sure, but less.Parser is still around.

@ftiasch
Copy link
Author

ftiasch commented Apr 28, 2015

Yes. But I think we'd better be consistent with what lessc is doing.

2015-04-28 19:18 GMT+08:00 Josef Šimánek notifications@github.com:

Sure, but less.Parser is still around.


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

@rstacruz
Copy link
Contributor

sadly it looks like commonjs.rb doesn't support 2 things:

  • index.js (require('./less-node') resolving to less-node/index.js)
  • relative paths (require(./environment))

the first one is easy to fix but the second is a bit more challenging.

@ftiasch
Copy link
Author

ftiasch commented May 21, 2015

@rstacruz I have made a pull request cowboyd/commonjs.rb#18 to fix these two (Thanks to @simi for adding tests).

However, the pr seems not yet be merged.

@rstacruz
Copy link
Contributor

hmm, tests are failing on this one even with that PR being applied to common.js.

@rstacruz
Copy link
Contributor

6) Less::Parser when configured with multiple load paths will load files from both paths
     Failure/Error: subject.parse('@import "one.less";').to_css.gsub(/\n/,'').strip.should eql ".one {  width: 1;}"
     NoMethodError:
       undefined method `less' for Less:Module
     # ./lib/less/parser.rb:62:in `parse'
  58) Less::Parser less-test selectors.less
     Failure/Error: Less.tree.functions[:add] = lambda do |*args| # function (a, b)
     NoMethodError:
       undefined method `functions' for #<V8::Object:0x007ff77a924748>
     # ./spec/less/parser_spec.rb:114:in `block (3 levels) in <top (required)>'

@cowboyd
Copy link
Owner

cowboyd commented May 21, 2015

Note: commonjs cowboyd/commonjs.rb#18 causes that library's test suite to fail which is why it has not yet been merged.

@cowboyd
Copy link
Owner

cowboyd commented May 21, 2015

Actually nevermind, looks like the specs are passing on that one, there is still a little bit of work to do there wrt test coverage.

@simi
Copy link

simi commented May 21, 2015

I'm still on this. But I'm not sure about my question in #79 (comment).

@cowboyd
Copy link
Owner

cowboyd commented May 21, 2015

@simi is there a problem supporting both?

@simi
Copy link

simi commented May 21, 2015

I don't see any benefits for that.

@cowboyd
Copy link
Owner

cowboyd commented May 21, 2015

Well, if we remove the old API, then it means that any dependent libraries need to be updated. This includes https://github.com/metaskills/less-rails for example.

@ftiasch
Copy link
Author

ftiasch commented May 21, 2015

I need to add some words here: I also tried to update less-rails, but the old rails helper (asset-url, asset-path etc) seems not working and I cannot figure out why.

So I didn't send a PR on less-rails.

There may be some more work to be done.

@simi
Copy link

simi commented May 21, 2015

@ftiasch can you share your code?

@ftiasch
Copy link
Author

ftiasch commented May 21, 2015

@simi It's on ftiasch/less-rails . trivial change

@simi
Copy link

simi commented May 21, 2015

@cowboyd I think we can keep public API same for both variants.

@tBaxter tBaxter mentioned this pull request Apr 19, 2016
@stickbyatlas
Copy link

Was bitten by the pre-2.x version of LESS in a project the other day, since @import statements can't include variables from sibling files. As a result, I ended up having to kick out to the JS version via bash in my Jekyll project:

desc "Compile LESS master to CSS output file"
task :compile_less do

    puts "\n"
    print "Compiling LESS... "

    sh '/usr/local/bin/lessc assets/css/styles.less _site/assets/css/styles.less.css'

    puts "done!"
    puts "\n"
end # task :compile_less

This feels gross to me. Does anybody have a better way to use Less 2.x? I'm a total Ruby / Jekyll novice and have no idea what I'm doing, but the above worked and let me add the Rake task to my Guardfile.

@brendon
Copy link

brendon commented Nov 7, 2018

Is there any enthusiasm for getting this moving again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to LESS 2.X
7 participants