From 3b0f87c8877fd0f0721e7290c489f13b4674b593 Mon Sep 17 00:00:00 2001
From: Jose Luis Blanco-Claraco <joseluisblancoc@gmail.com>
Date: Tue, 4 Feb 2025 12:51:44 +0100
Subject: [PATCH] FIX: parser bug; it should not try to parse commented-out env
 variables

---
 mola_yaml/src/yaml_helpers.cpp       | 20 +++++++++++++
 mola_yaml/tests/test-yaml-parser.cpp | 43 ++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/mola_yaml/src/yaml_helpers.cpp b/mola_yaml/src/yaml_helpers.cpp
index a5c08bf8..5f848a93 100644
--- a/mola_yaml/src/yaml_helpers.cpp
+++ b/mola_yaml/src/yaml_helpers.cpp
@@ -88,9 +88,23 @@ std::string mola::yaml_to_string(const mrpt::containers::yaml& cfg)
 
 namespace
 {
+
+bool line_pos_is_commented_out(const std::string& text, size_t pos)
+{
+    for (;;)
+    {
+        if (text[pos] == '#') return true;
+        if (!pos || text[pos] == '\n' || text[pos] == '\r') break;
+        pos--;
+    }
+    return false;
+}
+
 std::string parseVars(
     const std::string& text, const mola::YAMLParseOptions& opts)
 {
+    using namespace std::string_literals;
+
     MRPT_TRY_START
 
     const auto start = text.find("${");
@@ -111,6 +125,12 @@ std::string parseVars(
 
     const auto [varname, defaultValue] = splitVerticalBar(varnameOrg);
 
+    if (line_pos_is_commented_out(text, start))
+    {
+        return parseVars(
+            pre + "$ {"s + varname + "}"s + post.substr(post_end + 1), opts);
+    }
+
     // 1st try:  match to env vars
     std::string varvalue;
     const char* v = ::getenv(varname.c_str());
diff --git a/mola_yaml/tests/test-yaml-parser.cpp b/mola_yaml/tests/test-yaml-parser.cpp
index 39f4d92a..626c0af3 100644
--- a/mola_yaml/tests/test-yaml-parser.cpp
+++ b/mola_yaml/tests/test-yaml-parser.cpp
@@ -48,6 +48,16 @@ b: "foo"
 e: '${foo|default1}'
 )###";
 
+const std::string txt2 = R"###(# sample yaml
+a: 3
+f: '${zoo}'
+)###";
+
+const std::string txt3 = R"###(# sample yaml
+a: 3
+b: 3  #f: '${zoo}'
+)###";
+
 void test_parseSimple()
 {
     {
@@ -86,6 +96,36 @@ void test_parseCustomVars()
         ASSERT_(y.isMap());
         ASSERT_EQUAL_(y["e"].as<std::string>(), se);
     }
+
+    // catch undefined variables:
+    {
+        bool did_throw = false;
+        try
+        {
+            const auto y =
+                mola::parse_yaml(mrpt::containers::yaml::FromText(txt2));
+        }
+        catch (const std::exception&)
+        {
+            did_throw = true;
+        }
+        ASSERT_(did_throw);
+    }
+
+    // do not throw if they are commented out:
+    {
+        bool did_throw = false;
+        try
+        {
+            const auto y =
+                mola::parse_yaml(mrpt::containers::yaml::FromText(txt3));
+        }
+        catch (const std::exception&)
+        {
+            did_throw = true;
+        }
+        ASSERT_(!did_throw);
+    }
 }
 
 void test_parseIncludes()
@@ -130,9 +170,6 @@ void test_parseIncludes()
 
 }  // namespace
 
-MRPT_TODO("Possible bug: #$include{} shouldn't be parsed")
-MRPT_TODO("bug: #${var} shouldn't be parsed")
-
 int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv)
 {
     try