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

Option: Map URL is now disableable (#357) #440

Merged
merged 1 commit into from
Jul 28, 2014
Merged

Option: Map URL is now disableable (#357) #440

merged 1 commit into from
Jul 28, 2014

Conversation

am11
Copy link
Contributor

@am11 am11 commented Jul 12, 2014

Fixes #357.

Couple of notes:

  • The option is reversed omit_source_map_url as opposed to source_map_url to make it a non-breaking change (for instance, the current state of node-sass is working just fine with this change. later they can provide a CLI switch and the code to back in sass.js and binding.cpp to make use of this feature).
  • I used C99's stdbool in sass_interface.h. With GCC's std=c99 and VC12 (devenv2013), its supported and compiled well on Win7 (don't know about the others compilers). I saw @akhleung's comment about lack of bool support in C. Let me know, if you guys have concerns regarding C99 (particularly <stdbool.h>), I will change it to int.

akhleung pushed a commit that referenced this pull request Jul 28, 2014
Option: Map URL is now disableable (#357)
@akhleung akhleung merged commit aa02300 into sass:master Jul 28, 2014
@mgreter
Copy link
Contributor

mgreter commented Jul 29, 2014

I have a regression with this merge in my unit tests (Perl Binding CSS-Sass). Since this merge libsass will always include the sourceMappingURL if omit_source_map_url is not set. I wonder if this makes sense if source_map_file is not set to anything at all?

Currently appended to the resulting css:

/*# sourceMappingURL= */'

Changing

if (!omit_source_map_url) output += format_source_mapping_url(source_map_file);

to

if (source_map_file != "" && !omit_source_map_url) {
  output += format_source_mapping_url(source_map_file);
}

fixes my unit tests

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.

Add option for appending sourceMappingURL comment
3 participants