Skip to content

Bracketed escaping #1709

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

Closed
wants to merge 4 commits into from
Closed

Bracketed escaping #1709

wants to merge 4 commits into from

Conversation

jayvdb
Copy link

@jayvdb jayvdb commented Oct 22, 2020

Adds {\}, {{} and {}} as an escaping mechanism in settings.

{{} and {}} are not implemented so they can be used safely together.
Additional coding is needed to handle values from the engine being pushed
back into the engine, so they are re-escaped.

Fixes #1708

Contribution checklist:

(also see CONTRIBUTING.rst for details)

  • wrote descriptive pull request text
  • added/updated test(s)
  • updated/extended the documentation
  • added relevant issue keyword
    in message body
  • added news fragment in changelog folder
    • fragment name: <issue number>.<type>.rst for example (588.bugfix.rst)
    • <type> is must be one of bugfix, feature, deprecation,breaking, doc, misc
    • if PR has no issue: consider creating one first or change it to the PR number after creating the PR
    • "sign" fragment with "by :user:<your username>"
    • please use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files - by :user:superuser."
    • also see examples
  • added yourself to CONTRIBUTORS (preserving alphabetical order)

)
envconfig = config.envconfigs["python"]
# This fails: ConfigError: substitution key 'b c' not found
# assert envconfig.setenv["FOO"] == "a{b c}d"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

highlighting this as what fails with the incomplete implementation.

Incomplete.
@jayvdb jayvdb force-pushed the bracketed-escaping branch from 4c4d5c7 to 89b1e0e Compare October 23, 2020 06:17
@gaborbernat
Copy link
Member

Adds {\}, {{} and {}} as an escaping mechanism in settings.

{{} and {}} are not implemented so they can be used safely together.
Additional coding is needed to handle values from the engine being pushed
back into the engine, so they are re-escape

I would prefer not to introduce new ways of doing things. As such only \ character I'm interested in supporting, I don't agree with supporting {{ or }} 🤔

@jayvdb
Copy link
Author

jayvdb commented Oct 23, 2020

I don't agree with supporting {{ or }}

I am also not convinced about adding those; I've tried to lay out the problems at

#1708

That commit in this PR is mostly to show where the problem exists, so anyone can play with it if they think it is easy to solve.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now has merge conflicts.

@gaborbernat
Copy link
Member

I'll close this for now until you can attend to the PR again. Once you've done we can reopen.

@gaborbernat gaborbernat closed this Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backslash-ed curly bracket in settings
2 participants