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

Fork tinytemplate? #758

Closed
NobodyXu opened this issue Feb 1, 2023 · 8 comments
Closed

Fork tinytemplate? #758

NobodyXu opened this issue Feb 1, 2023 · 8 comments
Assignees

Comments

@NobodyXu
Copy link
Member

NobodyXu commented Feb 1, 2023

IDK this is a good idea, but I really want to fork it and simplify its internals and drop dep serde and serde-json, similar to this PR bheisler/TinyTemplate#24 but never get merged since the owner does not want to release a new major version.

I think that tinytemplate could expose a new trait Introspectable:

pub enum Value<'a> {
    Null,
    Boolean(bool),
    SignedInteger(i128),
    UnsignedInteger(u128),
    Float(f32),
    Double(f64),
    String(&'a str),
    ArraySlice(&'a [Self]),
    ArrayIter(Box<dyn Iterator<Item = Self> + 'a>),
    Object(&'a dyn Introspectable),
}

trait Introspectable {
    fn inspect_field(&self, field_name: &str) -> Value<'_>;
}

then change tinytemplate::TinyTemplate::render to:

pub fn render<'a, C: 'a>(&self, template: &str, context: C) -> Result<String>
where
    C: Into<Value<'a>>,

In the future, I also have plan to make writing package.metadata.binstall easier by allowing user to specify arbitary keys there that can be used inside pkg-url and bin-dir, or even allowing additional templates.

This will be especially helpful if they need to override package.metadata.binstall for specific targets.

@passcod
Copy link
Member

passcod commented Feb 1, 2023

I don't know that our usage of templates really merits a) a full fork and b) if we're going to have our own implementation, a crate. It strikes me the usage we have of it could be a few string replaces, or a scan-parse and render if we want to be fancy/more complete.

@passcod
Copy link
Member

passcod commented Feb 1, 2023

Specifically, the format is ARBITRARY TEXT? { WHITESPACE* NAME-TOKEN WHITESPACE* } ...

Technically there's conditionals, loops, and formatters, but a) we don't have much use for these and b) we never advertise that these features exist or that the internal templating is done with a particular crate, so there should be no expectation that any of that is possible.

Parsing the above is straightforward even with a hand-written parser, it would then produce an AST which would really be an abstract syntax array, something like:

assert_eq!(
	Template::from_str("{ repo }/releases/download/{ version }/"),
	Ok(Template(vec![
		Variable("repo"),
		Text("/releases/download/"),
		Variable("version"),
		Text("/"),
	]))
);

which can be "rendered" like (~pseudo-rust):

impl Template {
	fn render_into(&self, values: SomeKindOfMap<&str, dyn Display>, buffer: &mut impl Write) -> Result<()> {
		for item in &self.0 {
			match item {
				Text(text) => write!(buffer, "{}", text)?,
				Variable(name) => values.get(name).ok_or(Error::MissingVar(name))?,
			}
		}

		Ok(())
    }

	fn render(&self, values: SomeKindOfMap<&str, dyn Display>) -> Result<String> {
		let mut buf = String::with_capacity(self.0.iter().map(|item| match item { Variable(_) => 0, Text(t) => t.len() }).sum());
		self.render_into(values, &mut buf)?;
		Ok(buf)
	}
}

@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 2, 2023

Yeah, we probably don't need that many features provided by tinytemplate.

I was going to say that conditional might be useful, but tinytemplate only supports boolean inside conditions and supports no comparsion operators, so that features is useless to use anyway.

For the loop, it's only useful for array/Vec, but we really don't provide anything like that.

Considering that tinytemplate is designed for html rendering, these features make sense, but they are indeed useless for us.

@passcod Perhaps we should really consider making our own dynamic formatting crate focused on simplicity modeled after format!?

@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 4, 2023

Another advantages of having our own impl is that checking whether a variable is present will be much easier.
In #757, I use str::contains to check whether the template uses a variable, which is a terrible solution.

@passcod
Copy link
Member

passcod commented Feb 4, 2023

I'm implementing this.

@passcod passcod self-assigned this Feb 4, 2023
@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 4, 2023

I'm implementing this.

I was actually writing something similar, but I am quite busy recently and I will let you have the fun!

My design is to have sth. like:

pub enum Item<'a> {
    Var(VarName<'a>), 
    Text(Literal<'a>),
}
/// VarName has new, const unsafe new_unchecked, same for Literal
pub struct VarName<'a>(beef::Cow<'a, str>);
pub struct Template<'a>(beef::Cow<'a, [Item<'a>]>);

My reasoning for this design is that we might want to use the template engine in const, where we know the input are valid template.

I.e. gh_crate_meta has some fallback pkg-fmt that is entirely known at compile-time.

I was thinking of manually creating Item (consists of var and literal), then concat Templates at runtime.

I use beef::CoW here so that creation of Item/Template would not need any allocation at all and can just hold a static reference instead.

Also, beef::Cow is 24 bytes large, 8 bytes smaller than std::borrow::Cow.
Ok 64-bit platform we could also switch to beef::lean::CoW, which would be 16 bytes, even smaller and the same size as a slice.

Hope this would help you!

@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 4, 2023

P.S. It will be great if we can have format_args! like macro, but with lazy binding to variables.

@passcod
Copy link
Member

passcod commented Feb 5, 2023

Initial implementation at #766, to be improved on

@NobodyXu NobodyXu mentioned this issue Feb 5, 2023
6 tasks
@passcod passcod closed this as not planned Won't fix, can't repro, duplicate, stale Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants