Skip to content
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

Support escaping of interpolation sequences in RefValue strings #68

Closed
wants to merge 2 commits into from

Conversation

ppmathis
Copy link
Contributor

@ppmathis ppmathis commented Feb 9, 2017

RefValue string interpolation is now being done with a simplified lexer/parser and supports escaping, to avoid unwanted string interpolation. The new string interpolation mechanism does not break backwards compatibility (except [escape sequence][interpolation sequence], which will now be solely printed as [interpolation sequence]) to the existing solution, by falling back to a literal escape character when there is nothing to escape.

This pull request is meant to close issue #55, which requested the ability to escape string interpolation sequences. The new escape character, which is defined as a backslash ('\') per default, could also be used as an escape character for the application list, to solve issue #63 - but this should be probably put into a separate PR.

Happy to accept all kinds of feedback - tried to keep the code clean by eliminating all PyLint errors in the RefValue module, except the one stating that there is no module docstring (as this project does not seem to have them anywhere either). Also added some tests for backwards compatibility and the escaping feature itself.

RefValue string interpolation is now being done with a simplified lexer/parser and supports escaping, to avoid unwanted string interpolation. The new string interpolation mechanism does not break backwards compatibility (except <escape><string interpolation>) to the existing solution, by falling back to a literal escape character when there is nothing to escape.
The new implementation of string interpolation could lead to breaking backwards-compatibility with older versions, if the string included an escape character followed by an interpolation end sentinel. This led to just the interpolation end sentinel to be printed, which has now been fixed, by printing the escape character as well.
@lottspot
Copy link
Collaborator

We discussed this a bit in the issue thread, but I prefer this solution to the problem and would like to see this PR merged. I'll give @Rtzq0 a couple days to take a look at this and provide feedback if he has any. Otherwise, we'll move forward with merging this.

@Rtzq0
Copy link
Collaborator

Rtzq0 commented Feb 15, 2017

I think this looks good but I haven't had a chance to test it yet. @lottspot have you performed any testing?

If not I'd say provisionally yes I'm all for it and this looks great, but I'd like to wait until i get home to check out this branch.

@ppmathis
Copy link
Contributor Author

No idea how many active reclass users are watching this repository but getting as many testers as possible would be definitely great. For me it works just fine, covering dozens of nested variables and such for SaltStack, but maybe it would break somewhere else or there might still be a rare edge case.

I tried to cover all situations that came into my mind - but there's always that one user doing something real special :)

@ppmathis ppmathis closed this May 16, 2018
@ppmathis
Copy link
Contributor Author

As I am no longer using reclass and this pull request did not seem to move on for more than a year, I decided to close it by myself unmerged. If anyone is interested in taking over or taking snippets/parts of my solution, feel free to do so.

AndrewPickford pushed a commit to AndrewPickford/reclass that referenced this pull request Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants