Skip to content

parser: recognize comments in macro calls#487

Merged
Kijewski merged 1 commit intoaskama-rs:masterfrom
Kijewski:6051930453639168
Jun 16, 2025
Merged

parser: recognize comments in macro calls#487
Kijewski merged 1 commit intoaskama-rs:masterfrom
Kijewski:6051930453639168

Conversation

@Kijewski
Copy link
Copy Markdown
Member

@Kijewski Kijewski requested a review from GuillaumeGomez June 16, 2025 17:19
@Kijewski Kijewski added bug Something isn't working fuzzing error A bug found by fuzzing parser Related to the parser labels Jun 16, 2025
@Kijewski
Copy link
Copy Markdown
Member Author

I was too lazy to file an issue that explains the error found by the fuzzer.

Another option would have been to parse comments, but I don't see when you would need them, we have {# comments #} after all. Also, it would enable eye-sores like ↓ which I really don't want to see in actual use:

{{ x!(// }}
) }}

:)

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

I'm not sure it's a good idea, or at least not the right way to handle it. For bigger (rust) macros, it's very common to have code comments to explain what's going on. And for crates like uniffi-rs, which generates code, it could be a big blocker to not be able to have comments in macro calls. So if they write a!(// haha), then it's their problem, not ours.

@Kijewski Kijewski changed the title parser: reject comments in macro calls parser: recognize comments in macro calls Jun 16, 2025
@Kijewski
Copy link
Copy Markdown
Member Author

I implemented recognizing comments, but I would still advice against using them.

struct InnerLineDoc;

#[derive(Template)]
#[template(ext = "html", source = "{{ e!(/*! hello) }}")]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What happens for this case:

{{ e!(//)
}}

?

Copy link
Copy Markdown
Member Author

@Kijewski Kijewski Jun 16, 2025

Choose a reason for hiding this comment

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

error: expected ) but found }

The line comment // makes the parser skip the closing paren ), so the next character it sees is }, but it expects ) to close the macro call.

I added tests to ensure that behavior (and found a typo in my implementation this way, oops).

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

Thanks a lot!

@Kijewski Kijewski merged commit 568ced0 into askama-rs:master Jun 16, 2025
42 checks passed
@Kijewski Kijewski deleted the 6051930453639168 branch June 16, 2025 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fuzzing error A bug found by fuzzing parser Related to the parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants