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

Make source_map_embed independent of source_map_file #1516

Merged
merged 1 commit into from
Sep 3, 2015

Conversation

mgreter
Copy link
Contributor

@mgreter mgreter commented Sep 2, 2015

This should IMO address #885

The source_map_embed option no longer needs source_map_file to be present. We currently always return a source-map on the C-API level (AFAIR), but from the semantics we do not guarantee that, when neither of two main options are set. source_map_embed has higher priority and source_map_contents doesn't do anything on its own.

So we have these options in regard of source-maps:

source_map_file: libsass only adds the sourceMappingURL to the output
source_map_embed: generate sourceMappingURL with embedded source-map
source_map_contents: adds the sourceContents array to the source-map
omit_source_map_url: suppress any sourceMappingURL output

@mgreter mgreter added this to the 3.3 milestone Sep 2, 2015
@mgreter mgreter self-assigned this Sep 2, 2015
@am11
Copy link
Contributor

am11 commented Sep 2, 2015

I have tested with your branch. It seems like even prior to this delta, some parts of #885 were addressed with previous improvements in source-map and file related PRs.

With this PR, AFAICT, all the concerns are addressed and the feature is much resilient than it used to be.

Thanks @mgreter for doing this. Your sourcemap library is precious! 😎

mgreter added a commit that referenced this pull request Sep 3, 2015
Make `source_map_embed` independent of `source_map_file`
@mgreter mgreter merged commit 93a3df3 into sass:master Sep 3, 2015
@mgreter mgreter deleted the feature/indep-source-map-embed-opt branch September 3, 2015 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants