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

Added the dbg and format filters #517

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
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
6 changes: 6 additions & 0 deletions src/builtins/filters/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,12 @@ pub fn as_str(value: &Value, _: &HashMap<String, Value>) -> Result<Value> {
to_value(&value.render()).map_err(Error::json)
}

/// Prints the passed value to stdout before returning it
pub fn dbg(value: &Value, _: &HashMap<String, Value>) -> Result<Value> {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this should be a default filter, people might want to log instead etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's a bad thing as if someone wants log they can just make one. This is mainly (as with the dbg!() macro) for debugging. I've found that in nearly every project I've done with Tera I end up recreating this, and it's simple enough to add

Copy link
Owner

Choose a reason for hiding this comment

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

It's easy to add as you say so I would keep it out of the builtins one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this function even related to formatting? Should it be a separate PR?

println!("{:#?}", value);
Ok(value.clone())
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
140 changes: 139 additions & 1 deletion src/builtins/filters/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,109 @@ pub fn filesizeformat(value: &Value, _: &HashMap<String, Value>) -> Result<Value
.map(std::result::Result::unwrap)
}

/// Formats integers using the `fmt` string given
///
/// Formatting options (All options passed via `format(fmt="<option>")`:
/// * `:X` Upper hex (`42` => `2A`)
/// * `:x` Lower hex (`42` => `2a`)
/// * `:o` Octal (`42` => `52`)
/// * `:b` Binary (`42` => `101010`)
/// * `:E` Upper exponent (`42.0` => `4.2E1`)
/// * `:e` Lower exponent (`42.0` => `4.2e1`)
///
/// Additionally, the `#` modifier can be passed to some formatters as well:
/// * `:#X` Upper hex (`42` => `0x2A`)
/// * `:#x` Lower hex (`42` => `0x2a`)
/// * `:#o` Octal (`42` => `0o52`)
/// * `:#b` Binary (`42` => `0b101010`)
pub fn format(value: &Value, args: &HashMap<String, Value>) -> Result<Value> {
let fmt = if let Some(fmt) = args.get("fmt") {
try_get_value!("format", "fmt", String, fmt)
} else {
return Err(Error::msg("Filter `format` expected an arg called `fmt`"));
};
let mut chars = fmt.chars();

if !matches!(chars.next(), Some(':')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see matches! macro being used anywhere else in this project. Should it be an IF or MATCH ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

matches!() is in the stdlib.

return Err(Error::msg("Format specifiers for the `format` filter must start with `:`"));
}

let mut spec = chars.next().ok_or_else(|| {
Error::msg("Format specifiers for the `format` filter must have more than one character")
})?;

let alternative = if spec == '#' {
spec = chars.next().ok_or_else(|| {
Error::msg("Format strings for the `format` filter with a modifier must have a format specifier")
})?;
true
} else {
false
};

macro_rules! unwrap_integers {
($val:expr, if $alt:ident { $alt_fmt:expr } else { $fmt:expr }, $err:expr) => {
if let Some(uint) = $val.as_u64() {
if $alt {
format!($alt_fmt, uint)
} else {
format!($fmt, uint)
}
} else if let Some(int) = $val.as_i64() {
if $alt {
format!($alt_fmt, int)
} else {
format!($fmt, int)
}
} else {
return Err($err);
}
};
}

let value = match spec {
'X' => unwrap_integers!(
value,
if alternative { "{:#X}" } else { "{:X}" },
Error::msg("`:X` only takes integer values")
),
'x' => unwrap_integers!(
value,
if alternative { "{:#x}" } else { "{:x}" },
Error::msg("`:x` only takes integer values")
),
'o' => unwrap_integers!(
value,
if alternative { "{:#o}" } else { "{:o}" },
Error::msg("`:o` only takes integer values")
),
'b' => unwrap_integers!(
value,
if alternative { "{:#b}" } else { "{:b}" },
Error::msg("`:b` only takes integer values")
),

'E' => {
let float = try_get_value!("format", "value", f64, value);
format!("{:E}", float)
}
'e' => {
let float = try_get_value!("format", "value", f64, value);
format!("{:e}", float)
}

unrecognized => {
return Err(Error::msg(format!("Unrecognized format specifier: `:{}`", unrecognized)))
}
};

Ok(Value::String(value))
}
Copy link
Owner

Choose a reason for hiding this comment

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

Is that exactly what Jinja2 is doing?
Can we add a link to the implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That wasn't based on Jinja, it was just based on Rust, I have no knowledge of what Jinja does

Copy link
Owner

Choose a reason for hiding this comment

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

Tera is trying to roughly match what Jinja is doing. The format filter of Jinja is essentially printf (https://jinja.palletsprojects.com/en/2.11.x/templates/#format) so it would need match functionality if it has the same name. We could rename it to avoid confusion

Copy link
Contributor

Choose a reason for hiding this comment

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

Implementing printf(https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting) is quite involved. I wonder how much of would actually get used and if a Rust programmer would know what to do with it. I can see the point of staying close to Jinja2, but matching printf is more like staying close to Python.

Copy link
Owner

Choose a reason for hiding this comment

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

Yep that's fine, just need to rename it so people using both are not confused by having the same name but different functionalities.


#[cfg(test)]
mod tests {
use super::*;
use serde_json::value::to_value;
use serde_json::{json, value::to_value};
use std::collections::HashMap;

#[test]
Expand Down Expand Up @@ -181,4 +280,43 @@ mod tests {
assert!(result.is_ok());
assert_eq!(result.unwrap(), to_value("117.74 MB").unwrap());
}

#[test]
fn formatting() {
let values = [
(json!(42), ":X", "2A"),
(json!(42), ":x", "2a"),
(json!(42), ":o", "52"),
(json!(42), ":b", "101010"),
(json!(42.0), ":E", "4.2E1"),
(json!(42.0), ":e", "4.2e1"),
(json!(42), ":#X", "0x2A"),
(json!(42), ":#x", "0x2a"),
(json!(42), ":#o", "0o52"),
(json!(42), ":#b", "0b101010"),
];
let mut args = HashMap::new();

for (value, fmt, expected) in values.iter() {
args.insert(String::from("fmt"), json!(fmt));

let result = format(value, &args);
assert!(result.is_ok());
assert_eq!(json!(expected), result.unwrap());
}
}

#[test]
fn fmt_required() {
let args = HashMap::new();
assert!(format(&json!("It don't matter"), &args).is_err());
}

#[test]
fn unrecognized_formatter() {
let mut args = HashMap::new();
args.insert(String::from("fmt"), json!("I do not exist"));

assert!(format(&json!("It don't matter"), &args).is_err());
}
}
2 changes: 2 additions & 0 deletions src/tera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ impl Tera {

self.register_filter("pluralize", number::pluralize);
self.register_filter("round", number::round);
self.register_filter("format", number::format);

#[cfg(feature = "builtins")]
self.register_filter("filesizeformat", number::filesizeformat);
Expand All @@ -556,6 +557,7 @@ impl Tera {
self.register_filter("date", common::date);
self.register_filter("json_encode", common::json_encode);
self.register_filter("as_str", common::as_str);
self.register_filter("dbg", common::dbg);

self.register_filter("get", object::get);
}
Expand Down