-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Allow to use symbols on the front matter #396
Allow to use symbols on the front matter #396
Conversation
I realize now I badly worded my initial issue. I've added some additional context. This is a great fix @JuanVqz, but we'll need a couple more changes to address the problem I was seeing. |
I'll add the |
@@ -131,7 +131,7 @@ def convert(content, convertible) | |||
end | |||
|
|||
def matches(ext, convertible) | |||
if convertible.data[:template_engine] == "erb" || | |||
if convertible.data[:template_engine].to_s == "erb" || | |||
(convertible.data[:template_engine].nil? && | |||
@config[:template_engine] == "erb") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaredmoody should I add to_s
here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant @jaredcwhite 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I add
to_s
here too?
@JuanVqz I'd suggest a broad search for "erb", might need to update a few places.
Probably have the same issue with "liquid" as well.
7bd719e
to
7b90b66
Compare
@JuanVqz Do you think this will be ready for me to review soon? I'd be happy to help get it over the finish line. 🙏 |
Yes, please. I think it's ready |
Hey @JuanVqz, I'll merge in the PR but will also revert the changes to the YAML parsing — I don't want our YAML parser to be able to use symbols. (In the original issue the symbols in question were coming from Ruby front matter, not YAML, so I think keeping YAML string-only is fine.) |
@jaredcwhite thank you for your feedback |
@JuanVqz no problem — your contribution is much appreciated! |
This is a 🐛 bug fix.
Summary
Add Symbol in the PERMITED_CLASSES array.
Context
Fixes #395