Skip to content

Relax filesizeformat filter requirements #216

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

Merged
Merged
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
4 changes: 1 addition & 3 deletions rinja/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ rustdoc-args = ["--generate-link-to-definition", "--cfg=docsrs"]
maintenance = { status = "actively-developed" }

[features]
default = ["config", "humansize", "urlencode"]
default = ["config", "urlencode"]
full = ["default", "code-in-doc", "serde_json"]
code-in-doc = ["rinja_derive/code-in-doc"]
config = ["rinja_derive/config"]
humansize = ["rinja_derive/humansize", "dep:humansize"]
serde_json = ["rinja_derive/serde_json", "dep:serde", "dep:serde_json"]
urlencode = ["rinja_derive/urlencode", "dep:percent-encoding"]

Expand All @@ -39,7 +38,6 @@ with-warp = ["rinja_derive/with-warp"]
[dependencies]
rinja_derive = { version = "=0.3.5", path = "../rinja_derive" }

humansize = { version = "2", optional = true }
Copy link
Member

Choose a reason for hiding this comment

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

I think removing a feature is a breaking change. Maybe it would be better to keep it as a noop for now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can make the next release a major one. Problem solved like that. :)

Copy link
Member

Choose a reason for hiding this comment

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

Works for me! :D

num-traits = { version = "0.2.6", optional = true }
percent-encoding = { version = "2.1.0", optional = true }
serde = { version = "1.0", optional = true }
Expand Down
93 changes: 83 additions & 10 deletions rinja/src/filters/humansize.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::convert::Infallible;
use std::fmt;
use std::mem::MaybeUninit;
use std::str::from_utf8_unchecked;

use humansize::{DECIMAL, ISizeFormatter, ToF64};
use super::FastWritable;

/// Returns adequate string representation (in KB, ..) of number of bytes
///
Expand All @@ -21,24 +23,95 @@ use humansize::{DECIMAL, ISizeFormatter, ToF64};
/// assert_eq!(tmpl.to_string(), "Filesize: 1.23 MB.");
/// ```
#[inline]
pub fn filesizeformat(b: &impl ToF64) -> Result<FilesizeFormatFilter, Infallible> {
Ok(FilesizeFormatFilter(b.to_f64()))
pub fn filesizeformat(b: f32) -> Result<FilesizeFormatFilter, Infallible> {
Ok(FilesizeFormatFilter(b))
}

#[derive(Debug, Clone, Copy)]
pub struct FilesizeFormatFilter(f64);
pub struct FilesizeFormatFilter(f32);

impl fmt::Display for FilesizeFormatFilter {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_fmt(format_args!("{}", ISizeFormatter::new(self.0, &DECIMAL)))
self.write_into(f)
}
}

impl FastWritable for FilesizeFormatFilter {
fn write_into<W: fmt::Write + ?Sized>(&self, dest: &mut W) -> fmt::Result {
if self.0 < 1e3 {
(self.0 as u32).write_into(dest)?;
dest.write_str(" B")
} else if let Some((prefix, factor)) = SI_PREFIXES
.iter()
.copied()
.find_map(|(prefix_factor, max)| (self.0 < max).then_some(prefix_factor))
{
// u32 is big enough to hold the number 999_999
let scaled = (self.0 * factor) as u32;
(scaled / 100).write_into(dest)?;
dest.write_str(format_frac(&mut MaybeUninit::uninit(), prefix, scaled))
} else {
too_big(self.0, dest)
}
}
}

/// Formats `buffer` to contain the decimal point, decimal places and unit
fn format_frac(buffer: &mut MaybeUninit<[u8; 8]>, prefix: u8, scaled: u32) -> &str {
// LLVM generates better byte code for register sized buffers, so we add some NULs
let buffer = buffer.write(*b"..0 kB\0\0");
buffer[4] = prefix;

let frac = scaled % 100;
let buffer = if frac == 0 {
&buffer[3..6]
} else if frac % 10 == 0 {
// the decimal separator '.' is already contained in buffer[1]
buffer[2] = b'0' + (frac / 10) as u8;
&buffer[1..6]
} else {
// the decimal separator '.' is already contained in buffer[0]
buffer[1] = b'0' + (frac / 10) as u8;
buffer[2] = b'0' + (frac % 10) as u8;
&buffer[0..6]
};
// SAFETY: we know that the buffer contains only ASCII data
unsafe { from_utf8_unchecked(buffer) }
}

#[cold]
fn too_big<W: fmt::Write + ?Sized>(value: f32, dest: &mut W) -> fmt::Result {
// the value exceeds 999 QB, so we omit the decimal places
write!(dest, "{:.0} QB", value / 1e30)
}

/// `((si_prefix, factor), limit)`, the factor is offset by 10**2 to account for 2 decimal places
const SI_PREFIXES: &[((u8, f32), f32)] = &[
((b'k', 1e-1), 1e6),
((b'M', 1e-4), 1e9),
((b'G', 1e-7), 1e12),
((b'T', 1e-10), 1e15),
((b'P', 1e-13), 1e18),
((b'E', 1e-16), 1e21),
((b'Z', 1e-19), 1e24),
((b'Y', 1e-22), 1e27),
((b'R', 1e-25), 1e30),
((b'Q', 1e-28), 1e33),
];

