-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ParserConfig class to encapsulate file path settings #439
Changes from 13 commits
c4e2ba4
89dc847
7badb17
b4f1da3
e3e05f3
f9e3ef2
deda523
b9bb22b
ddafb64
74eacf0
f088be8
5084a6d
5dfe1a0
1cf635a
e42bbda
6388959
fde3b1b
6f53a9f
8977cd1
5972b53
1d321e0
8a9e4e8
53af3c7
ed20a74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ set (headers | |
Noise.hh | ||
Param.hh | ||
parser.hh | ||
ParserConfig.hh | ||
Pbr.hh | ||
Physics.hh | ||
Plane.hh | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,130 @@ | ||
/* | ||
* Copyright 2020 Open Source Robotics Foundation | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
* | ||
*/ | ||
|
||
#ifndef SDF_PARSER_CONFIG_HH_ | ||
#define SDF_PARSER_CONFIG_HH_ | ||
|
||
#include <functional> | ||
#include <map> | ||
#include <string> | ||
#include <vector> | ||
|
||
#include <sdf/Error.hh> | ||
#include "sdf/sdf_config.h" | ||
#include "sdf/system_util.hh" | ||
|
||
namespace sdf | ||
{ | ||
inline namespace SDF_VERSION_NAMESPACE | ||
{ | ||
// Forward declare private data class. | ||
class ParserConfigPrivate; | ||
|
||
/// This class contains configuration options for the libsdformat parser. | ||
/// | ||
/// The configuration options include: | ||
/// * A callback function used to locate a file from a given URI | ||
/// * A map from URI schemes to search directories | ||
/// | ||
/// For backward compatibility, the functions sdf::setFindCallback() and | ||
/// sdf::addURIPath() update a singleton ParserConfig object, which can be | ||
/// retrieved by ParserConfig::GlobalConfig(). | ||
/// | ||
/// The functions sdf::readFile(), sdf::readString(), and \ref Root::Load | ||
/// "sdf::Root::Load()" have overloads that take a ParserConfig object. If | ||
/// the ParserConfig object is omitted, these functions will use the singleton | ||
/// ParserConfig object. | ||
/// | ||
/// Example: | ||
/// To set an additional URI scheme search directory without affecting the | ||
/// global config, | ||
/// | ||
/// \code{.cpp} | ||
/// // Copy the global config | ||
/// sdf::ParserConfig config = ParserConfig::GlobalConfig(); | ||
/// | ||
/// // Add the new scheme to the config | ||
/// config.AddURIPath("newScheme://", "path/to/directory"); | ||
/// | ||
/// // Use the new config when loading a new SDFormat file | ||
/// sdf::Root root; | ||
/// root.Load("path/to/file.sdf", config); | ||
/// \endcode | ||
class SDFORMAT_VISIBLE ParserConfig | ||
{ | ||
/// type alias for the map from URI scheme to search directories | ||
public: using SchemeToPathMap = | ||
std::map<std::string, std::vector<std::string> >; | ||
|
||
/// \brief Default constructor | ||
public: ParserConfig(); | ||
|
||
/// \brief Copy constructor | ||
/// \param[in] _config ParserConfig to copy. | ||
public: ParserConfig(const ParserConfig &_config); | ||
|
||
/// \brief Move constructor | ||
/// \param[in] _config ParserConfig to move. | ||
public: ParserConfig(ParserConfig &&_config) noexcept; | ||
|
||
/// \brief Move assignment operator. | ||
/// \param[in] _config ParserConfig to move. | ||
/// \return Reference to this. | ||
public: ParserConfig &operator=(ParserConfig &&_config) noexcept; | ||
|
||
/// \brief Copy assignment operator. | ||
/// \param[in] _config ParserConfig to copy. | ||
/// \return Reference to this. | ||
public: ParserConfig &operator=(const ParserConfig &_config); | ||
|
||
/// \brief Destructor | ||
public: ~ParserConfig(); | ||
|
||
/// Mutable access to a singleton ParserConfig that serves as the global | ||
/// ParserConfig object for all parsing operations | ||
azeey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// \return A mutable reference to the singleton ParserConfig object | ||
public: static ParserConfig &GlobalConfig(); | ||
|
||
/// \brief Get the find file callback function | ||
/// \return Immutable reference to the find file callback function | ||
public: const std::function<std::string(const std::string &)> & | ||
FindFileCallback() const; | ||
|
||
/// \brief Set the callback to use when libsdformat can't find a file. | ||
/// The callback should return a complete path to the requested file, or | ||
scpeters marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// and empty string if the file was not found in the callback. | ||
azeey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// \param[in] _cb The callback function. | ||
public: void SetFindCallback( | ||
std::function<std::string(const std::string &)> _cb); | ||
|
||
/// \brief Get the URI scheme to search directories map | ||
/// \return Immutable reference to the URI scheme to search directories map | ||
public: const SchemeToPathMap &URIPathMap() const; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit It smells weird to be able to look at all paths, add a new path, but not remove a path. (assymmetry) Is it hard to add that here? Or perhaps the right workflow is to just reassign? e.g. just clear it out. So if I wanted to remove or reorder a path, I would just do something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying to make this PR replicate existing functionality without adding any new features other than being able to specify a non-global config. It would be easy to add a function to remove a URI scheme, but adding a path to an existing URI scheme or reordering paths seem best suited for a future PR, IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aye, makes sense to me. |
||
|
||
/// \brief Associate paths to a URI. | ||
/// Example paramters: "model://", "/usr/share/models:~/.gazebo/models" | ||
/// \param[in] _uri URI that will be mapped to _path | ||
/// \param[in] _path Colon separated set of paths. | ||
public: void AddURIPath(const std::string &_uri, const std::string &_path); | ||
|
||
/// \brief Private data pointer. | ||
private: ParserConfigPrivate *dataPtr; | ||
}; | ||
} | ||
} | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,11 +21,12 @@ | |
#include <memory> | ||
#include <string> | ||
|
||
#include "sdf/Param.hh" | ||
#include "sdf/Element.hh" | ||
#include "sdf/Param.hh" | ||
#include "sdf/ParserConfig.hh" | ||
#include "sdf/Types.hh" | ||
#include "sdf/sdf_config.h" | ||
#include "sdf/system_util.hh" | ||
#include "sdf/Types.hh" | ||
|
||
#ifdef _WIN32 | ||
// Disable warning C4251 which is triggered by | ||
|
@@ -64,6 +65,21 @@ namespace sdf | |
bool _searchLocalPath = true, | ||
bool _useCallback = false); | ||
|
||
/// \brief Find the absolute path of a file. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new function, and the old flavor, don't fully specify how the searching actually takes place. (I understand it's implementation specific, but still.) From There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know about more concise, but I listed the search order in 6f53a9f. How does that look? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ooh, that's excellent! Thank you! |
||
/// \param[in] _filename Name of the file to find. | ||
/// \param[in] _searchLocalPath True to search for the file in the current | ||
/// working directory. | ||
/// \param[in] _useCallback True to find a file based on a registered | ||
/// callback if the file is not found via the normal mechanism. | ||
/// \param[in] _config Custom parser configuration | ||
/// \return File's full path. | ||
SDFORMAT_VISIBLE | ||
std::string findFile(const std::string &_filename, | ||
bool _searchLocalPath, | ||
bool _useCallback, | ||
const ParserConfig &_config); | ||
|
||
|
||
/// \brief Associate paths to a URI. | ||
/// Example paramters: "model://", "/usr/share/models:~/.gazebo/models" | ||
/// \param[in] _uri URI that will be mapped to _path | ||
|
@@ -78,7 +94,6 @@ namespace sdf | |
SDFORMAT_VISIBLE | ||
void setFindCallback(std::function<std::string (const std::string &)> _cb); | ||
|
||
|
||
/// \brief Base SDF class | ||
class SDFORMAT_VISIBLE SDF | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to below comment (how path resolution works), it's unclear to me what the precedence is for these different attributes (e.g. does callback override search paths, or the other way around).
Is there documentation that this can forward-reference, or should it be written here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some documentation in SDFImpl.hh (6f53a9f) and referenced it here in 1d321e0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Thanks!