Allow empty strings in plugin and custom attributes

[scpeters]

🦟 Bug fix

Fixes #725

Summary

Currently errors are generated when adding an attribute containing an empty string to a <plugin> block or as a custom attribute. This adds failing tests in https://github.com/gazebosim/sdformat/commit/0637c210053ae3d1b98e9be55b30f9b3f1e3516f to confirm the bug and fixes the errors in https://github.com/gazebosim/sdformat/commit/e55a9e1a251b650ac0588e0e2b7323f0ca900961 by setting _required == 0 when calling Element::AddAttribute.

Checklist

• [X] Signed all commits for DCO
• [X] Added tests
• [ ] Updated documentation (as needed)
• [ ] Updated migration guide (as needed)
• [ ] Consider updating Python bindings (if the library has them)
• [X] codecheck passed (See contributing)
• [X] All tests passed (See test coverage)
• [X] While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.44%. Comparing base (ff9b3ad) to head (e55a9e1). Report is 13 commits behind head on sdf14.

:exclamation: Current head e55a9e1 differs from pull request most recent head 39036ee

Please upload reports for the commit 39036ee to get more accurate results.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## sdf14 #1407 +/- ## ========================================== + Coverage 92.42% 92.44% +0.01% ========================================== Files 134 135 +1 Lines 17751 17837 +86 ========================================== + Hits 16406 16489 +83 - Misses 1345 1348 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[scpeters]

Looks good, but can you check the output of ToString() to make sure the custom attribute is included? I seem to recall that was a problem.

ok, thanks for bringing that up. I updated ElementPrivate::PrintAttributes in https://github.com/gazebosim/sdformat/pull/1407/commits/a4f811f7fae269b77a8c35f16324340ce5f1f214 to always print custom attributes

Empty attributes in the <plugin/> element won't be printed by ToString unless they are in a custom xml namespace, as I've noticed with the following test patch:

diff --git a/test/integration/plugin_attribute.cc b/test/integration/plugin_attribute.cc
index 7f4f9f4c..b66c9da1 100644
--- a/test/integration/plugin_attribute.cc
+++ b/test/integration/plugin_attribute.cc
@@ -65,4 +65,9 @@ TEST(PluginAttribute, ParseAttributes)
     EXPECT_EQ(value->Get<std::string>(""),
               std::string("value"));
   }
+  {
+    auto pluginToString = plugin->ToString("");
+    EXPECT_NE(std::string::npos, pluginToString.find("empty_attribute=''"))
+      << pluginToString;
+  }
 }

I don't have a good idea for how to fix it, so I'd prefer to open an issue documenting that behavior and potentially close it as won't fix

[scpeters]

LGTM! Opening an issue to document the behavior in <plugin> sounds good.

opened a new issue: https://github.com/gazebosim/sdformat/issues/1421