Skip to content

Implement first and blank arguments for |indent#401

Merged
Kijewski merged 2 commits intoaskama-rs:masterfrom
Kijewski:pr-indent
Apr 17, 2025
Merged

Implement first and blank arguments for |indent#401
Kijewski merged 2 commits intoaskama-rs:masterfrom
Kijewski:pr-indent

Conversation

@Kijewski
Copy link
Copy Markdown
Member

@Kijewski Kijewski commented Apr 15, 2025

@Kijewski Kijewski force-pushed the pr-indent branch 4 times, most recently from 7d78cc4 to 74030c9 Compare April 15, 2025 12:39
@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

Gonna wait for #397 to be merged before reviewing this one. 😉

@Kijewski
Copy link
Copy Markdown
Member Author

Rebased.

Comment thread askama/src/filters/alloc.rs
Comment thread askama/src/filters/alloc.rs
Comment thread askama/src/filters/alloc.rs
fn as_indent(&self) -> &str;
}

impl AsIndent for str {
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.

CowStr I suppose too?

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.

Cow<'_, T> is implemented further down.

assert_eq!(
indent("hello\n\n bar", 4, true, true).unwrap().to_string(),
" hello\n \n bar"
);
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.

Can you add test with string prefixes and basically all supported types in a separate test please?

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.

Added.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

As a follow-up we could also allow to have named arguments in indent. :)

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

Looks good to me, thanks!

@Kijewski
Copy link
Copy Markdown
Member Author

As a follow-up we could also allow to have named arguments in indent. :)

Yes, for the filter |pluralize it would make sense, too. Multiple optional parameters can be a little bit confusing without named parameters.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

Agreed!

@Kijewski Kijewski merged commit 7881bc1 into askama-rs:master Apr 17, 2025
41 checks passed
@Kijewski Kijewski deleted the pr-indent branch April 17, 2025 13:07
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.

indent filter should allow to indent empty lines and first line

2 participants