Skip to content

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

Closed
wants to merge 1 commit into from
Closed

Remove comments from properties and values in parse time. #49

wants to merge 1 commit into from

Conversation

vladkens
Copy link

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.

@jonathanong
Copy link
Contributor

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.

@tj
Copy link
Member

tj commented Oct 16, 2013

some hacks require madness like this but whether or not they are relevant anymore who knows, I stay away from that stuff haha

@ghost ghost assigned tj Nov 4, 2013
@matanox
Copy link

matanox commented Dec 6, 2013

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 replace function is efficient). Something like:

  # 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.

@jonathanong
Copy link
Contributor

i'm +1 to that but people are inevitably going to complain. plus, i don't see performance an issue with this library.

@andreypopp
Copy link
Contributor

Removing comments before parsing would break position information and so error reporting and source maps.

@BYK
Copy link

BYK commented Dec 8, 2013

The correct regex for CSS comments is /\/\*[^*]*\*+([^/*][^*]*\*+)*\//g btw. This is from http://www.w3.org/TR/CSS21/grammar.html

@jonathanong
Copy link
Contributor

hah. thank god for that regex! this PR's test was actually incorrect. thank god for not pulling it :)

@BYK
Copy link

BYK commented Dec 30, 2013

@jonathanong is there a release planned that includes this change?

@jonathanong
Copy link
Contributor

yeah v2

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.

6 participants