Skip to content

Filters linebreaks, paragraphbreaks and linebreaksbr only need core#486

Merged
GuillaumeGomez merged 4 commits intoaskama-rs:masterfrom
Kijewski:pr-more-core
Jun 17, 2025
Merged

Filters linebreaks, paragraphbreaks and linebreaksbr only need core#486
GuillaumeGomez merged 4 commits intoaskama-rs:masterfrom
Kijewski:pr-more-core

Conversation

@Kijewski
Copy link
Copy Markdown
Member

@Kijewski Kijewski commented Jun 14, 2025

No description provided.

@Kijewski Kijewski changed the title filter linebreaks in core Filters linebreaks, paragraphbreaks and linebreaksbr only need core Jun 14, 2025
@Kijewski Kijewski marked this pull request as ready for review June 14, 2025 13:13
Comment on lines +306 to +307
<P>HELLO &</P><P>GOODBYE!</P>
<P>HELLO &</P><P>GOODBYE!</P>]
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 old output was wrong, IMHO. The extra '\n' did not match the description.

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.

👍

Comment thread askama/src/filters/core.rs
Comment thread askama/src/filters/core.rs Outdated
Comment thread askama/src/filters/core.rs
Comment thread askama/src/filters/core.rs
Comment thread askama/src/filters/core.rs Outdated
let (has_eol, line) = strip_newline_suffix(line);
self.0.write_str(line)?;
if has_eol {
self.0.write_str("<br/>")?;
Copy link
Copy Markdown
Collaborator

@GuillaumeGomez GuillaumeGomez Jun 15, 2025

Choose a reason for hiding this comment

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

Suggested change
self.0.write_str("<br/>")?;
self.0.write_str("<br>")?;

@Kijewski
Copy link
Copy Markdown
Member Author

Kijewski commented Jun 15, 2025

I added the . to finish the close sentences, and refactored the line splitting a bit.


Omitting the /> from <br/> and other void elements was valid for all installments of HTML (not XHTML, though). But actually using this feature is an antipattern, IMHO, because it violates the robustness principle. The server should terminate void elements, but the client must not depend on it.

The current implementation would work in XML formats like RSS or ATOM, too. Without it closing the <br> it won't.

@Kijewski
Copy link
Copy Markdown
Member Author

@GuillaumeGomez, so, are you OK with keeping the />?

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

I don't mind.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 1275b58 into askama-rs:master Jun 17, 2025
42 checks passed
@Kijewski
Copy link
Copy Markdown
Member Author

Thank you for reviewing! :)

@Kijewski Kijewski deleted the pr-more-core branch June 17, 2025 14:54
@Kijewski Kijewski mentioned this pull request Jun 17, 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