Remove Sass_List_Delimiter in favour of an is_bracketed property

The list delimiter property only existed as a way to preserve parser state on the node in order to reused the existing `parse_list` function. This PR instead adds a `parse_bracketed_list` function to explicitly parse bracketed lists and sets the `is_bracketed` property on the `List` node. It turns out this is exactly what Ruby Sass does.
sass · Jan 4, 2017 · 83ec7e7 · 83ec7e7
1 parent 46f5244
commit 83ec7e7
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 37 deletions.
diff --git a/include/sass/values.h b/include/sass/values.h
@@ -35,13 +35,6 @@ enum Sass_Separator {
   SASS_HASH
 };
 
-  // Tags for denoting Sass list delimiters
-enum Sass_List_Delimiter {
-  SASS_NO_DELIMITER,
-  SASS_PARENTHESIS,
-  SASS_BRACKETS
-};
-
 // Value Operators
 enum Sass_OP {
   AND, OR,                   // logical connectives

diff --git a/src/ast.hpp b/src/ast.hpp
@@ -1047,25 +1047,24 @@ namespace Sass {
   private:
     ADD_PROPERTY(enum Sass_Separator, separator)
     ADD_PROPERTY(bool, is_arglist)
-    ADD_PROPERTY(enum Sass_List_Delimiter, delimiter)
+    ADD_PROPERTY(bool, is_bracketed)
     ADD_PROPERTY(bool, from_selector)
   public:
     List(ParserState pstate,
-         size_t size = 0, enum Sass_Separator sep = SASS_SPACE, bool argl = false,
-         enum Sass_List_Delimiter delimiter = SASS_NO_DELIMITER)
+         size_t size = 0, enum Sass_Separator sep = SASS_SPACE, bool argl = false, bool bracket = false)
     : Value(pstate),
       Vectorized<Expression_Obj>(size),
       separator_(sep),
       is_arglist_(argl),
-      delimiter_(delimiter),
+      is_bracketed_(bracket),
       from_selector_(false)
     { concrete_type(LIST); }
     List(const List* ptr)
     : Value(ptr),
       Vectorized<Expression_Obj>(*ptr),
       separator_(ptr->separator_),
       is_arglist_(ptr->is_arglist_),
-      delimiter_(ptr->delimiter_),
+      is_bracketed_(ptr->is_bracketed_),
       from_selector_(ptr->from_selector_)
     { concrete_type(LIST); }
     std::string type() { return is_arglist_ ? "arglist" : "list"; }
@@ -1074,7 +1073,6 @@ namespace Sass {
       return separator() == SASS_SPACE ?
         " " : (compressed ? "," : ", ");
     }
-    bool is_bracketed() const { return delimiter() == SASS_BRACKETS; }
     bool is_invisible() const { return empty() && !is_bracketed(); }
     Expression_Obj value_at_index(size_t i);
 

diff --git a/src/debugger.hpp b/src/debugger.hpp
@@ -595,11 +595,11 @@ inline void debug_ast(AST_Node_Ptr node, std::string ind, Env* env)
     std::cerr << " (" << pstate_source_position(node) << ")";
     std::cerr << " (" << expression->length() << ") " <<
       (expression->separator() == SASS_COMMA ? "Comma " : expression->separator() == SASS_HASH ? "Map " : "Space ") <<
-      (expression->delimiter() == SASS_PARENTHESIS ? "Parenthesis " : expression->delimiter() == SASS_BRACKETS ? "Bracket " : "None ") <<
       " [delayed: " << expression->is_delayed() << "] " <<
       " [interpolant: " << expression->is_interpolant() << "] " <<
       " [listized: " << expression->from_selector() << "] " <<
       " [arglist: " << expression->is_arglist() << "] " <<
+      " [bracketed: " << expression->is_bracketed() << "] " <<
       " [expanded: " << expression->is_expanded() << "] " <<
       " [hash: " << expression->hash() << "] " <<
       std::endl;

diff --git a/src/eval.cpp b/src/eval.cpp
@@ -462,7 +462,7 @@ namespace Sass {
                                l->length(),
                                l->separator(),
                                l->is_arglist(),
-                               l->delimiter());
+                               l->is_bracketed());
     for (size_t i = 0, L = l->length(); i < L; ++i) {
       ll->append((*l)[i]->perform(this));
     }

