Newlines: Parse \f and normalize in comments (#2849)

Also:

1. Moves the `normalize*` functions and `rtrim` to a separate object file,
   so that they can be unit tested quickly and easily.
2. Changes `string_to_output` to also replace `\r\n`, fixing Windows tests.
3. Improves other `normalize_*` functions and adds tests.

Refs #2843

sass-spec PRs:

1. Bug fixes to make it run: sass/sass-spec#1365
2. Enabling the newly passing tests: sass/sass-spec#1366

Makefile.conf

@@ -29,6 +29,7 @@ SOURCES = \
 	bind.cpp \
 	file.cpp \
 	util.cpp \
+	util_string.cpp \
 	json.cpp \
 	units.cpp \
 	values.cpp \

src/emitter.cpp

@@ -3,6 +3,7 @@
 #include "context.hpp"
 #include "output.hpp"
 #include "emitter.hpp"
+#include "util_string.hpp"
 #include "utf8_string.hpp"
 
 namespace Sass {
@@ -134,13 +135,13 @@ namespace Sass {
     // write space/lf
     flush_schedules();
 
-    if (in_comment && output_style() == COMPACT) {
-      // unescape comment nodes
-      std::string out = comment_to_string(text);
-      // add to buffer
-      wbuf.buffer += out;
-      // account for data in source-maps
+    if (in_comment) {
+      std::string out = Util::normalize_newlines(text);
+      if (output_style() == COMPACT) {
+        out = comment_to_compact_string(out);
+      }
       wbuf.smap.append(Offset(out));
+      wbuf.buffer += std::move(out);
     } else {
       // add to buffer
       wbuf.buffer += text;

src/eval.cpp

@@ -30,6 +30,7 @@
 #include "expand.hpp"
 #include "color_maps.hpp"
 #include "sass_functions.hpp"
+#include "util_string.hpp"
 
 namespace Sass {
 

src/fn_miscs.cpp

@@ -2,6 +2,7 @@
 #include "expand.hpp"
 #include "fn_utils.hpp"
 #include "fn_miscs.hpp"
+#include "util_string.hpp"
 
 namespace Sass {
 

src/fn_utils.cpp

@@ -4,6 +4,7 @@
 
 #include "parser.hpp"
 #include "fn_utils.hpp"
+#include "util_string.hpp"
 
 namespace Sass {
 

src/lexer.cpp

@@ -153,11 +153,11 @@ namespace Sass {
     // Match word boundary (zero-width lookahead).
     const char* word_boundary(const char* src) { return is_character(*src) || *src == '#' ? 0 : src; }
 
-    // Match linefeed /(?:\n|\r\n?)/
+    // Match linefeed /(?:\n|\r\n?|\f)/
     const char* re_linebreak(const char* src)
     {
       // end of file or unix linefeed return here
-      if (*src == 0 || *src == '\n') return src + 1;
+      if (*src == 0 || *src == '\n' || *src == '\f') return src + 1;
       // a carriage return may optionally be followed by a linefeed
       if (*src == '\r') return *(src + 1) == '\n' ? src + 2 : src + 1;
       // no linefeed
@@ -169,7 +169,7 @@ namespace Sass {
     const char* end_of_line(const char* src)
     {
       // end of file or unix linefeed return here
-      return *src == 0 || *src == '\n' || *src == '\r' ? src : 0;
+      return *src == 0 || *src == '\n' || *src == '\r' || *src == '\f' ? src : 0;
     }
 
     // Assert end_of_file boundary (/\z/)

src/output.cpp

@@ -92,7 +92,6 @@ namespace Sass {
 
   void Output::operator()(Comment* c)
   {
-    std::string txt = c->text()->to_string(opt);
     // if (indentation && txt == "/**/") return;
     bool important = c->is_important();
     if (output_style() != COMPRESSED || important) {

src/parser.cpp

@@ -11,6 +11,7 @@
 #include "color_maps.hpp"
 #include "sass/functions.h"
 #include "error_handling.hpp"
+#include "util_string.hpp"
 
 // Notes about delayed: some ast nodes can have delayed evaluation so
 // they can preserve their original semantics if needed. This is most

src/prelexer.cpp

@@ -263,7 +263,7 @@ namespace Sass {
       >(src);
     }
 
-    // Match a line comment (/.*?(?=\n|\r\n?|\Z)/.
+    // Match a line comment (/.*?(?=\n|\r\n?|\f|\Z)/.
     const char* line_comment(const char* src)
     {
       return sequence<

src/util.cpp

@@ -163,42 +163,64 @@ namespace Sass {
     std::replace(str.begin(), str.end(), '\n', ' ');
   }
 
-  // bell characters are replaced with spaces
-  // also eats spaces after line-feeds (ltrim)
+  // 1. Removes whitespace after newlines.
+  // 2. Replaces newlines with spaces.
+  //
+  // This method only considers LF and CRLF as newlines.
   std::string string_to_output(const std::string& str)
   {
-    std::string out("");
-    bool lf = false;
-    for (auto i : str) {
-      if (i == '\n') {
-        out += ' ';
-        lf = true;
-      } else if (!(lf && isspace(i))) {
-        out += i;
-        lf = false;
+    std::string result;
+    result.reserve(str.size());
+    std::size_t pos = 0;
+    while (true) {
+      const std::size_t newline = str.find_first_of("\n\r", pos);
+      if (newline == std::string::npos) break;
+      result.append(str, pos, newline - pos);
+      if (str[newline] == '\r') {
+        if (str[newline + 1] == '\n') {
+          pos = newline + 2;
+        } else {
+          // CR without LF: append as-is and continue.
+          result += '\r';
+          pos = newline + 1;
+          continue;
+        }
+      } else {
+        pos = newline + 1;
+      }
+      result += ' ';
+      const std::size_t non_space = str.find_first_not_of(" \f\n\r\t\v", pos);
+      if (non_space != std::string::npos) {
+        pos = non_space;
       }
     }
-    return out;
+    result.append(str, pos, std::string::npos);
+    return result;
   }
 
   std::string escape_string(const std::string& str)
   {
-    std::string out("");
-    for (auto i : str) {
-      if (i == '\n') {
-        out += "\\n";
-      } else if (i == '\r') {
-        out += "\\r";
-      } else if (i == '\t') {
-        out += "\\t";
-      } else {
-        out += i;
+    std::string out;
+    out.reserve(str.size());
+    for (char c : str) {
+      switch (c) {
+        case '\n':
+          out.append("\\n");
+          break;
+        case '\r':
+          out.append("\\r");
+          break;
+        case '\f':
+          out.append("\\f");
+          break;
+        default:
+          out += c;
       }
     }
     return out;
   }
 
-  std::string comment_to_string(const std::string& text)
+  std::string comment_to_compact_string(const std::string& text)
   {
     std::string str = "";
     size_t has = 0;
@@ -207,7 +229,6 @@ namespace Sass {
     for (auto i : text) {
       if (clean) {
         if (i == '\n') { has = 0; }
-        else if (i == '\r') { has = 0; }
         else if (i == '\t') { ++ has; }
         else if (i == ' ') { ++ has; }
         else if (i == '*') {}
@@ -219,8 +240,6 @@ namespace Sass {
         }
       } else if (i == '\n') {
         clean = true;
-      } else if (i == '\r') {
-        clean = true;
       } else {
         str += i;
       }
@@ -508,33 +527,6 @@ namespace Sass {
   }
 
   namespace Util {
-    using std::string;
-
-    std::string rtrim(const std::string &str) {
-      std::string trimmed = str;
-      size_t pos_ws = trimmed.find_last_not_of(" \t\n\v\f\r");
-      if (pos_ws != std::string::npos)
-      { trimmed.erase(pos_ws + 1); }
-      else { trimmed.clear(); }
-      return trimmed;
-    }
-
-    std::string normalize_underscores(const std::string& str) {
-      std::string normalized = str;
-      for(size_t i = 0, L = normalized.length(); i < L; ++i) {
-        if(normalized[i] == '_') {
-          normalized[i] = '-';
-        }
-      }
-      return normalized;
-    }
-
-    std::string normalize_decimals(const std::string& str) {
-      std::string prefix = "0";
-      std::string normalized = str;
-
-      return normalized[0] == '.' ? normalized.insert(0, prefix) : normalized;
-    }
 
     bool isPrintable(Ruleset* r, Sass_Output_Style style) {
       if (r == NULL) {

src/util.hpp

@@ -38,7 +38,7 @@ namespace Sass {
   std::string read_css_string(const std::string& str, bool css = true);
   std::string evacuate_escapes(const std::string& str);
   std::string string_to_output(const std::string& str);
-  std::string comment_to_string(const std::string& text);
+  std::string comment_to_compact_string(const std::string& text);
   std::string read_hex_escapes(const std::string& str);
   std::string escape_string(const std::string& str);
   void newline_to_space(std::string& str);
@@ -91,11 +91,6 @@ namespace Sass {
 
   namespace Util {
 
-    std::string rtrim(const std::string& str);
-
-    std::string normalize_underscores(const std::string& str);
-    std::string normalize_decimals(const std::string& str);
-
     bool isPrintable(Ruleset* r, Sass_Output_Style style = NESTED);
     bool isPrintable(Supports_Block* r, Sass_Output_Style style = NESTED);
     bool isPrintable(Media_Block* r, Sass_Output_Style style = NESTED);

src/util_string.cpp

@@ -0,0 +1,57 @@
+#include "util_string.hpp"
+
+#include <algorithm>
+
+namespace Sass {
+namespace Util {
+
+std::string rtrim(const std::string &str) {
+  std::string trimmed = str;
+  size_t pos_ws = trimmed.find_last_not_of(" \t\n\v\f\r");
+  if (pos_ws != std::string::npos) {
+    trimmed.erase(pos_ws + 1);
+  } else {
+    trimmed.clear();
+  }
+  return trimmed;
+}
+
+std::string normalize_newlines(const std::string& str) {
+  std::string result;
+  result.reserve(str.size());
+  std::size_t pos = 0;
+  while (true) {
+    const std::size_t newline = str.find_first_of("\n\f\r", pos);
+    if (newline == std::string::npos) break;
+    result.append(str, pos, newline - pos);
+    result += '\n';
+    if (str[newline] == '\r' && str[newline + 1] == '\n') {
+      pos = newline + 2;
+    } else {
+      pos = newline + 1;
+    }
+  }
+  result.append(str, pos, std::string::npos);
+  return result;
+}
+
+std::string normalize_underscores(const std::string& str) {
+  std::string normalized = str;
+  std::replace(normalized.begin(), normalized.end(), '_', '-');
+  return normalized;
+}
+
+std::string normalize_decimals(const std::string& str) {
+  std::string normalized;
+  if (!str.empty() && str[0] == '.') {
+    normalized.reserve(str.size() + 1);
+    normalized += '0';
+    normalized += str;
+  } else {
+    normalized = str;
+  }
+  return normalized;
+}
+
+}  // namespace Sass
+}  // namespace Util

src/util_string.hpp

@@ -0,0 +1,17 @@
+#ifndef SASS_UTIL_STRING_H
+#define SASS_UTIL_STRING_H
+
+#include <string>
+
+namespace Sass {
+namespace Util {
+
+std::string rtrim(const std::string& str);
+
+std::string normalize_newlines(const std::string& str);
+std::string normalize_underscores(const std::string& str);
+std::string normalize_decimals(const std::string& str);
+
+}  // namespace Sass
+}  // namespace Util
+#endif  // SASS_UTIL_STRING_H

test/Makefile

@@ -1,16 +1,22 @@
 CXX ?= c++
 CXXFLAGS := -I ../include/ -std=c++11 -fsanitize=address -g -O1 -fno-omit-frame-pointer
 
-test: test_shared_ptr
+test: test_shared_ptr test_util_string
 
 test_shared_ptr: build/test_shared_ptr
 	@ASAN_OPTIONS="symbolize=1" build/test_shared_ptr
 
+test_util_string: build/test_util_string
+	@ASAN_OPTIONS="symbolize=1" build/test_util_string
+
 build:
 	@mkdir build
 
-build/test_shared_ptr: | build
-	@$(CXX) $(CXXFLAGS) -o build/test_shared_ptr test_shared_ptr.cpp ../src/memory/SharedPtr.cpp
+build/test_shared_ptr: test_shared_ptr.cpp ../src/memory/SharedPtr.cpp | build
+	$(CXX) $(CXXFLAGS) -o build/test_shared_ptr test_shared_ptr.cpp ../src/memory/SharedPtr.cpp
+
+build/test_util_string: test_util_string.cpp ../src/util_string.cpp | build
+	$(CXX) $(CXXFLAGS) -o build/test_util_string test_util_string.cpp ../src/util_string.cpp
 
 clean: | build
 	rm -rf build