From 650c6140bd92e366734f5bcb8e33feea82309115 Mon Sep 17 00:00:00 2001
From: Jean Pierre Cimalando <jp-dev@inbox.ru>
Date: Thu, 14 May 2020 20:48:08 +0200
Subject: [PATCH] parser: dollar expansions and multiple #define on the same
 line

---
 src/sfizz/parser/Parser.cpp | 23 ++++++++++---
 tests/FilesT.cpp            | 10 +++++-
 tests/ParsingT.cpp          | 66 +++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp
index 455c0aec..2c349e20 100644
--- a/src/sfizz/parser/Parser.cpp
+++ b/src/sfizz/parser/Parser.cpp
@@ -163,7 +163,18 @@ void Parser::processDirective()
 
         std::string value;
         extractToEol(reader, &value);
+
+#if 1
+        // ARIA/not Cakewalk: cut the value after the first word
+        size_t position = value.find_first_of(" \t");
+        if (position != value.npos) {
+            absl::string_view excess(&value[position], value.size() - position);
+            reader.putBackChars(excess);
+            value.resize(position);
+        }
+#else
         trimRight(value);
+#endif
 
         addDefinition(id, value);
     }
@@ -457,21 +468,25 @@ std::string Parser::expandDollarVars(const SourceRange& range, absl::string_view
             std::string name;
             name.reserve(64);
 
-            while (i < n && isIdentifierChar(src[i]))
+            // ARIA: we will accumulate any chars after $, until this is the
+            //       name of a known variable
+            auto def = _currentDefinitions.end();
+            while (i < n && isIdentifierChar(src[i]) && def == _currentDefinitions.end()) {
                 name.push_back(src[i++]);
+                def = _currentDefinitions.find(name);
+            }
 
             if (name.empty()) {
                 emitWarning(range, "Expected variable name after $.");
                 continue;
             }
 
-            auto it = _currentDefinitions.find(name);
-            if (it == _currentDefinitions.end()) {
+            if (def == _currentDefinitions.end()) {
                 emitWarning(range, "The variable `" + name + "` is not defined.");
                 continue;
             }
 
-            dst.append(it->second);
+            dst.append(def->second);
         }
     }
 
diff --git a/tests/FilesT.cpp b/tests/FilesT.cpp
index dbfaea38..0ae8f068 100644
--- a/tests/FilesT.cpp
+++ b/tests/FilesT.cpp
@@ -339,14 +339,22 @@ TEST_CASE("[Files] sw_default and playing with switches")
     REQUIRE( synth.getRegionView(3)->isSwitchedOn() );
 }
 
-
 TEST_CASE("[Files] wrong (overlapping) replacement for defines")
 {
     sfz::Synth synth;
     synth.loadSfzFile(fs::current_path() / "tests/TestFiles/SpecificBugs/wrong-replacements.sfz");
+
     REQUIRE( synth.getNumRegions() == 3 );
+
+#if 0
+    // Note: test checked to be wrong under Sforzando 1.961
+    //       It is the shorter matching $-variable which matches among both.
+    //       The rest of the variable name creates some trailing junk text
+    //       which Sforzando accepts without warning. (eg. `key=52Edge`)
     REQUIRE( synth.getRegionView(0)->keyRange.getStart() == 52 );
     REQUIRE( synth.getRegionView(0)->keyRange.getEnd() == 52 );
+#endif
+
     REQUIRE( synth.getRegionView(1)->keyRange.getStart() == 57 );
     REQUIRE( synth.getRegionView(1)->keyRange.getEnd() == 57 );
     REQUIRE(!synth.getRegionView(2)->amplitudeCC.empty());
diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp
index 84031586..b259e4a7 100644
--- a/tests/ParsingT.cpp
+++ b/tests/ParsingT.cpp
@@ -523,3 +523,69 @@ param3=baz param4=quux /* block comment */)");
         REQUIRE(mock.fullBlockHeaders == expectedHeaders);
         REQUIRE(mock.fullBlockMembers == expectedMembers);
 }
+
+TEST_CASE("[Parsing] Overlapping definition identifiers")
+{
+        sfz::Parser parser;
+        ParsingMocker mock;
+        parser.setListener(&mock);
+        parser.parseString("/overlappingDefinitionIdentifiers.sfz",
+R"(#define $abc foo
+#define $abcdef bar
+<region> sample=$abc.wav
+<region> sample=$abcdef.wav)");
+
+        std::vector<std::vector<sfz::Opcode>> expectedMembers = {
+            {{"sample", "foo.wav"}},
+            {{"sample", "foodef.wav"}},
+        };
+        std::vector<std::string> expectedHeaders = {
+            "region", "region"
+        };
+        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);
+}
+
+TEST_CASE("[Parsing] Interpretation of the value of #define")
+{
+        sfz::Parser parser;
+        ParsingMocker mock;
+        parser.setListener(&mock);
+        parser.parseString("/defineValues.sfz",
+R"(#define $a foo #define $b bar <region> sample=$a-$b.wav
+<region>#define $c toto sample=$c.wav)");
+
+        std::vector<std::vector<sfz::Opcode>> expectedMembers = {
+            {{"sample", "foo-bar.wav"}},
+            {{"sample", "toto.wav"}},
+        };
+        std::vector<std::string> expectedHeaders = {
+            "region", "region"
+        };
+        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);
+}