Implement Render trait for Option

[Svenskunganka]

Given a template struct of this:

#[derive(TemplateOnce)]
#[template(path = "login.stpl")]
struct Login {
  username: Option<String>
}

PR allows to do this:

<input type="text" name="username" value="<%= username %>">

instead of:

<input type="text" name="username" value="<%= if let Some(usr) = username { usr } %>">
// or
<% if let Some(usr) = username { %>
<input type="text" name="username" value="<%= usr %>">
<% } %>
// or (inefficient, allocates empty String and renders it):
<input type="text" name="username" value="<%= username.unwrap_or_else(|| "".to_owned()) %>">

The drawback is that it's easier to accidentally render dead markup, e.g:

<span><%= username %></span>
// When None, results in:
<span></span>

I also have an implementation for Result<T, E> where T: Render over in https://github.com/Svenskunganka/sailfish/tree/render_result but after some consideration I don't think that's a good idea - but please give your opinion as well! See also #52

[netlify[bot]]

✔️ Deploy Preview for rust-sailfish ready!

🔨 Explore the source changes: 7ce8cd9ffcd583311b33322aad3ec79e86d06db0

🔍 Inspect the deploy log: https://app.netlify.com/sites/rust-sailfish/deploys/622c8bdf00f4120009bd3723

😎 Browse the preview: https://deploy-preview-84--rust-sailfish.netlify.app

[vthg2themax]

@jdrouet @Kogia-sima What do you think? As for me, I like the idea. It would make coding less verbose, however the issue I have is about the dead markup problem. That would be harder to find, and debug if there is an issue I would think, which is why Rust has safeguards like an Option structure. I kind of like the requirement the coder has to acknowledge the possibility of it being nothing, which is why I lean toward not wanting us to implement this. @Svenskunganka Please let me know if you have any thoughts on this as well, and thank you for your pull request!

[godofdream]

I think it is an good Idea, however I would rather see something like a different syntax.

<%= username %> for strings
<%=~ username %> for option stuff

[vthg2themax]

@godofdream Why the different syntax? Just curious. I like the current one because it reminds me of old ASP.

[godofdream]

@vthg2themax this would ommit the risk of accidentially render Options

[vthg2themax]

@godofdream I can understand that then. Perhaps the code committer may want to make the change possibly?

[Svenskunganka]

Sorry for taking a while to respond!

Dead markup is definitely a valid concern. But Sailfish isn't strictly a HTML templating library - it can be used to render any kind of text. If I recall correctly there used to be a crate on crates.io that used Sailfish to render a Debian package manifest or something along those lines. For non-HTML formats where "dead markup" isn't an issue, this could be an ergonomic addition.

It is still possible to render dead markup, but I guess harder to do on accident:

<span><%= if let Some(usr) = username { usr } %></span>
// when None, results in:
<span></span>

---

I think it is an good Idea, however I would rather see something like a different syntax.

<%= username %> for strings
<%=~ username %> for option stuff

Is the idea that <%=~ ... %> would inform Sailfish to remove the surrounding HTML tags when it's Option::None? That would unfortunately make Sailfish context-aware of HTML. Even though non-HTML uses of Sailfish is in the minority, I think it's important to support such use-cases.

What about cases like this where you'd want the operator to remove both the <li> and <span>?

<ul>
  <li>
    <span><%=~ an_option %></span>
  </li>
</ul>

[vthg2themax]

@godofdream @Svenskunganka @Kogia-sima I have been thinking it would be cool if we had like a 'sailfish-web' option for our crate that would allow us to opt-in to implementing specific ergonomic changes to our project such as making it easier for developers to handle these option elements in a more web focused way that gives preference to developer productivity if they opt-in, but also still will allow things to proceed quickly, and safely. What do you guys think?

[PaulDotSH]

@godofdream @Svenskunganka @Kogia-sima I have been thinking it would be cool if we had like a 'sailfish-web' option for our crate that would allow us to opt-in to implementing specific ergonomic changes to our project such as making it easier for developers to handle these option elements in a more web focused way that gives preference to developer productivity if they opt-in, but also still will allow things to proceed quickly, and safely. What do you guys think?

Well I can implement features you'd like into sailfish-web, under some feature maybe called "convenience"?