-
Notifications
You must be signed in to change notification settings - Fork 293
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(':')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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)) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that exactly what Jinja2 is doing? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implementing There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
|
@@ -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()); | ||
} | ||
} |
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 don't think this should be a default filter, people might want to log instead etc
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 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 addThere 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.
It's easy to add as you say so I would keep it out of the builtins one.
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.
Is this function even related to formatting? Should it be a separate PR?