Fix parsing 'type' attibutes in plugins (#809)

Signed-off-by: Louise Poubel <[email protected]> * Add line with unrecognized type Signed-off-by: Steve Peters <[email protected]> * Fix for custom element with attribute 'type' (#811) * Reverted changes to parser for custom element with attribute 'type', added fix in ParamPrivate::ValueFromStringImpl instead Signed-off-by: Aaron Chong <[email protected]> Co-authored-by: Steve Peters <[email protected]> Co-authored-by: Aaron Chong <[email protected]>
gazebosim · Jan 3, 2022 · 6404afd · 6404afd
1 parent 8785b7a
commit 6404afd
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 24 deletions.
diff --git a/src/Param.cc b/src/Param.cc
@@ -640,14 +640,17 @@ bool ParamPrivate::ValueFromStringImpl(const std::string &_typeName,
   std::string tmp(trimmed);
   std::string lowerTmp = lowercase(trimmed);
 
-  // "true" and "false" doesn't work properly
-  if (lowerTmp == "true")
+  // "true" and "false" doesn't work properly (except for string)
+  if (_typeName != "string" && _typeName != "std::string")
   {
-    tmp = "1";
-  }
-  else if (lowerTmp == "false")
-  {
-    tmp = "0";
+    if (lowerTmp == "true")
+    {
+      tmp = "1";
+    }
+    else if (lowerTmp == "false")
+    {
+      tmp = "0";
+    }
   }
 
   bool isHex = lowerTmp.compare(0, 2, "0x") == 0;

diff --git a/src/Plugin_TEST.cc b/src/Plugin_TEST.cc
@@ -186,7 +186,8 @@ TEST(DOMPlugin, LoadWithChildren)
   std::string pluginStr = R"(<plugin name='3D View' filename='MinimalScene'>
   <ignition-gui>
     <title>3D View</title>
-    <property type='bool' key='showTitleBar'>false</property>
+    <property type='Ignition.Msgs.Boolean'>false</property>
+    <property type='bool' key='showTitleBar'>0</property>
     <property type='string' key='state'>docked</property>
   </ignition-gui>
   <engine>ogre</engine>

diff --git a/src/parser.cc b/src/parser.cc
@@ -1912,13 +1912,13 @@ void copyChildren(ElementPtr _sdf,
   for (elemXml = _xml->FirstChildElement(); elemXml;
        elemXml = elemXml->NextSiblingElement())
   {
-    std::string elem_name = elemXml->Name();
+    std::string elemName = elemXml->Name();
 
-    if (_sdf->HasElementDescription(elem_name))
+    if (_sdf->HasElementDescription(elemName))
     {
       if (!_onlyUnknown)
       {
-        sdf::ElementPtr element = _sdf->AddElement(elem_name);
+        sdf::ElementPtr element = _sdf->AddElement(elemName);
 
         // FIXME: copy attributes
         for (const auto *attribute = elemXml->FirstAttribute();
@@ -1941,26 +1941,18 @@ void copyChildren(ElementPtr _sdf,
     {
       ElementPtr element(new Element);
       element->SetParent(_sdf);
-      element->SetName(elem_name);
-      std::optional<std::string> typeName = std::nullopt;
+      element->SetName(elemName);
       for (const tinyxml2::XMLAttribute *attribute = elemXml->FirstAttribute();
            attribute; attribute = attribute->Next())
       {
-        const std::string attributeName(attribute->Name());
-        if (attributeName == "type")
-          typeName = attribute->Value();
-
-        element->AddAttribute(attributeName, "string", "", 1, "");
-        element->GetAttribute(attributeName)->SetFromString(
-          attribute->Value());
+        element->AddAttribute(attribute->Name(), "string", "", 1, "");
+        element->GetAttribute(attribute->Name())->SetFromString(
+            attribute->Value());
       }
 
       if (elemXml->GetText() != nullptr)
       {
-        if (typeName.has_value())
-          element->AddValue(typeName.value(), elemXml->GetText(), true);
-        else
-          element->AddValue("string", elemXml->GetText(), true);
+        element->AddValue("string", elemXml->GetText(), true);
       }
 
       copyChildren(element, elemXml, _onlyUnknown);