Fix #760, Disables MiniJinja auto-escaping on filename#1239
Fix #760, Disables MiniJinja auto-escaping on filename#1239jsuereth merged 6 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1239 +/- ##
=====================================
Coverage 80.3% 80.3%
=====================================
Files 110 110
Lines 8984 9009 +25
=====================================
+ Hits 7222 7243 +21
- Misses 1762 1766 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
crates/weaver_forge/src/lib.rs
Outdated
| .or_else(|| name.strip_suffix(".jinja2")) | ||
| .unwrap_or(name); | ||
| match name.rsplit('.').next().unwrap_or("") { | ||
| "html" | "htm" | "xml" => minijinja::AutoEscape::Html, |
There was a problem hiding this comment.
Should this include markdown, java, or not?
I'm wondering if we should make this configurable...
There was a problem hiding this comment.
From the docs:
The default logic for auto escaping based on file extension.
Html: .html, .htm, .xml
Json: .json, .json5, .js, .yaml, .yml
None: all others
It seems highly unlikely that you want escaping if you're building a yaml file in the context of weaver. But, yeah we could prevent the breaking change and make this a config option instead if you think that's better?
There was a problem hiding this comment.
@jsuereth - I made this per template configurable in the weaver.yaml. This now has the added benefit that, regardless of file extension, you can enable auto escaping.
| /// deserialization from `weaver.yaml`. | ||
| #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq)] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub(crate) enum AutoEscapeMode { |
Fixes #760
💥 BREAKING CHANGE 💥
Auto-escaping is now off by default (
none) for all templates, regardless of file extension. To opt in, setauto_escape: htmlorauto_escape: jsonper template inweaver.yaml. Within a template,{% autoescape false %}blocks can selectively disable escaping for sections. Use|tojsonfor explicit JSON/YAML value escaping where needed.See: https://docs.rs/minijinja/latest/minijinja/fn.default_auto_escape_callback.html
Also fixed the order of precedence for template and format usage:
This allows you to have a
yamltarget even thoughyamlis now a builtin.