Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect nil attributes at runtime #208

Merged

Conversation

luontola
Copy link
Contributor

Before this change (html [:br (identity nil)]) used to produced "<br></br>". After this change it will produce "<br />".

The use case where this can matter is if the user has a function which returns either a map of attributes or nil.

@luontola luontola force-pushed the detect-runtime-attrs-vs-content branch from 7c11cdb to d9a8b7d Compare December 14, 2023 21:12
Copy link
Owner

@weavejester weavejester left a comment

Choose a reason for hiding this comment

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

Some very minor formatting suggestions. Could you also ensure the commit message uses plaintext rather than markdown formatting. So something like:

Fix formatting of nil attributes at runtime

Fix forms like (html [:br (identity nil)]) to produce "<br />" or "<br>"
rather than "<br></br>".

src/hiccup/compiler.clj Outdated Show resolved Hide resolved
src/hiccup/compiler.clj Outdated Show resolved Hide resolved
@weavejester
Copy link
Owner

Thanks for the fix! And apologies for taking a little while to take a closer look.

@luontola luontola force-pushed the detect-runtime-attrs-vs-content branch from d9a8b7d to 181b936 Compare December 21, 2023 12:04
@luontola
Copy link
Contributor Author

Changes done.

Fix forms like (html [:br (identity nil)]) to produce "<br />" or "<br>"
rather than "<br></br>".

The use case where this issue can happen is if the user has a function
which returns either a map of attributes or nil.
@luontola luontola force-pushed the detect-runtime-attrs-vs-content branch from 181b936 to 7196a47 Compare January 5, 2024 17:06
@luontola
Copy link
Contributor Author

luontola commented Jan 5, 2024

@weavejester This too is ready

@weavejester weavejester merged commit 92f18b7 into weavejester:master Jan 9, 2024
1 check passed
@luontola luontola deleted the detect-runtime-attrs-vs-content branch January 9, 2024 13:47
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