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

Add clarity to sourceMap option #606

Merged
merged 1 commit into from
Jan 7, 2015
Merged

Add clarity to sourceMap option #606

merged 1 commit into from
Jan 7, 2015

Conversation

jescalan
Copy link
Contributor

@jescalan jescalan commented Jan 6, 2015

Very small PR that adds the fact that you must specify outFile as well in order to generate a source map.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f5d27d9 on jenius:readme into * on sass:master*.

@am11
Copy link
Contributor

am11 commented Jan 6, 2015

Developers are pretty normal humans (perhaps I am being little optimistic here) but the relative paths is quite a known concept, even outside the dev community. :)

w.r.t is "with respect to".

sourceMap:true generates sourcemap at the same path as outFile path.

sourceMap:/absolute/path/to/file.map is acceceptable as well.

However, when source-map option is set to a relative file path, we resolve the path with respect to the path of that of output file.

This does not mean that the outFile option can not be set as a relative path. It surely can, and in which case we resolve outFile w.r.t file options (if available), which again can be set to absolute or relative path. In case when file option is set to a relative path or outFile is relative and file is missing, we resolve them against process.cwd(), which returns "current directory location" of the node process.

If the text in readme is not depicting the intended behavior, feel free to propose a better alternative.

Thanks for the PR! 👍


A logical question: why don't we resolve sourcemap path against process.cwd() if outFile is missing?

It is because it does not make sense to generate a source-map file without having the destination.

Also, note that now we have embedded content option in source-map as well, so it should probably be a legit case to have not outFile set. I haven't tested the behavior of libsass against this, but in future we will find out. Ideally, we should have a feature working with least options set.

@jescalan
Copy link
Contributor Author

jescalan commented Jan 7, 2015

The fact that you had to write that lengthy of a comment to explain what it meant is exactly my point. Here's how I might reword it:

sourceMap: You must define this option as well as outFile in order to generate a source map. If it is set to true, the source map will be generated at the path provided in the outFile option. If set to a path (string), the source map will be generated at the provided path.

Does that seem correct? It makes more sense to me on initial reading.

@am11
Copy link
Contributor

am11 commented Jan 7, 2015

Yeah it looks fine. Just to be consistent with rest of the text:

... If set to a path (String)...

@jescalan
Copy link
Contributor Author

jescalan commented Jan 7, 2015

Ok just rebased to include the modified text. 🎉

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d97ca3e on jenius:readme into a11fafd on sass:master.

am11 added a commit that referenced this pull request Jan 7, 2015
Add clarity to sourceMap option
@am11 am11 merged commit 520dd86 into sass:master Jan 7, 2015
@am11
Copy link
Contributor

am11 commented Jan 7, 2015

Thanks @Jenius! 👍

@jescalan jescalan deleted the readme branch January 8, 2015 15:46
@jescalan
Copy link
Contributor Author

jescalan commented Jan 8, 2015

Whoo! 🎉

jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
[WIP] Fixes and improves travis ci build
Friendly-users referenced this pull request in Friendly-users/node-sass Jul 9, 2024
-----
It is inappropriate to include political and offensive content in public code repositories.

Public code repositories should be neutral spaces for collaboration and community, free from personal or political views that could alienate or discriminate against others. Political content, especially that which targets or disparages minority groups, can be harmful and divisive. It can make people feel unwelcome and unsafe, and it can create a hostile work environment.

Please refrain from adding such content to public code repositories.
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.

None yet

3 participants