gazebosim / gazebosim/sdformat

Stop using global variables

Open
#373 6 comments 2 reactions 1 assignee Claimed by @azeey View on GitHub
Dominant language
C++
Stars
216
Forks
125
Avg merge
1d 14h
Merged PRs (30d)
14

Description

There are several places where global variables are used in `libsdformat`, which makes it difficult to control behavior when there are multiple paths that parse SDFormat files. As @jwnimmer-tri noted in https://github.com/osrf/sdformat/issues/334#issuecomment-683985626:

> parsing should always be done against a parsing context or configuration, so that using-code can pass down customizations to any given request to parse, without having process-wide globals. We could add more argument to `LoadSdfString` but that gets awkward when there are too many. A parsing object would help consolidate (and reuse) the configuration.

> In the long term, the best outcome is that libsdformat posts its messages (calls a function) on some logger interface or infrastructure, so that downstream code can easily capture / prefix / redirect / threshold the messages. That might even be better formulated as a "parsing error" interface, rather than unstructured text logging. A parsing library really has no business just remarking plain text into a console -- if the input text is malformed, then libsdformat should be reporting those errors in a structured way (with input line and column numbers, severity, configuration of strict vs permissive parsing, etc.) so that application code can inform the user in a way consistent with the application's behavior.

### Places where global variables are used:

* https://github.com/osrf/sdformat/blob/sdformat10_10.0.0-pre2/src/Console.cc#L31-L42
* https://github.com/osrf/sdformat/blob/sdformat10_10.0.0-pre2/src/SDF.cc#L43-L47
* https://github.com/osrf/sdformat/blob/sdformat10_10.0.0-pre2/src/parser_urdf.cc#L53-L63

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.