Skip to content

Add support for expression-call syntax for macros#518

Merged
Kijewski merged 3 commits intoaskama-rs:masterfrom
seijikun:mr-expr-call
Jul 21, 2025
Merged

Add support for expression-call syntax for macros#518
Kijewski merged 3 commits intoaskama-rs:masterfrom
seijikun:mr-expr-call

Conversation

@seijikun
Copy link
Copy Markdown
Contributor

@seijikun seijikun commented Jul 4, 2025 •
edited
Loading

Initial implementation.

Known limitations:

  • no named arguments
{%- macro input(type) -%}
{{ type }}
{% endmacro -%}

{{- input(type=1) -}}

Depends on (and contains): #510
Fixes: #517

@seijikun seijikun force-pushed the mr-expr-call branch 9 times, most recently from b720cd4 to 486fa5a Compare July 5, 2025 22:31
@seijikun
Copy link
Copy Markdown
Contributor Author

seijikun commented Jul 5, 2025

Okay, I think I'm ready for review with this change.
The diff shows a lot of changed lines, but not many of them actually changed - I just moved them around a bit.

I added a helpers module for the generator, where complex/reusable parts of the code generation logic can be moved into - especially if they are needed multiple times. Macro invocations are a prime example of this, because it involves a lot of stuff. This reduces the function/file size in generator a bit. Though I had to make some generator methods pub(crate) for it to work.
(I initially tried to do it with functions inside generator - but clippy said 9 function parameters is too much 😅 - so the change was not entirely voluntary, but I like it)

After this major refactoring, the implementation of #517 was very straightforward.

@seijikun seijikun marked this pull request as ready for review July 5, 2025 22:39
@seijikun seijikun changed the title Add support expression-call syntax for macros Add support for expression-call syntax for macros Jul 5, 2025
@seijikun seijikun force-pushed the mr-expr-call branch 2 times, most recently from c92d179 to 36c473b Compare July 6, 2025 14:06
@seijikun
Copy link
Copy Markdown
Contributor Author

seijikun commented Jul 6, 2025 •
edited
Loading

Minor change:

  • Now can use {% if is caller defined %} to check how the macro was called. (call-block or call-expr)
  • More unit-tests

GuillaumeGomez
GuillaumeGomez previously approved these changes Jul 6, 2025
View reviewed changes
@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

GuillaumeGomez commented Jul 6, 2025

Looks good to me, thanks!

Considering how big it is, I'll let @Kijewski take a look as well as I'm sure I missed things.

@Kijewski btw, little advice: read the PR commit by commit, much simpler (my first read was horrible 😆 )

@seijikun
Copy link
Copy Markdown
Contributor Author

seijikun commented Jul 14, 2025

Rebased after the code was touched by: #527
Only affected file was: src/generator/helpers/macro_invocation.rs

GuillaumeGomez
GuillaumeGomez previously approved these changes Jul 16, 2025
View reviewed changes
@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

GuillaumeGomez commented Jul 16, 2025

Still looks good to me. Still waiting for @Kijewski's review whenever they have time and motivation. 😉

seijikun added 3 commits July 21, 2025 09:14
@seijikun
generator: Move macro invocation code generation to helper module
e68e271 
Introduced a new helpers module where code generation can be placed.
For this, some generator methods had to become pub(crate) instead of private.
@seijikun
Add unit-tests for askama-rs#517
c9f4651
@seijikun
Fix askama-rs#517: Implement expr-call syntax for macro invocations
ea6e158
@seijikun
Copy link
Copy Markdown
Contributor Author

seijikun commented Jul 21, 2025

Rebased to latest changes.

@GuillaumeGomez GuillaumeGomez requested a review from Kijewski July 21, 2025 08:12
Kijewski
Kijewski approved these changes Jul 21, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Kijewski Kijewski left a comment

Choose a reason for hiding this comment

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

Sorry for the late review and that you had to rebase multiple times!

Thank you very much, this is great!

@Kijewski Kijewski merged commit 53ddd3c into askama-rs:master Jul 21, 2025
42 checks passed
@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

GuillaumeGomez commented Jul 21, 2025

Thanks to both of you!

@Kijewski Kijewski mentioned this pull request Jul 21, 2025
Closed
@seijikun seijikun mentioned this pull request Jul 21, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kijewski Kijewski Kijewski approved these changes

@GuillaumeGomez GuillaumeGomez GuillaumeGomez left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support expression-call syntax for macros

3 participants

@seijikun @GuillaumeGomez @Kijewski