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.

Sign up for GitHub

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

Accept literals in attribute names #396

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Accept literals in attribute names #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9bd607d
Accept literals in attribute names
RedPhoenixQ Oct 19, 2023
54ebfd8
Accept literals in attribute names
RedPhoenixQ Nov 5, 2023
c33ee62
Propper handling of literals in name_to_string()
RedPhoenixQ Nov 6, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
[#377](https://github.com/lambda-fairy/maud/pull/377)
- Implement `Render` for `Arc<T>`
[#380](https://github.com/lambda-fairy/maud/pull/380)
- Accept literals in attribute names
[#396](https://github.com/lambda-fairy/maud/pull/396)

## [0.25.0] - 2023-04-16

Expand Down
28 changes: 28 additions & 0 deletions maud/tests/basic_syntax.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,34 @@ fn hyphens_in_attribute_names() {
);
}

#[test]
fn string_literals_in_attribute_names() {
let result = html! { this "@sentence:-is.not"="false" of-course {} };
assert_eq!(
result.into_string(),
r#"<this @sentence:-is.not="false" of-course></this>"#
);
}

#[test]
fn raw_string_literals_in_attribute_names() {
let result = html! { this r#"@sentence:-is.not"#="false" of-course {} };
assert_eq!(
result.into_string(),
r#"<this @sentence:-is.not="false" of-course></this>"#
);
}

#[test]
fn other_literals_in_attribute_names() {
let result =
html! { this b"byte_string"="false" 123="123" 2.5 true 'a'="a" b'b'="b" of-course {} };
assert_eq!(
result.into_string(),
r#"<this byte_string="false" 123="123" 2.5 true a="a" b="b" of-course></this>"#
Copy link
Owner

Choose a reason for hiding this comment

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

I think string literals, and maybe integers, are enough.

Floating point numbers overlap with class shorthand which also uses .. There's no use case for a single character. And no other part of Maud accepts byte strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lambda-fairy, addressing this issue, to keep ast::name_to_string() as an infallible api (always returning String without errors) I handled all the cases since I didn't see any issue with also handling ByteStr and others so they produce the expected internal string. Current implementation should always produce valid html.

The class shorthand shouldn't be broken since classes starting with numbers are disallowed in CSS so all classes before should still be possible. Also the only time a class starting with a number wouldn't be interpreted as a class would be 123.123 (space followed by numbers with dot and no spaces), but 123 .123, anything.123 and .classname123.123 would still parse as a class shorthand for the class 123.

Copy link
Owner

Choose a reason for hiding this comment

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

Having the interpretation change just by adding a space is the confusing part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#398 fixes this issue by running the existing literal() function in the parser when encountering a literal, reusing existing error behaviour. I've added Int as an accepted literal, but can revoke that change if you decide that Int shouldn't be allowed.

);
}

#[test]
fn class_shorthand() {
let result = html! { p { "Hi, " span.name { "Lyra" } "!" } };
Expand Down
19 changes: 18 additions & 1 deletion maud_macros/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use proc_macro2::{TokenStream, TokenTree};
use proc_macro_error::SpanRange;
use syn::Lit;

#[derive(Debug)]
pub enum Markup {
Expand Down Expand Up @@ -217,5 +218,21 @@ pub fn join_ranges<I: IntoIterator<Item = SpanRange>>(ranges: I) -> SpanRange {
}

pub fn name_to_string(name: TokenStream) -> String {
name.into_iter().map(|token| token.to_string()).collect()
name.into_iter()
.map(|token| {
if let TokenTree::Literal(literal) = token {
match Lit::new(literal.clone()) {
Lit::Str(str) => str.value(),
Lit::Char(char) => char.value().to_string(),
Lit::ByteStr(byte) => {
String::from_utf8(byte.value()).expect("Invalid utf8 byte")
}
Lit::Byte(byte) => (byte.value() as char).to_string(),
_ => literal.to_string(),
}
} else {
token.to_string()
}
})
.collect()
}
13 changes: 7 additions & 6 deletions maud_macros/src/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,12 +702,13 @@ impl Parser {
/// Parses an identifier, without dealing with namespaces.
fn try_name(&mut self) -> Option<TokenStream> {
let mut result = Vec::new();
if let Some(token @ TokenTree::Ident(_)) = self.peek() {
self.advance();
result.push(token);
} else {
return None;
}
match self.peek() {
Some(token @ TokenTree::Ident(_)) | Some(token @ TokenTree::Literal(_)) => {
self.advance();
result.push(token);
}
_ => return None,
};
let mut expect_ident = false;
loop {
expect_ident = match self.peek() {
Expand Down
Loading