diff --git a/src/functions.cpp b/src/functions.cpp
@@ -1304,7 +1304,7 @@ namespace Sass {
       if (l->empty()) error("argument `$list` of `" + std::string(sig) + "` must not be empty", pstate);
       double index = std::floor(n->value() < 0 ? l->length() + n->value() : n->value() - 1);
       if (index < 0 || index > l->length() - 1) error("index out of bounds for `" + std::string(sig) + "`", pstate);
-      List_Ptr result = SASS_MEMORY_NEW(List, pstate, l->length(), l->separator(), false, l->delimiter());
+      List_Ptr result = SASS_MEMORY_NEW(List, pstate, l->length(), l->separator(), false, l->is_bracketed());
       for (size_t i = 0, L = l->length(); i < L; ++i) {
         result->append(((i == index) ? v : (*l)[i]));
       }
@@ -1340,12 +1340,12 @@ namespace Sass {
       String_Constant_Obj sep = ARG("$separator", String_Constant);
       enum Sass_Separator sep_val = (l1 ? l1->separator() : SASS_SPACE);
       Value* bracketed = ARG("$bracketed", Value);
-      enum Sass_List_Delimiter delimiter = (l1 ? l1->delimiter() : SASS_NO_DELIMITER);
+      bool is_bracketed = (l1 ? l1->is_bracketed() : false);
       if (!l1) {
         l1 = SASS_MEMORY_NEW(List, pstate, 1);
         l1->append(ARG("$list1", Expression));
         sep_val = (l2 ? l2->separator() : SASS_SPACE);
-        delimiter = (l2 ? l2->delimiter() : SASS_NO_DELIMITER);
+        is_bracketed = (l2 ? l2->is_bracketed() : false);
       }
       if (!l2) {
         l2 = SASS_MEMORY_NEW(List, pstate, 1);
@@ -1366,9 +1366,9 @@ namespace Sass {
       String_Constant_Obj bracketed_as_str = SASS_MEMORY_CAST_PTR(String_Constant, bracketed);
       bool bracketed_is_auto = bracketed_as_str && unquote(bracketed_as_str->value()) == "auto";
       if (!bracketed_is_auto) {
-        delimiter = bracketed->is_false() ? SASS_NO_DELIMITER : SASS_BRACKETS;
+        is_bracketed = !bracketed->is_false();
       }
-      List_Obj result = SASS_MEMORY_NEW(List, pstate, len, sep_val, false, delimiter);
+      List_Obj result = SASS_MEMORY_NEW(List, pstate, len, sep_val, false, is_bracketed);
       result->concat(&l1);
       result->concat(&l2);
       return result.detach();

diff --git a/src/parser.cpp b/src/parser.cpp
@@ -1000,13 +1000,8 @@ namespace Sass {
     List_Obj map = SASS_MEMORY_NEW(List, pstate, 0, SASS_HASH);
 
     // it's not a map so return the lexed value as a list value
-    if (!lex_css< exactly<':'> >()) {
-      List_Obj list = SASS_MEMORY_CAST(List, key);
-      if (list && list->delimiter() == SASS_NO_DELIMITER) {
-        list->delimiter(SASS_PARENTHESIS);
-      }
-      return key;
-    }
+    if (!lex_css< exactly<':'> >())
+    { return key; }
 
     Expression_Obj value = parse_space_list();
 
@@ -1037,6 +1032,62 @@ namespace Sass {
     return &map;
   }
 
+  Expression_Obj Parser::parse_bracketed_list()
+  {
+    // check if we have an empty list
+    // return the empty list as such
+    if (peek_css<
+          exactly<']'>
+       >(position))
+    {
+      // return an empty list (nothing to delay)
+      return SASS_MEMORY_NEW(List, pstate, 0, SASS_SPACE, false, true);
+    }
+
+    bool has_paren = peek_css< exactly<'('> >();
+
+    // now try to parse a space list
+    Expression_Obj list = parse_space_list();
+    // if it's a singleton, return it (don't wrap it)
+    if (!peek_css< exactly<','> >(position)) {
+      List_Obj l = SASS_MEMORY_CAST(List, list);
+      if (!l || l->is_bracketed() || has_paren) {
+        List_Obj bracketed_list = SASS_MEMORY_NEW(List, pstate, 1, SASS_SPACE, false, true);
+        bracketed_list->append(&list);
+        return &bracketed_list;
+      }
+        l->is_bracketed(&list);
+        return &l;
+    }
+
+    // if we got so far, we actually do have a comma list
+    List_Obj bracketed_list = SASS_MEMORY_NEW(List, pstate, 2, SASS_COMMA, false, true);
+    // wrap the first expression
+    bracketed_list->append(list);
+
+    while (lex_css< exactly<','> >())
+    {
+      // check for abort condition
+      if (peek_css< alternatives <
+            exactly<';'>,
+            exactly<'}'>,
+            exactly<'{'>,
+            exactly<')'>,
+            exactly<']'>,
+            exactly<':'>,
+            end_of_file,
+            exactly<ellipsis>,
+            default_flag,
+            global_flag
+          > >(position)
+      ) { break; }
+      // otherwise add another expression
+      bracketed_list->append(parse_space_list());
+    }
+    // return the list
+    return &bracketed_list;
+  }
+
   // parse list returns either a space separated list,
   // a comma separated list or any bare expression found.
   // so to speak: we unwrap items from lists if possible here!
@@ -1339,18 +1390,10 @@ namespace Sass {
     }
     else if (lex_css< exactly<'['> >()) {
       // explicit bracketed
-      Expression_Obj value = parse_list();
+      Expression_Obj value = parse_bracketed_list();
       // lex the expected closing square bracket
       if (!lex_css< exactly<']'> >()) error("unclosed squared bracket", pstate);
-      // fix delimiter
-      List_Obj list = SASS_MEMORY_CAST(List, value);
-      if (!list || list->delimiter() != SASS_NO_DELIMITER) {
-        List_Ptr outer_list = SASS_MEMORY_NEW(List, pstate, 1, SASS_SPACE, false, SASS_BRACKETS);
-        outer_list->append(&value);
-        return outer_list;
-      }
-      list->delimiter(SASS_BRACKETS);
-      return value;
+      return &value;
     }
     // string may be interpolated
     // if (lex< quoted_string >()) {

diff --git a/src/parser.hpp b/src/parser.hpp
@@ -257,6 +257,7 @@ namespace Sass {
     bool parse_number_prefix();
     Declaration_Obj parse_declaration();
     Expression_Obj parse_map();
+    Expression_Obj parse_bracketed_list();
     Expression_Obj parse_list(bool delayed = false);
     Expression_Obj parse_comma_list(bool delayed = false);
     Expression_Obj parse_space_list();