Improve block_comment parsing

Start to use `lex_css` and `peek_css` to indicate if you
want to skip over `block_comments` or only "white-space".
"white-space" in this case also includes `line_comments`!

Fixes #941

Author: mgreter

parser.cpp

@@ -253,7 +253,7 @@ namespace Sass {
         else error("expecting another url or quoted path in @import list", pstate);
       }
       first = false;
-    } while (lex< exactly<','> >());
+    } while (lex_css< exactly<','> >());
     return imp;
   }
 
@@ -286,7 +286,7 @@ namespace Sass {
       // if there's anything there at all
       if (!peek< exactly<')'> >()) {
         do (*params) << parse_parameter();
-        while (lex< alternatives < spaces,block_comment, exactly<','> > >());
+        while (lex_css< exactly<','> >());
       }
       while (lex< alternatives < spaces, block_comment > >()) {};
       if (!lex< exactly<')'> >()) error("expected a variable name (e.g. $x) or ')' for the parameter list for " + name, pstate);
@@ -339,7 +339,7 @@ namespace Sass {
       // if there's anything there at all
       if (!peek< exactly<')'> >()) {
         do (*args) << parse_argument();
-        while (lex< alternatives < block_comment, exactly<','> > >());
+        while (lex_css< exactly<','> >());
       }
       while (lex< block_comment >());
       if (!lex< exactly<')'> >()) error("expected a variable name (e.g. $x) or ')' for the parameter list for " + name, pstate);
@@ -505,7 +505,7 @@ namespace Sass {
         }
         if (peek_newline()) ref_wrap->has_line_break(true);
       }
-      while (peek< sequence < optional_css_whitespace, optional < block_comment >, exactly<','> > >())
+      while (peek_css< exactly<','> >())
       {
         // consume everything up and including the comma speparator
         reloop = lex< sequence < optional_css_comments, exactly<','> > >() != 0;
@@ -528,7 +528,7 @@ namespace Sass {
   {
     Position sel_source_position(-1);
     Compound_Selector* lhs;
-    if (peek< alternatives <
+    if (peek_css< alternatives <
           exactly<'+'>,
           exactly<'~'>,
           exactly<'>'>
@@ -549,7 +549,7 @@ namespace Sass {
     bool cpx_lf = peek_newline();
 
     Complex_Selector* rhs;
-    if (peek< alternatives <
+    if (peek_css< alternatives <
                 exactly<','>,
                 exactly<')'>,
                 exactly<'{'>,
@@ -589,10 +589,10 @@ namespace Sass {
         return seq;
       }
     }
-    if (sawsomething && lex< sequence< negate< functional >, alternatives< identifier_fragment, universal, quoted_string, dimension, percentage, number > > >()) {
+    if (sawsomething && lex_css< sequence< negate< functional >, alternatives< identifier_fragment, universal, quoted_string, dimension, percentage, number > > >()) {
       // saw an ampersand, then allow type selectors with arbitrary number of hyphens at the beginning
       (*seq) << new (ctx.mem) Type_Selector(pstate, unquote(lexed));
-    } else if (lex< sequence< negate< functional >, alternatives< type_selector, universal, quoted_string, dimension, percentage, number > > >()) {
+    } else if (lex_css< sequence< negate< functional >, alternatives< type_selector, universal, quoted_string, dimension, percentage, number > > >()) {
       // if you see a type selector
       (*seq) << new (ctx.mem) Type_Selector(pstate, lexed);
       sawsomething = true;
@@ -603,14 +603,15 @@ namespace Sass {
     }
 
     while (!peek< spaces >(position) &&
-           !(peek < alternatives < exactly<'+'>,
-                                   exactly<'~'>,
-                                   exactly<'>'>,
-                                   exactly<','>,
-                                   exactly<')'>,
-                                   exactly<'{'>,
-                                   exactly<'}'>,
-                                   exactly<';'>
+           !(peek_css < alternatives <
+               exactly<'+'>,
+               exactly<'~'>,
+               exactly<'>'>,
+               exactly<','>,
+               exactly<')'>,
+               exactly<'{'>,
+               exactly<'}'>,
+               exactly<';'>
              > >(position))) {
       (*seq) << parse_simple_selector();
     }
@@ -619,6 +620,7 @@ namespace Sass {
 
   Simple_Selector* Parser::parse_simple_selector()
   {
+    lex < css_comments >();
     if (lex< id_name >() || lex< class_name >()) {
       return new (ctx.mem) Selector_Qualifier(pstate, unquote(lexed));
     }
@@ -725,29 +727,29 @@ namespace Sass {
 
   Attribute_Selector* Parser::parse_attribute_selector()
   {
-    lex< exactly<'['> >();
+    lex_css< exactly<'['> >();
     ParserState p = pstate;
-    if (!lex< attribute_name >()) error("invalid attribute name in attribute selector", pstate);
+    if (!lex_css< attribute_name >()) error("invalid attribute name in attribute selector", pstate);
     string name(lexed);
-    if (lex< exactly<']'> >()) return new (ctx.mem) Attribute_Selector(p, name, "", 0);
-    if (!lex< alternatives< exact_match, class_match, dash_match,
-                            prefix_match, suffix_match, substring_match > >()) {
+    if (lex_css< exactly<']'> >()) return new (ctx.mem) Attribute_Selector(p, name, "", 0);
+    if (!lex_css< alternatives< exact_match, class_match, dash_match,
+                                prefix_match, suffix_match, substring_match > >()) {
       error("invalid operator in attribute selector for " + name, pstate);
     }
     string matcher(lexed);
 
     String* value = 0;
-    if (lex< identifier >()) {
+    if (lex_css< identifier >()) {
       value = new (ctx.mem) String_Constant(p, lexed);
     }
-    else if (lex< quoted_string >()) {
+    else if (lex_css< quoted_string >()) {
       value = parse_interpolated_chunk(lexed, true); // needed!
     }
     else {
       error("expected a string constant or identifier in attribute selector for " + name, pstate);
     }
 
-    if (!lex< exactly<']'> >()) error("unterminated attribute selector for " + name, pstate);
+    if (!lex_css< exactly<']'> >()) error("unterminated attribute selector for " + name, pstate);
     return new (ctx.mem) Attribute_Selector(p, name, matcher, value);
   }
 
@@ -979,10 +981,10 @@ namespace Sass {
     Map* map = new (ctx.mem) Map(pstate, 1);
     (*map) << make_pair(key, value);
 
-    while (lex< exactly<','> >())
+    while (lex_css< exactly<','> >())
     {
       // allow trailing commas - #495
-      if (peek< exactly<')'> >(position))
+      if (peek_css< exactly<')'> >(position))
       { break; }
 
       Expression* key = parse_list();

parser.hpp

@@ -76,7 +76,7 @@ namespace Sass {
     {
 
       // maybe use optional start position from arguments?
-      const char* it_before_token = start ? start : position;
+      const char* it_position = start ? start : position;
 
       // skip white-space?
       if (mx == url ||
@@ -88,11 +88,13 @@ namespace Sass {
           mx == optional_css_comments ||
           mx == optional_css_whitespace
       ) {
-        return it_before_token;
+        return it_position;
       }
 
       // skip over spaces, tabs and sass line comments
-      return optional_css_whitespace(it_before_token);
+      const char* pos = optional_css_whitespace(it_position);
+      // always return a valid position
+      return pos ? pos : it_position;
 
     }
 
@@ -112,33 +114,34 @@ namespace Sass {
     // white-space handling is built into the lexer
     // this way you do not need to parse it yourself
     // some matchers don't accept certain white-space
+    // we do not support start arg, since we manipulate
+    // sourcemap offset and we modify the position pointer!
     template <prelexer mx>
     const char* lex()
     {
 
-      // remeber interesting position
-      const char* wspace_start = position;
-
       // sneak up to the actual token we want to lex
       // this should skip over white-space if desired
       const char* it_before_token = sneak < mx >(position);
 
-      // advance position (add whitespace before current token)
-      before_token = after_token.inc(position, it_before_token);
-
       // now call matcher to get position after token
       const char* it_after_token = mx(it_before_token);
 
       // assertion that we got a valid match
       if (it_after_token == 0) return 0;
+      // assertion that we actually lexed something
+      if (it_after_token == it_before_token) return 0;
 
       // create new lexed token object (holds all parse result information)
-      lexed = Token(wspace_start, it_before_token, it_after_token);
+      lexed = Token(position, it_before_token, it_after_token);
+
+      // advance position (add whitespace before current token)
+      before_token = after_token.add(position, it_before_token);
 
-      // update position of after_token (add token to before position)
-      after_token = before_token.inc(it_before_token, it_after_token);
+      // update after_token position for current token
+      after_token.add(it_before_token, it_after_token);
 
-      // ToDo: could probably do this incremetal on original object
+      // ToDo: could probably do this incremetal on original object (API wants offset?)
       pstate = ParserState(path, source, lexed, before_token, after_token - before_token);
 
       // advance internal char iterator
@@ -151,17 +154,18 @@ namespace Sass {
     const char* lex_css()
     {
       // throw away comments
+      // update srcmap position
       lex < css_comments >();
       // now lex a token
       return lex< mx >();
     }
 
     // skips over css comments
     template <prelexer mx>
-    const char* peek_css()
+    const char* peek_css(const char* start = 0)
     {
       // now peek a token (skip comments first)
-      return peek< mx >(peek < css_comments >());
+      return peek< mx >(peek < css_comments >(start));
     }
 
 #ifdef __clang__

position.cpp

@@ -33,8 +33,9 @@ namespace Sass {
   // increase offset by given string (mostly called by lexer)
   // increase line counter and count columns on the last line
   // ToDo: make the col count utf8 aware
-  void Offset::add(const char* begin, const char* end)
+  Offset Offset::add(const char* begin, const char* end)
   {
+    if (end == 0) return *this;
     while (begin < end && *begin) {
       if (*begin == '\n') {
         ++ line;
@@ -45,6 +46,7 @@ namespace Sass {
       }
       ++begin;
     }
+    return *this;
   }
 
   // increase offset by given string (mostly called by lexer)
@@ -103,9 +105,10 @@ namespace Sass {
   ParserState::ParserState(string path, const char* src, Token token, Position position, Offset offset)
   : Position(position), path(path), src(src), offset(offset), token(token) { }
 
-  void Position::add(const char* begin, const char* end)
+  Position Position::add(const char* begin, const char* end)
   {
     Offset::add(begin, end);
+    return *this;
   }
 
   Position Position::inc(const char* begin, const char* end) const

position.hpp

@@ -19,7 +19,7 @@ namespace Sass {
       Offset(const size_t line, const size_t column);
 
       // return new position, incremented by the given string
-      void add(const char* begin, const char* end);
+      Offset add(const char* begin, const char* end);
       Offset inc(const char* begin, const char* end) const;
 
       // init/create instance from const char substring
@@ -59,7 +59,7 @@ namespace Sass {
       const Position operator+ (const Offset &off) const;
       const Offset operator- (const Offset &off) const;
       // return new position, incremented by the given string
-      void add(const char* begin, const char* end);
+      Position add(const char* begin, const char* end);
       Position inc(const char* begin, const char* end) const;
 
     public: // overload output stream operator