Support toggling attributes that have a value

[DataTriny]

Background

It is already possible to toggle empty attributes like so:

html! { elem attr[toggle] { ... } }

On most cases (if not all), adding an empty value to an attribute will do the same as not adding anything. But if you are dynamically building an element with multiple optional attributes, the generated HTML will be unnecessary longer.

Proposition

I would suggest combining the syntax for toggling empty attributes and the syntax for defining attributes with a value.

Therefore, it would be possible to write something like this:

let show_attr = true;
let attr_value = "...";
html! {
    elem attr[show_attr]=(attr_value) { ... }
}

Alternatives

• Don't modify maud at all and rely on conditional structures to build attributes. This is fine if the element only contains one optional attribute, but is impractical as soon as there are more:

  let show1 = true;
  let value1 = "...";
  let show2 = true;
  let value2 = "...";
  html! {
      @if show1 && show2 {
          elem attr1=(value1) attr2=(value2) { ... }
      }
      @else if show1 && !show2 {
          elem attr1=(value1) { ... }
      }
      @else if !show1 && show2 {
          elem attr2=(value2) { ... }
      }
      @else {
          elem { ... }
      }
  }

• Expect the toggle after the value:

  html! {
      elem attr=(attr_value)[show_attr] { ... }
  }

  I don't have a preference for one or the other, they both make sense and should not introduce a breaking change in the syntax parsing logic.

• Allow attribute values to be Option<T>:

  html! {
      // attr would be shown.
      elem attr=(Some(attr_value)) { ... }
  
      // attr would be hidden.
      elem attr=(None) { ... }
  }

  This makes for a shorter syntax, but might be harder to read.

What do you think? If you are interested, I already have a PR implementing the first solution.

[lambda-fairy]

Thanks for mapping this out!

I've had this requested elsewhere (#240 (comment)), and markup.rs supports it as well, so I think the idea has merit.

Before going into syntax – I wonder if we should only support Option<T>, like your third option?

My reasons are:

• You can implement the boolean flag approach on top of the Option<T> one, but not vice versa.
• More importantly: with boolean flags, it's easy for them to get out of sync with the thing they're controlling. Especially if there are many of them.

@DataTriny @zopieux what do you think?

[DataTriny]

Hi @lambda-fairy ,
The boolean syncing issue didn't really came to my mind at the time, but it is something to consider indeed. Plus I think most of the time you would use this syntax to toggle an optional value, so you would probably have an Option<T> already.

Looking back at my first comment, the second alternative would conflict with str indexing syntax I guess.

[zopieux]

I like the suggestion of only accepting Option<T>. I'm a bit confused about the API though (not the syntax itself but the expected behavior).

To sum up, do I understand correctly that it would be:

let toggle = true;
html! { checkbox checked[toggle] {} }  // current syntax for a valueless attr
// ↑ Would this syntax be deprecated?

// Non-Option
let id = 42;
html! { div data-id=(id) {} }  // <div data-id="42">
let id = "42";
html! { div data-id=(id) {} }  // <div data-id="42">

// Option<not bool>
let id = Some(42usize);
html! { div data-id=(id) {} }  // <div data-id="42">
let id: Option<usize> = None;
html! { div data-id=(id) {} }  // <div>
let flag = Some("false");
html! { div data-flag=(flag) {} }  // <div data-flag="false">
let flag = Some("");
html! { div data-flag=(flag) {} }  // <div data-flag="">

// Option<bool>.
let toggle = Some(true);
html! { checkbox checked=(toggle) {} }  // <checkbox checked>
let toggle = Some(false);
html! { checkbox checked=(toggle) {} }  // <checkbox>
let toggle: Option<bool /* not that it matters though */> = None;
html! { checkbox checked=(toggle) {} }  // <checkbox>

[DataTriny]

To be more precise, my proposal only involved passing Option<&str> as a value. Allowing Option<bool> for toggling valueless attributes seems a bit weird to me since both None and Some(false) could be used to hide the attribute. If we really want to allow Option type to toggle valueless attributes, maybe Option<()> would make more sense, it clearly indicate that there is no value there.

[zopieux]

Ok @DataTriny, so if my understanding is correct:

1. get rid of the attr[bool] syntax
2. have attr=(value) everywhere, with value: Option<T>, meaning that what is currently attr=(s) would have to be attr=(Some(s)) or attr=(s.into())
3. do not produce the attr=… html at all if value is None
4. special-case value = Some<()> to produce a value-less attr (checked instead of checked="")

While that makes sense, don't you think point 2. would make html! blocks quite verbose with into() or Some everywhere?

[DataTriny]

Hi @zopieux ,

Since my original proposal didn't target valueless attributes, I wasn't suggesting to get rid of the attr[toggle] syntax. I wasn't suggesting to get rid of the current syntax for attributes with value either, but rather extending it so that it would accept both &str and Option<&str> values (introducing no breaking change).

So to go through all your points:

1. Keep this syntax as is, unless we want to go the Option<()> route.
2. Accept both &str and Option<&str> as values if possible, so that existing code don't have to be modified, and attributes with values that don't need to be toggled could still use the old, simpler syntax.
3. Correct.
4. To be determined, but wasn't part of my original proposal, otherwise correct.

[zopieux]

That makes sense. Backward compatibility notwithstanding, I must admit keeping the toggle syntax sounds important so that bothchecked[t] and .class[t] stay nice and short.

I'm not sure we need 4. in that situation.

So yeah, big +1 on that proposal!

[lambda-fairy]

Thanks for the proposal!

I would make two minor changes:

• For syntax, I prefer: attr=[attr_value]. This communicates both the presence of a value (=) and the ability to toggle ([]). Also, I believe this will be easier to implement than overloading the existing syntax.
• Allow all Option<T: Render>, not just Option<&str>.

So here's what I'm thinking:

If there is an expression caption with type Option<T: Render>, then

img alt=[caption];

is valid syntax. If caption is Some, then its value will be used; if None, then the alt attribute will be omitted.

If that sounds good to you @DataTriny @zopieux, would one of you like to try implement it? 🙂

[zopieux]

I can try, but I'll be away from my development workstation for a few days :-) Thanks for weighting in, I'm fine with your latest suggestions.

[DataTriny]

Hello, and thank you for taking the time to investigate this proposal.

Not being familiar at all with Rust's procedural macros, I started doing some research which led me to think that overloading the existing syntax would probably be hard. Introducing the =[..] syntax would simplify this, but it apparently is tricky to make sur you actually have an Option<T>: we of course want to allow literal values and variables, but if type aliases are involved then the procedural macro will not work.

Do you have more knowledge on this @lambda-fairy ?

I can also try to implement it, once this point is resolved.

[lambda-fairy]

@DataTriny yep that's right. In Rust, macro expansion happens before name resolution and type checking. So information from the latter phases won't be available.

I don't think that will be a problem in our case, though. Because if we see =[..] syntax, we can generate code that assumes the expression is an Option<T>, without checking it. The compiler will raise an error later if that's incorrect.

If you're curious about how an overloading solution works, check out the markup.rs code (macro, runtime). It works around the lack of type information by putting the decision in the Render trait. (There's no need to do that here; just thought you might find it interesting 🙂 )