#[test]
#[allow(clippy::needless_borrows_for_generic_args)]
fn test_filesizeformat() {
assert_eq!(filesizeformat(&0).unwrap().to_string(), "0 B");
assert_eq!(filesizeformat(&999u64).unwrap().to_string(), "999 B");
assert_eq!(filesizeformat(&1000i32).unwrap().to_string(), "1 kB");
assert_eq!(filesizeformat(&1023).unwrap().to_string(), "1.02 kB");
assert_eq!(filesizeformat(&1024usize).unwrap().to_string(), "1.02 kB");
assert_eq!(filesizeformat(0.).unwrap().to_string(), "0 B");
assert_eq!(filesizeformat(999.).unwrap().to_string(), "999 B");
assert_eq!(filesizeformat(1000.).unwrap().to_string(), "1 kB");
assert_eq!(filesizeformat(1023.).unwrap().to_string(), "1.02 kB");
assert_eq!(filesizeformat(1024.).unwrap().to_string(), "1.02 kB");
assert_eq!(filesizeformat(1100.).unwrap().to_string(), "1.1 kB");
assert_eq!(filesizeformat(9_499_014.).unwrap().to_string(), "9.49 MB");
assert_eq!(
filesizeformat(954_548_589.2).unwrap().to_string(),
"954.54 MB"
);
}
2 changes: 0 additions & 2 deletions rinja/src/filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

mod builtin;
mod escape;
#[cfg(feature = "humansize")]
mod humansize;
#[cfg(feature = "serde_json")]
mod json;
Expand All @@ -27,7 +26,6 @@ pub use self::escape::{
AutoEscape, AutoEscaper, Escaper, FastWritable, Html, HtmlSafe, HtmlSafeOutput, MaybeSafe,
Safe, Text, Unsafe, Writable, WriteWritable, e, escape, safe,
};
#[cfg(feature = "humansize")]
pub use self::humansize::filesizeformat;
#[cfg(feature = "serde_json")]
pub use self::json::{AsIndent, json, json_pretty};
Expand Down
1 change: 1 addition & 0 deletions rinja/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub use rinja_derive::Template;
#[doc(hidden)]
pub use crate as shared;
pub use crate::error::{Error, Result};
pub use crate::helpers::PrimitiveType;

/// Main `Template` trait; implementations are generally derived
///
Expand Down
1 change: 0 additions & 1 deletion rinja_actix/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ default = ["rinja/default"]
full = ["rinja/full"]
code-in-doc = ["rinja/code-in-doc"]
config = ["rinja/config"]
humansize = ["rinja/humansize"]
serde_json = ["rinja/serde_json"]
urlencode = ["rinja/urlencode"]

Expand Down
1 change: 0 additions & 1 deletion rinja_axum/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ default = ["rinja/default"]
full = ["rinja/full"]
code-in-doc = ["rinja/code-in-doc"]
config = ["rinja/config"]
humansize = ["rinja/humansize"]
serde_json = ["rinja/serde_json"]
urlencode = ["rinja/urlencode"]

Expand Down
1 change: 0 additions & 1 deletion rinja_derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ proc-macro = true
[features]
code-in-doc = ["dep:pulldown-cmark"]
config = ["dep:serde", "dep:basic-toml", "parser/config"]
humansize = []
urlencode = []
serde_json = []
with-actix-web = []
Expand Down
14 changes: 4 additions & 10 deletions rinja_derive/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1657,21 +1657,15 @@ impl<'a> Generator<'a> {
buf: &mut Buffer,
name: &str,
args: &[WithSpan<'_, Expr<'_>>],
node: &WithSpan<'_, T>,
_node: &WithSpan<'_, T>,
) -> Result<DisplayWrap, CompileError> {
if cfg!(not(feature = "humansize")) {
return Err(ctx.generate_error(
&format!("the `{name}` filter requires the `humansize` feature to be enabled"),
node,
));
}

// All filters return numbers, and any default formatted number is HTML safe.
buf.write(format_args!(
"rinja::filters::HtmlSafeOutput(rinja::filters::{name}(",
"rinja::filters::HtmlSafeOutput(rinja::filters::{name}(\
rinja::helpers::get_primitive_value(&("
));
self._visit_args(ctx, buf, args)?;
buf.write(")?)");
buf.write(")) as rinja::helpers::core::primitive::f32)?)");
Ok(DisplayWrap::Unwrapped)
}

Expand Down
1 change: 0 additions & 1 deletion rinja_derive_standalone/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ __standalone = []

code-in-doc = ["dep:pulldown-cmark"]
config = ["dep:serde", "dep:basic-toml", "parser/config"]
humansize = []
urlencode = []
serde_json = []
with-actix-web = []
Expand Down
1 change: 0 additions & 1 deletion rinja_rocket/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ default = ["rinja/default"]
full = ["rinja/full"]
code-in-doc = ["rinja/code-in-doc"]
config = ["rinja/config"]
humansize = ["rinja/humansize"]
serde_json = ["rinja/serde_json"]
urlencode = ["rinja/urlencode"]

Expand Down
1 change: 0 additions & 1 deletion rinja_warp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ default = ["rinja/default"]
full = ["rinja/full"]
code-in-doc = ["rinja/code-in-doc"]
config = ["rinja/config"]
humansize = ["rinja/humansize"]
serde_json = ["rinja/serde_json"]
urlencode = ["rinja/urlencode"]

Expand Down
15 changes: 15 additions & 0 deletions testing/tests/filters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,18 @@ fn test_linebreaks() {
"<p>&#60;script&#62;<br/>alert(&#39;Hello, world!&#39;)<br/>&#60;/script&#62;</p>",
);
}

// Regression tests for <https://github.com/rinja-rs/rinja/issues/215>.
#[test]
fn test_filesizeformat() {
#[derive(Template)]
#[template(
source = r#"{% if let Some(x) = s %}{{x|filesizeformat}}{% endif %}"#,
ext = "html"
)]
struct S {
s: Option<u32>,
}

assert_eq!(S { s: Some(12) }.render().unwrap(), "12 B");
}