Skip to content

Updates libsass to latest version #3

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

Closed
wants to merge 83 commits into from
Closed

Conversation

mgreter
Copy link
Contributor

@mgreter mgreter commented Oct 22, 2013

Hello

I tried to update libsass, as I ran into a problem with the currently included version of libsass. Unfortunately I've hit a few obstacles on the way, since upstream seems to have changed a little bit. I tried to adjust the code to my best knowledge and guesses.

Hope this pull request is useful!

Best wishes
Marcel Greter

P.s. I also made a pull request upstream:
sass/libsass#181

Customizes the build, as it would not compile otherwise.
Removed SASS_PERCENTAGE and SASS_DIMENSION. Looks like they
have been merged into SASS_NUMBER. SASS_NUMBER now gets an
additional unit string (as SASS_DIMENSION did before).
@caldwell
Copy link
Contributor

Thanks! I did some similar work about a month ago (I just pushed up a WIP patch if you want to look). The problem I ran into was that libsass hadn't implemented the user defined functions at that point in time. It looked like they were still in the middle of their huge refactor. Have they implemented that yet?

@mgreter
Copy link
Contributor Author

mgreter commented Oct 23, 2013

You're right. Just made a a few tests and it seems that this is not yet implemented. At least I now know what the other code in Sass.xs does, wasn't aware of this feature :) Thanks for publishing the refactoring branch. Do you want me to add the other changes to that branch and create another pull request? Otherwise I think you can close this one.

@caldwell
Copy link
Contributor

I've grabbed your branch into my local repo so that when libsass gets back up to feature parity I can look at the difference between what you and I did in detail and apply the best parts. At first glance yours looks more polished than my WIP commit (though in my defense I quit immediately when it because clear that the function stuff wasn't there yet).

I noticed that your commit doesn't have an update for the libsass submodule, so I don't know which version you are based off of. If you would update that while we're thinking about it, it would be helpful when looking back on the commit in the future (fbd5cec is an example of a commit that updates the submodule).

@mgreter
Copy link
Contributor Author

mgreter commented Oct 23, 2013

While you wrote this, I've put the missing pieces together to bring back the custom c functions to libsass :)

I updated my CSS-Sass repository so the libsass submodule now points to my latest version. I just tested it on windows. Maybe you have some time to check it on other systems too. My C is quite rusty, so I may have introduced some bugs, specially cross-platform wise (altough I think it should work just fine).

So let's see what the people of libsass think of it. In the mean time feel free to use my codebase if you think it's safe or wait for the people of libsass to commit the feature back in.

Adds special parsing for IE opacity styles.
Adds special parsing for IE expressions.
@mgreter
Copy link
Contributor Author

mgreter commented Dec 26, 2013

FYI: I just updated my local clone to use the latest libsass version, which IMO should now support (maybe just partially) source mapping and extend. I rebased my commits, so I had to create a new branch to not mess up this pending pull request: https://github.com/mgreter/CSS-Sass/tree/rebase-01

I haven't yet tested it much, but it seems to compile and load just fine!

It seems that upstream is interested in adding the pull request (sass/libsass#181). I guess they are just too busy right now with other important stuff. There is also a pull request for sassruby (sass/ruby-libsass/pull/7) that would need the changes for custom c functions. Seems just a matter of time.

Happy Holidays!

Percentages and dimensions are replaced by number type.
Add the unit as a string to the second parameter to new.
Rebased my changes to the upstream master branch
The unit for numbers should never be undefined.
This will avoid warnings about the "use of
uninitialized value in subroutine entry".
Also adds perl function for sass_compile_file.
@mgreter
Copy link
Contributor Author

mgreter commented Feb 25, 2014

Just wanted to give you a heads up, that I updated/rebased my local branches of CSS::Sass and libsass to the lastest version. Implemented the source maps for CSS::Sass and updated the test suite. We already started to use it in production for our FE deployment process.

@mgreter
Copy link
Contributor Author

mgreter commented Mar 20, 2014

Just another heads up. I've added a command line tool that is basically a start for a ruby sass or sassc replacement (I called it psass, maybe you got a better idea for a name?). I figured it might be handy to test latest libsass implementation. So to speak, I also updated libsass to the latest version this week. Last but not least, I published a sass2scss converter script lately (perl) and ported it (out of fun?) to c/c++. As a test I also included it for CSS-Sass. Not sure if it would better have its own Module, but it's pretty small, so I guess it fits nicely into CSS-Sass. The intention in the end would be to have it integrated into libsass directly. But might be handy anyway.

@mgreter
Copy link
Contributor Author

mgreter commented Aug 6, 2014

Sorry for insisting! But I would really appreciate some feedback!

@mgreter
Copy link
Contributor Author

mgreter commented Aug 10, 2014

Bump!

@caldwell
Copy link
Contributor

Hi Marcel,

First off, I appreciate all your work on this. I've been busy with "real life" and other issues and haven't had a ton of time to work on this. I apologize for not being very responsive.

I have gone through and looked at your patches and I like about 80% of it. The main issue I have (and why I haven't just merged it as is) is I would like things to be a little more organized by feature. Currently this pull request has about 3 or 4 different things going on that aren't strictly relevant to each other:

  • Updating to the latest libsass
  • Travis-CI stuff
  • Source-maps
  • sass_compile_file()

My plan was to take your patches and separate them into sections related to each other, then git rebase to squash a lot of the churn from updating the submodules incrementally (which is understandable while it's being debugged but I find distracting in the end). I wasn't going to ask you to do it because this is largely my own biases on how things should be done.

But... I clearly haven't done it yet. Is that something you'd be inclined to do? If so, that would make my task less daunting. If not, that's ok—I will definitely get to it eventually (but maybe not on a timescale that makes you happy :-)).

@mgreter
Copy link
Contributor Author

mgreter commented Aug 10, 2014

I have created a new branch with cleaned up commits here: #4

@mgreter mgreter force-pushed the master branch 5 times, most recently from 5a5e994 to 8552d1d Compare September 23, 2014 22:55
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.

2 participants