-
Notifications
You must be signed in to change notification settings - Fork 46
Remove comments from properties and values in parse time. #49
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
Conversation
i don't mind merging this since i think doing this is pretty silly anyways, but are people going to be annoying with this? there are some people who want the CSS to be exact, including spacing. |
some hacks require madness like this but whether or not they are relevant anymore who knows, I stay away from that stuff haha |
As much as I understand this bug and pull request, isn't it better removing all comments throughout (including comments outside properties and values) before unleashing the parser? Are comments gracefully handled when appearing outside properties and values? sorry, I haven't checked. That would be just two lines of code and one pass over the entire input string for removing all comments from the whole input string (as much as javascript's # Regex replace: remove all CSS comments (of the form /* anything */)
# First back-slash escapes the string, not the regex
regex = new RegExp('/\\*.*?\\*/', 'g')
string = string.replace(regex, '') ...excuse my coffeescript ;-) As a user of this repo, I'd probably not favor the extra cycles for parsing out comments on each pass, as in this pull request. Maybe it's better not to parse each and every element, notwithstanding that it also matters what is the treatment of comments elsewhere in the CSS input. |
i'm +1 to that but people are inevitably going to complain. plus, i don't see performance an issue with this library. |
Removing comments before parsing would break position information and so error reporting and source maps. |
The correct regex for CSS comments is |
hah. thank god for that regex! this PR's test was actually incorrect. thank god for not pulling it :) |
@jonathanong is there a release planned that includes this change? |
yeah v2 |
Fix bug in issue #48. Now parser removes from properties and values comments, but doing it directly in this fields, for this reason parser has trues info about lineno and column for error messages.