-
Notifications
You must be signed in to change notification settings - Fork 12
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
Deny all rust keywords as macro names #23
Deny all rust keywords as macro names #23
Conversation
Please add a ui test too please. |
There always is a test for super: macro-super.rs. Should I add tests for the other keywords, too? |
|
||
/// Ensure that all strings are UTF-8, because we use `from_utf8_unchecked()` further down. | ||
#[test] | ||
fn ensure_utf8() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need to duplicate these consts between the two crates? Can't we make them public behind #[doc(hidden)]
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need to duplicate these consts between the two crates?
Sadly, I think yes. The list items are not the same with loop self Self
missing from the generator list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then why not add another list for these 3 keywords in rinja_parser
? Having some much code duplicated is itching me strongly. ^^'
If you want I don't mind doing it. In this case just merge this PR and I'll take a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case just merge this PR and I'll take a look.
Thanks! Sure, go ahead. :)
Oh if there is one already then it's fine. 👍 |
Per https://github.com/djc/askama/pull/1044#discussion_r1602289767.