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

Source Map Support #56

Merged
merged 7 commits into from
Dec 17, 2014
Merged

Source Map Support #56

merged 7 commits into from
Dec 17, 2014

Conversation

jescalan
Copy link
Owner

This pull request introduces a massively breaking change, but also includes full support for sourcemaps from any adapter that supports them. This means most css and js languages and hardly any of the html ones. The breaking change is that rather than returning a compiled string, you now get back an object which always contains a result key with the compiled result, and sometimes might include a sourcemap key as well.

The good news is that if, down the line, there is any other information that needs to be included (coffeescript already returns 2 different types of sourcemaps), the object is flexible enough to accommodate.

Tests are passing, the fail is a travis/libsass bug.


```

Docs below should explain the methods executed in the example above.
It's also important to note that accord return an object rather than a string from each of these methods. You can access the compiled result on the `result` property of this object. If the adapter supports source maps, the source map will also be on this object if you have passed in the correct options. Docs below should explain the methods executed in the example above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"note that accord return_s_ an object"

@notslang
Copy link
Contributor

still needs to be able to accept an object ({result: "input text", sourcemap: <the sourcemap from the last transform>}) and compose multiple sourcemaps. But other than that, it looks good.

@notslang
Copy link
Contributor

Oh, also - this requires a major version bump.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 720816c on 0.13.0 into * on master*.

jescalan pushed a commit that referenced this pull request Dec 17, 2014
@jescalan jescalan merged commit f061353 into master Dec 17, 2014
@jescalan jescalan deleted the 0.13.0 branch December 17, 2014 21:23
notslang referenced this pull request Dec 17, 2014
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.

3 participants