Merge pull request #196 from jpcima/block-comments

Support block comments
sfztools · Apr 20, 2020 · c2767b3 · c2767b3
2 parents 1dc83d3 + 353cea9
commit c2767b3
Show file tree

Hide file tree

Showing 4 changed files with 157 additions and 15 deletions.
diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp
@@ -7,6 +7,7 @@
 #include "Parser.h"
 #include "ParserPrivate.h"
 #include "absl/memory/memory.h"
+#include <cassert>
 
 namespace sfz {
 
@@ -113,7 +114,7 @@ void Parser::processTopLevel()
     while (!_included.empty()) {
         Reader& reader = *_included.back();
 
-        while (reader.skipChars(" \t\r\n") || skipComment(reader));
+        while (reader.skipChars(" \t\r\n") || skipComment());
 
         switch (reader.peekChar()) {
         case Reader::kEof:
@@ -348,32 +349,74 @@ void Parser::flushCurrentHeader()
     _currentOpcodes.clear();
 }
 
-bool Parser::hasComment(Reader& reader)
+Parser::CommentType Parser::getCommentType(Reader& reader)
 {
     if (reader.peekChar() != '/')
-        return false;
+        return CommentType::None;
 
     reader.getChar();
-    if (reader.peekChar() != '/') {
-        reader.putBackChar('/');
-        return false;
+
+    CommentType ret = CommentType::None;
+
+    switch (reader.peekChar()) {
+    case '/':
+        ret = CommentType::Line;
+        break;
+    case '*':
+        ret = CommentType::Block;
+        break;
     }
 
-    return true;
+    reader.putBackChar('/');
+    return ret;
 }
 
-size_t Parser::skipComment(Reader& reader)
+size_t Parser::skipComment()
 {
-    if (!hasComment(reader))
+    Reader& reader = *_included.back();
+
+    const CommentType commentType = getCommentType(reader);
+    if (commentType == CommentType::None)
         return 0;
 
+    SourceLocation start = reader.location();
+
     size_t count = 2;
     reader.getChar();
     reader.getChar();
 
-    int c;
-    while ((c = reader.getChar()) != Reader::kEof && c != '\r' && c != '\n')
-        ++count;
+    bool terminated = false;
+
+    switch (commentType) {
+    case CommentType::Line:
+        while (!terminated) {
+            int c = reader.getChar();
+            count += (c != Reader::kEof);
+            terminated = c == Reader::kEof || c == '\r' || c == '\n';
+        }
+        break;
+    case CommentType::Block:
+        {
+            int c1 = 0;
+            int c2 = reader.getChar();
+            count += (c2 != Reader::kEof);
+            while (!terminated && c2 != Reader::kEof) {
+                c1 = c2;
+                c2 = reader.getChar();
+                count += (c2 != Reader::kEof);
+                terminated = c1 == '*' && c2 == '/';
+            }
+        }
+        break;
+    default:
+        assert(false);
+        break;
+    }
+
+    if (!terminated) {
+        SourceLocation end = reader.location();
+        emitError({ start, end }, "Unterminated block comment.");
+    }
 
     return count;
 }
@@ -387,7 +430,14 @@ void Parser::trimRight(std::string& text)
 size_t Parser::extractToEol(Reader& reader, std::string* dst)
 {
     return reader.extractWhile(dst, [&reader](char c) {
-        return c != '\r' && c != '\n' && !(c == '/' && reader.peekChar() == '/');
+        if (c == '\r' || c == '\n')
+            return false;
+        if (c == '/') {
+            int c2 = reader.peekChar();
+            if (c2 == '/' || c2 == '*') // stop at comment
+                return false;
+        }
+        return true;
     });
 }
 

diff --git a/src/sfizz/parser/Parser.h b/src/sfizz/parser/Parser.h
@@ -84,8 +84,14 @@ class Parser {
     void flushCurrentHeader();
 
     // helpers
-    static bool hasComment(Reader& reader);
-    static size_t skipComment(Reader& reader);
+    enum class CommentType {
+        None,
+        Line,
+        Block,
+    };
+
+    static CommentType getCommentType(Reader& reader);
+    size_t skipComment();
     static void trimRight(std::string& text);
     static size_t extractToEol(Reader& reader, std::string* dst); // ignores comment
     std::string expandDollarVars(const SourceRange& range, absl::string_view src);

diff --git a/tests/DemoParser.cpp b/tests/DemoParser.cpp
@@ -38,6 +38,11 @@ static const char defaultSfzText[] = R"SFZ(
 // This is a SFZ test file with many problems.                                //
 //----------------------------------------------------------------------------//
 
+/*
+ * This is a block comment. Not all the SFZ players accept it.
+ * It can span over multiple lines.
+*/
+
 // opcode without header
 not_in_header=on    // warning
 
@@ -75,6 +80,12 @@ abcdef=$tata
 
 // opcode name which expands to invalid identifier
 $titi=1
+
+volume=10 /*
+block comments at the end of line
+*/
+
+/* unterminated block comment
 )SFZ";
 
 void Application::init()

diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp
@@ -448,3 +448,78 @@ param2=$bar)");
         REQUIRE(mock.fullBlockHeaders == expectedHeaders);
         REQUIRE(mock.fullBlockMembers == expectedMembers);
 }
+
+TEST_CASE("[Parsing] Block comments")
+{
+        sfz::Parser parser;
+        ParsingMocker mock;
+        parser.setListener(&mock);
+        parser.parseString("/blockComments.sfz",
+R"(/* A block comment (1) */
+/*
+A block comment (2) */
+/* A block comment (3)
+*/
+/* A block comment
+  (4) */
+/* A block comment /* // ** (5) */
+)");
+        REQUIRE(mock.beginnings == 1);
+        REQUIRE(mock.endings == 1);
+        REQUIRE(mock.errors.empty());
+        REQUIRE(mock.warnings.empty());
+        REQUIRE(mock.opcodes.empty());
+        REQUIRE(mock.headers.empty());
+        REQUIRE(mock.fullBlockHeaders.empty());
+        REQUIRE(mock.fullBlockMembers.empty());
+}
+
+TEST_CASE("[Parsing] Unterminated block comments")
+{
+        sfz::Parser parser;
+        ParsingMocker mock;
+        parser.setListener(&mock);
+        parser.parseString("/unterminatedBlockComment.sfz",
+R"(/* Unterminated block comment
+)");
+        REQUIRE(mock.beginnings == 1);
+        REQUIRE(mock.endings == 1);
+        REQUIRE(mock.errors.size() == 1);
+        REQUIRE(mock.warnings.empty());
+        REQUIRE(mock.opcodes.empty());
+        REQUIRE(mock.headers.empty());
+        REQUIRE(mock.fullBlockHeaders.empty());
+        REQUIRE(mock.fullBlockMembers.empty());
+}
+
+TEST_CASE("[Parsing] Comments after values")
+{
+        sfz::Parser parser;
+        ParsingMocker mock;
+        parser.setListener(&mock);
+        parser.parseString("/commentsAfterValues.sfz",
+R"(<header>
+param1=foo param2=bar // line comment
+param3=baz param4=quux /* block comment */)");
+        std::vector<std::vector<sfz::Opcode>> expectedMembers = {
+            {{"param1", "foo"}, {"param2", "bar"},
+             {"param3", "baz"}, {"param4", "quux"}}
+        };
+        std::vector<std::string> expectedHeaders = {
+            "header"
+        };
+        std::vector<sfz::Opcode> expectedOpcodes;
+
+        for (auto& members: expectedMembers)
+            for (auto& opcode: members)
+                expectedOpcodes.push_back(opcode);
+
+        REQUIRE(mock.beginnings == 1);
+        REQUIRE(mock.endings == 1);
+        REQUIRE(mock.errors.empty());
+        REQUIRE(mock.warnings.empty());
+        REQUIRE(mock.opcodes == expectedOpcodes);
+        REQUIRE(mock.headers == expectedHeaders);
+        REQUIRE(mock.fullBlockHeaders == expectedHeaders);
+        REQUIRE(mock.fullBlockMembers == expectedMembers);
+}