feat(junos): improve _convert_to_set_commands #132
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a proposed follow-up for #125 to make the parsing of set-based configurations more robust:
self.options["negation"]
instead of the hardcodeddelete
when checking if the line is already a "set" line{
/}
instead of the indentation to build the configuration treesplit("\n")
->splitlines()
: to handle different line breaksreplace(";", "")
->.rstrip(";")
:rstrip
feels safer in that it doesn't remove a semicolon appearing elsewhere in the line (not sure it could happen to have a string containing a semicolon but better safe than sorry?)if self.options["syntax_style"] == "juniper":
out of the function to remove unnecessary indentation and improve readabilityI believe relying on indentation as is currently done could lead to unexpected results (if the generated configuration is not perfectly indented for example) especially since the configuration format provides a way to traverse the tree using braces and doesn't rely on indentation to be parsed. This new implementation can also properly error out on invalid configuration (unterminated blocks / extra braces).
I'm curious to know if there was a specific reason I'm not seeing to use indentation instead of parsing the blocks directly?