Get rid of trailing slash saving
This m_trailingSlash was the source of bugs, so I got rid of it. A
caveat is that I lose this information if I want to recreate the path
back, but it isn't really too useful.
The actual bug was about conversion of the parsed path back to a string.
The problem was when the input had a trailing slash, the result string
would also have that trailing slash, which is undesirable (libyang
cannot deal with it).
This patch completely removes the information about the trailing slash,
so converting a *Path_ structure back to a string can't result with a
path that has a trailing slash.
However, I do think that there should be at least some sort of a test,
so the `cd` test converts paths back to strings and asserts that they
don't have trailing slashes.
Issue: https://tree.taiga.io/project/jktjkt-netconf-cli/issue/212
Change-Id: I08e02401580ce31c1e0412a5798cea20e7802ab4
diff --git a/src/ast_path.cpp b/src/ast_path.cpp
index 9c77f35..33b25fe 100644
--- a/src/ast_path.cpp
+++ b/src/ast_path.cpp
@@ -170,10 +170,9 @@
schemaPath_::schemaPath_() = default;
-schemaPath_::schemaPath_(const Scope scope, const std::vector<schemaNode_>& nodes, const TrailingSlash trailingSlash)
+schemaPath_::schemaPath_(const Scope scope, const std::vector<schemaNode_>& nodes)
: m_scope(scope)
, m_nodes(nodes)
- , m_trailingSlash(trailingSlash)
{
validatePathNodes(m_nodes);
}
@@ -188,10 +187,9 @@
dataPath_::dataPath_() = default;
-dataPath_::dataPath_(const Scope scope, const std::vector<dataNode_>& nodes, const TrailingSlash trailingSlash)
+dataPath_::dataPath_(const Scope scope, const std::vector<dataNode_>& nodes)
: m_scope(scope)
, m_nodes(nodes)
- , m_trailingSlash(trailingSlash)
{
validatePathNodes(m_nodes);
}
@@ -272,9 +270,6 @@
res = joinPaths(res, (prefixes == Prefixes::Always ? path.m_nodes.at(0).m_prefix.value().m_name + ":" : "") + std::visit(nodeToDataStringVisitor(), it.m_suffix));
}
}
- if (path.m_trailingSlash == TrailingSlash::Present) {
- res += "/";
- }
return res;
}
diff --git a/src/ast_path.hpp b/src/ast_path.hpp
index 8a86181..e7df783 100644
--- a/src/ast_path.hpp
+++ b/src/ast_path.hpp
@@ -114,11 +114,6 @@
bool operator==(const dataNode_& b) const;
};
-enum class TrailingSlash {
- Present,
- NonPresent
-};
-
enum class Scope {
Absolute,
Relative
@@ -126,22 +121,20 @@
struct schemaPath_ {
schemaPath_();
- schemaPath_(const Scope scope, const std::vector<schemaNode_>& nodes, const TrailingSlash trailingSlash = TrailingSlash::NonPresent);
+ schemaPath_(const Scope scope, const std::vector<schemaNode_>& nodes);
bool operator==(const schemaPath_& b) const;
Scope m_scope = Scope::Relative;
std::vector<schemaNode_> m_nodes;
- TrailingSlash m_trailingSlash = TrailingSlash::NonPresent;
// @brief Pushes a new fragment. Pops a fragment if it's nodeup_
void pushFragment(const schemaNode_& fragment);
};
struct dataPath_ {
dataPath_();
- dataPath_(const Scope scope, const std::vector<dataNode_>& nodes, const TrailingSlash trailingSlash = TrailingSlash::NonPresent);
+ dataPath_(const Scope scope, const std::vector<dataNode_>& nodes);
bool operator==(const dataPath_& b) const;
Scope m_scope = Scope::Relative;
std::vector<dataNode_> m_nodes;
- TrailingSlash m_trailingSlash = TrailingSlash::NonPresent;
// @brief Pushes a new fragment. Pops a fragment if it's nodeup_
void pushFragment(const dataNode_& fragment);
@@ -167,5 +160,5 @@
BOOST_FUSION_ADAPT_STRUCT(module_, m_name)
BOOST_FUSION_ADAPT_STRUCT(dataNode_, m_prefix, m_suffix)
BOOST_FUSION_ADAPT_STRUCT(schemaNode_, m_prefix, m_suffix)
-BOOST_FUSION_ADAPT_STRUCT(dataPath_, m_scope, m_nodes, m_trailingSlash)
-BOOST_FUSION_ADAPT_STRUCT(schemaPath_, m_scope, m_nodes, m_trailingSlash)
+BOOST_FUSION_ADAPT_STRUCT(dataPath_, m_scope, m_nodes)
+BOOST_FUSION_ADAPT_STRUCT(schemaPath_, m_scope, m_nodes)
diff --git a/src/path_parser.hpp b/src/path_parser.hpp
index 29dbd04..0aace94 100644
--- a/src/path_parser.hpp
+++ b/src/path_parser.hpp
@@ -21,7 +21,7 @@
x3::rule<listInstancePath_class, dataPath_> const listInstancePath = "listInstancePath";
x3::rule<leafListElementPath_class, dataPath_> const leafListElementPath = "leafListElementPath";
x3::rule<initializePath_class, x3::unused_type> const initializePath = "initializePath";
-x3::rule<trailingSlash_class, TrailingSlash> const trailingSlash = "trailingSlash";
+x3::rule<trailingSlash_class, x3::unused_type> const trailingSlash = "trailingSlash";
x3::rule<absoluteStart_class, Scope> const absoluteStart = "absoluteStart";
x3::rule<keyValue_class, keyValue_> const keyValue = "keyValue";
x3::rule<key_identifier_class, std::string> const key_identifier = "key_identifier";
@@ -300,9 +300,9 @@
// command. If we're in DataPathListEnd it doesn't make sense to parse put any more nodes after the
// final list.
if constexpr (PARSER_MODE == PathParserMode::AnyPath) {
- res = (-(trailingSlash >> x3::omit[pathCompletions<COMPLETION_MODE>{m_filterFunction}])).parse(begin, end, ctx, rctx, attrData.m_trailingSlash);
+ res = (-(trailingSlash >> x3::omit[pathCompletions<COMPLETION_MODE>{m_filterFunction}])).parse(begin, end, ctx, rctx, x3::unused);
} else {
- res = (-trailingSlash).parse(begin, end, ctx, rctx, attrData.m_trailingSlash);
+ res = (-trailingSlash).parse(begin, end, ctx, rctx, x3::unused);
}
}
}
@@ -322,7 +322,7 @@
// The schemaPath parser continues where the dataPath parser ended.
res = schemaPath.parse(begin, end, ctx, rctx, attrSchema.m_nodes);
auto trailing = -trailingSlash >> pathEnd;
- res = trailing.parse(begin, end, ctx, rctx, attrSchema.m_trailingSlash);
+ res = trailing.parse(begin, end, ctx, rctx, x3::unused);
attr = attrSchema;
}
}
@@ -397,7 +397,7 @@
x3::omit['/'] >> x3::attr(Scope::Absolute);
auto const trailingSlash_def =
- x3::omit['/'] >> x3::attr(TrailingSlash::Present);
+ x3::omit['/'];
auto const filterConfigFalse = [](const Schema& schema, const std::string& path) {
return schema.isConfig(path);
diff --git a/tests/cd.cpp b/tests/cd.cpp
index ee056ae..538e4fa 100644
--- a/tests/cd.cpp
+++ b/tests/cd.cpp
@@ -51,7 +51,6 @@
SECTION("trailing slash")
{
input = "cd example:a/";
- expected.m_path.m_trailingSlash = TrailingSlash::Present;
}
SECTION("no trailing slash")
{
@@ -65,7 +64,6 @@
SECTION("trailing slash")
{
input = "cd second:a/";
- expected.m_path.m_trailingSlash = TrailingSlash::Present;
}
SECTION("no trailing slash")
{
@@ -191,6 +189,9 @@
command_ command = parser.parseCommand(input, errorStream);
REQUIRE(command.type() == typeid(cd_));
REQUIRE(boost::get<cd_>(command) == expected);
+ // Converting the path back to a string should never result with a path with a trailing slash, even if the
+ // original input string has it.
+ REQUIRE(pathToDataString(boost::get<cd_>(command).m_path, Prefixes::WhenNeeded).back() != '/');
}
SECTION("invalid input")
{