Skip to content

Conversation

theashguy
Copy link
Collaborator

@theashguy theashguy commented May 27, 2021

Prior to this PR Render could only handle attributes without any punctuation in them OR hyphenated attributes. There are however a range of weird and wonderful attributes inside SVG and that various Javascript libraries use that require other types of punctuation OR multiple instances of punctuation in an attribute.

This PR improves the state of this (though I believe there are still some holes in how it works-- eg, I'm not sure it works if attributes begin with punctuation).

Additionally, it opens up the ability for attribute content to have special characters in it-- a requirement if you want to pre-load some data into a JS script for instance as a json object.

@theashguy theashguy changed the title Fixes to the way Render handles tags Fixes to the way Render handles attributes May 27, 2021
for (key, value) in attributes.drain() {
write!(writer, " {}=\"", key)?;
escape_html(&value, writer)?;
write!(writer, "{}", value)?;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here we stop escaping the html of data that goes into attributes. This does mean library users will need to do this themselves. In future I'd suggest some kind of marker that would escape this unless it was attached.


let punct = input.parse().unwrap();
name.push_punct(punct);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This loop is similar to the implementation in parse_separated_nonempty_with. Key thing I was looking to do here was to give us the ability to modify how and when this accrues / breaks so we can better handle the edge cases. I'm not sure I ended up going very far with this but think it probably makes sense to hold onto this here.


if !not_punned {
not_punned = input.peek3(syn::Token![=]);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because there are multiple sets of punctuation that can occur in a row we look forward to see if any of the next 3 Punctuated's have an equals in it. This may ultimately become a limitation of the library that it can only handle 2 distinct piece of punctuation before the equals though that would be a vast improvement.

Some(p) => p.as_char().to_string(),
None => "".to_string(),
}
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This properly collapses our punctuation and ident fields back into a proper string when there are multiple.

html! { <input x-bind:class={"{ 'active': tab === 'foo' }"} /> },
r#"<input x-bind:class="{ 'active': tab === 'foo' }"/>"#
);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This block is the only addition to the tests-- the rest is rustfmt.

@theashguy
Copy link
Collaborator Author

Have added my notes for review. @Schniz @vpzomtrrfrt @Tarnadas

Self: Sized,
{
let mut buf = String::new();
self.render_into(&mut buf).unwrap();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This stuff makes the Render trait more portable-- we still need sized as its a dependency of the writer structure we're doing, but we can at least scope this requirement to the render method so that it'll only complain about being passed around when used directly in the html! macro... 🤞

write!(writer, " {}=\"", key)?;
escape_html(&value, writer)?;
write!(writer, "\"")?;
if key.chars().nth(0).unwrap_or('.') == 'b' && key.chars().nth(1).unwrap_or('.') == '!' {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not my favorite addition as its a bit of a hack-- definitely want a better way to do this in the future. This interprets b! at the start of an attribute as a boolean attribute in HTML, which will cause it to optionally render the key based on a value of "true".

Mostly because we need some way to do this today as things like selected='false' in an option isn't actually valid html.

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

Successfully merging this pull request may close these issues.

1 participant