Skip to content

Commit

Permalink
Suggestions for deprecating kSdfScopeDelimiter (#1265)
Browse files Browse the repository at this point in the history
Signed-off-by: Addisu Z. Taddese <[email protected]>
  • Loading branch information
azeey authored Mar 31, 2023
1 parent 632570e commit 75ceb7d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
19 changes: 14 additions & 5 deletions include/sdf/Types.hh
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,21 @@ namespace sdf
inline namespace SDF_VERSION_NAMESPACE {
//

static const std::string &SdfScopeDelimiter()
namespace internal
{
static const gz::utils::NeverDestroyed<std::string> delimiter{"::"};
return delimiter.Access();
}
static const std::string &kSdfScopeDelimiter = SdfScopeDelimiter();
/// \brief Initializes the scope delimiter as a function-local static
/// variable so it can be used to initialize kSdfScopeDelimiter.
/// \note This should not be used directly in user code. It will likely be
/// removed in libsdformat 15 with kSdfScopeDelimiter.
SDFORMAT_VISIBLE const std::string &SdfScopeDelimiter();
} // namespace internal

constexpr std::string_view kScopeDelimiter{"::"};

// Deprecated because it violates the Google Style Guide as it is not
// trivially destructible. Please use sdf::kScopeDelimiter instead.
GZ_DEPRECATED(14)
inline const std::string &kSdfScopeDelimiter = internal::SdfScopeDelimiter();

/// \brief The source path replacement if it was parsed from a string,
/// instead of a file.
Expand Down
26 changes: 17 additions & 9 deletions src/Types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,33 +96,33 @@ std::ostream &operator<<(std::ostream &_out, const sdf::Errors &_errs)
std::pair<std::string, std::string> SplitName(
const std::string &_absoluteName)
{
const auto pos = _absoluteName.rfind(kSdfScopeDelimiter);
const auto pos = _absoluteName.rfind(kScopeDelimiter);
if (pos != std::string::npos)
{
const std::string first = _absoluteName.substr(0, pos);
const std::string second =
_absoluteName.substr(pos + kSdfScopeDelimiter.size());
_absoluteName.substr(pos + kScopeDelimiter.size());
return {first, second};
}
return {"", _absoluteName};
}

static bool EndsWithDelimiter(const std::string &_s)
{
if (_s.size() < kSdfScopeDelimiter.size())
if (_s.size() < kScopeDelimiter.size())
return false;

const size_t startPosition = _s.size() - kSdfScopeDelimiter.size();
const size_t startPosition = _s.size() - kScopeDelimiter.size();
return _s.compare(
startPosition, kSdfScopeDelimiter.size(), kSdfScopeDelimiter) == 0;
startPosition, kScopeDelimiter.size(), kScopeDelimiter) == 0;
}

static bool StartsWithDelimiter(const std::string &_s)
{
if (_s.size() < kSdfScopeDelimiter.size())
if (_s.size() < kScopeDelimiter.size())
return false;

return _s.compare(0, kSdfScopeDelimiter.size(), kSdfScopeDelimiter) == 0;
return _s.compare(0, kScopeDelimiter.size(), kScopeDelimiter) == 0;
}

// Join a scope name prefix with a local name using the scope delimeter
Expand All @@ -138,11 +138,19 @@ std::string JoinName(
const bool localNameStartsWithDelimiter = StartsWithDelimiter(_localName);

if (scopeNameEndsWithDelimiter && localNameStartsWithDelimiter)
return _scopeName + _localName.substr(kSdfScopeDelimiter.size());
return _scopeName + _localName.substr(kScopeDelimiter.size());
else if (scopeNameEndsWithDelimiter || localNameStartsWithDelimiter)
return _scopeName + _localName;
else
return _scopeName + kSdfScopeDelimiter + _localName;
return _scopeName + std::string(kScopeDelimiter) + _localName;
}

/////////////////////////////////////////////////
const std::string &internal::SdfScopeDelimiter()
{
static const gz::utils::NeverDestroyed<std::string> delimiter{
std::string()};
return delimiter.Access();
}
}
}
2 changes: 1 addition & 1 deletion src/Utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ static std::optional<std::string> computeAbsoluteName(
std::advance(it, 1);
for (; it != names.rend(); ++it)
{
absoluteParentName.append(kSdfScopeDelimiter);
absoluteParentName.append(kScopeDelimiter);
absoluteParentName.append(*it);
}

Expand Down

0 comments on commit 75ceb7d

Please sign in to comment.