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

Fix the parser behavior for texts containing curly-braces when the Attributes extension is enabled #1040

Merged
merged 5 commits into from
Aug 14, 2024

Conversation

xavierlacot
Copy link
Contributor

The issue

As stated in #1035 (comment), the PR #986 , which introduced the ability to create valueless attributes using the Attributes extension, causes issues when a markdown document includes text strings that contain curly braces.

For example, the following markdown text

Elastic{ON} Tour San Francisco

is rendered by the official commonmark dingus as:

<p>Elastic{ON} Tour San Francisco</p>

The league/commonmark library with the "Attributes" extension enabled was used to render such markdown the same way until the commit 2138460. Since then, it renders this markdown as:

<p>Elastic Tour San Francisco</p>

which is obviously something we do not want.

What does this PR

This PR reverts the changes introduced in #986 and #1035 and adds more test cases, to show how to create valid boolean-attributes.

A section has also been added in the documentation to show how to write attributes that will be rendered as empty-value attributes in HTML.

Copy link
Member

@colinodell colinodell left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

While I do like the simplicity of allowing {attribute}, I think it's entirely reasonable to revert that feature in favor of the more explicit approach that is less likely to collide with other user input.

@colinodell colinodell merged commit 393a451 into thephpleague:2.5 Aug 14, 2024
16 checks passed
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