Skip to content

Add named arguments for filters #403

Merged
Kijewski merged 10 commits intoaskama-rs:masterfrom
Kijewski:pr-named-filter-args
Apr 21, 2025
Merged

Add named arguments for filters #403
Kijewski merged 10 commits intoaskama-rs:masterfrom
Kijewski:pr-named-filter-args

Conversation

@Kijewski
Copy link
Copy Markdown
Member

@Kijewski Kijewski commented Apr 17, 2025

Todo:

  • write tests
  • update book

@Kijewski Kijewski changed the title Add named argument for filters Add named arguments for filters Apr 17, 2025
@Kijewski Kijewski force-pushed the pr-named-filter-args branch 7 times, most recently from 3fa45ee to 06500bd Compare April 19, 2025 00:23
@Kijewski Kijewski marked this pull request as ready for review April 19, 2025 00:24
@Kijewski Kijewski requested a review from GuillaumeGomez April 19, 2025 00:25
@Kijewski
Copy link
Copy Markdown
Member Author

The book is not updated, yet, but the changes to the code can already be reviewed. The diff is rather big, and should be digested commit-by-commit, but I think the change is worth it.

Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs Outdated
@Kijewski Kijewski force-pushed the pr-named-filter-args branch 2 times, most recently from dcaaddb to 79f8389 Compare April 19, 2025 14:34
@Kijewski Kijewski added this to the askama v0.14.0 milestone Apr 19, 2025
@Kijewski Kijewski force-pushed the pr-named-filter-args branch 2 times, most recently from 8d9cc26 to 9aa2c3e Compare April 19, 2025 16:28
Comment thread testing/tests/named_filter_arguments.rs
Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs Outdated
@Kijewski Kijewski force-pushed the pr-named-filter-args branch from 9aa2c3e to 74130a1 Compare April 19, 2025 20:01
Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs Outdated
format_args!(
"filter `{name}` accepts at most {} argument{}{}",
filter_args.len() - 1,
if filter_args.len() != 2 { "s" } else { "" },
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.

The comparison to 2 made me bug for a while. XD

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just imagine that rust's arrays are 1-based. :D

Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs
Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread askama_derive/src/generator/filter.rs Outdated
Err(ctx.generate_error(r#"use filter format like `"a={} b={}"|format(a, b)`"#, node))
}

fn visit_fmt_filter(
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.

Since this is the same code as format, shouldn't it be merged into one function and we only pass the filter name for the error messages?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's different from format. It's {{ var | fmt("{:?}") }} vs {{ "{var1} {var2}" | format(var1 = 1, var2 = 2) }}.

Comment thread askama_derive/src/generator/filter.rs Outdated
Comment thread book/src/filters.md Outdated
The order of the named arguments does not matter, but named arguments must come after
positional (i.e. unnamed) arguments.

E.g. the filter `pluralize` takes two optional arguments: `sg` and `pl` (singular and plural),
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.

Btw, why not using the full name instead of sg/pl?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The words are just too long, and sg and pl are universally understood abbreviations, I think.

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.

It's the first time I ever saw them, so I'm definitely not convinced. ^^'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

If there is a debate in the first place, I think it makes more sense to just go with with the full word over abbreviations.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You can add a commit to the PR to change the spelling.

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.

Thanks!

@Kijewski Kijewski force-pushed the pr-named-filter-args branch from 74130a1 to 5ccc0d8 Compare April 20, 2025 21:07
@Kijewski Kijewski merged commit 7c5deda into askama-rs:master Apr 21, 2025
41 checks passed
@Kijewski Kijewski deleted the pr-named-filter-args branch April 21, 2025 21:11
@Kijewski Kijewski mentioned this pull request Apr 21, 2025
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.

2 participants