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

Add a to_yaml filter #525

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions crates/weaver_forge/src/extensions/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub(crate) fn add_filters(env: &mut Environment<'_>, target_config: &WeaverConfi
env.add_filter("flatten", flatten);
env.add_filter("split_id", split_id);
env.add_filter("regex_replace", regex_replace);
env.add_filter("to_yaml", to_yaml);
}

/// Add utility functions to the environment.
Expand Down Expand Up @@ -131,6 +132,16 @@ pub fn acronym(acronyms: Vec<String>) -> impl Fn(&str) -> String {
}
}

fn to_yaml(value: Value) -> Result<String, minijinja::Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should take the tojson filter implemented in the standard minijinja library as an example. The method’s signature is slightly different, and more importantly, they address the compatibility of the output with HTML (pay attention to the last map function). We don’t need the pretty-printing part since YAML is always pretty-printed and we probably don't need the indent parameter too.

See the permalink below.

https://github.com/mitsuhiko/minijinja/blob/d1ce496e101239bdfba01e29f407f0a57b8e4aed/minijinja/src/filters.rs#L1018

let value = serde_json::to_value(value)
.map_err(|error| minijinja::Error::new(ErrorKind::BadSerialization, error.to_string()))?;
Comment on lines +136 to +137
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unnecessary, you can pass a reference to minijinja::Value directly to serde_yaml::to_string because it implements serde::Serialize.


let yaml = serde_yaml::to_string(&value)
.map_err(|error| minijinja::Error::new(ErrorKind::BadSerialization, error.to_string()))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps there's a way to write a test to exercise the error condition?


Ok(yaml)
}

#[cfg(test)]
mod tests {
use crate::extensions::util::add_filters;
Expand Down Expand Up @@ -159,4 +170,41 @@ mod tests {
"This A test with multiple A's"
);
}

#[test]
fn test_to_yaml() {
let mut env = Environment::new();
let ctx = serde_json::json!({
"user": {
"name": "Alice",
"age": 30,
"is_active": true,
"skills": ["Rust", "JavaScript"],
"details": {
"city": "Wonderland",
"email": "alice@example.com"
}
}
});
let config = crate::config::WeaverConfig::default();

add_filters(&mut env, &config);

let expected_yaml = "\
age: 30
details:
city: Wonderland
email: alice@example.com
is_active: true
name: Alice
skills:
- Rust
- JavaScript
";

assert_eq!(
env.render_str("{{ user | to_yaml }}", &ctx).unwrap(),
expected_yaml
);
}
}
Loading