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

Allow setting attributes without values in Attributes extension #985

Open
svenluijten opened this issue Jul 29, 2023 · 1 comment · Fixed by #986
Open

Allow setting attributes without values in Attributes extension #985

svenluijten opened this issue Jul 29, 2023 · 1 comment · Fixed by #986
Labels
do not close Issue which won't close due to inactivity enhancement New functionality or behavior
Milestone

Comments

@svenluijten
Copy link
Contributor

svenluijten commented Jul 29, 2023

Description

I'm writing my own extension that makes use of the capabilities of the baked-in attributes extension. However, upon testing, I found that it is currently not possible to add an attribute without a value on an element.

I would like to be able to add an attribute without any value to prevent me (and the eventual users of my own extension) from having to add an arbitrary value every time like so:

![image](/assets/image.jpg){my-attribute=true}

Example

Consider parsing the following Markdown with CommonMark, with the Attributes extension loaded:

![image](/assets/image.jpg){my-attribute}

I would expect the following HTML to be rendered:

<p><img src="/assets/image.jpg" alt="image" my-attribute /></p>

However, I'm seeing the following:

<p><img src="/assets/image.jpg" alt="image" />{my-attribute}</p>

I believe this has to do with the League\CommonMark\Util\RegexHelper::PARTIAL_ATTRIBUTEVALUESPEC constant enforcing the presence of an equals character (=) and a subsequent value.

If this were to be changed, the way we extract the attribute name and value would also have to be changed in League\CommonMark\Extension\Attributes\Util\AttributesHelper.

I'll try to whip up a quick PR to get this going, but would love some input if this is even something you want to consider for this project before I dive in fully 🙂

Did this project help you today? Did it make you happy in any way?

I'm exceptionally happy with the way this project is set up. It was super easy to dig and start working on my own extension because of the outstanding documentation and the easy-to-follow code when I needed to dig deeper and do some source-diving. So thank you ❤️

@svenluijten svenluijten added the enhancement New functionality or behavior label Jul 29, 2023
@colinodell
Copy link
Member

I really like this proposal! Let's do it :)

I believe this has to do with the League\CommonMark\Util\RegexHelper::PARTIAL_ATTRIBUTEVALUESPEC constant enforcing the presence of an equals character (=) and a subsequent value.

If possible, we should try to avoid changing those PARTIAL_ regular expressions in RegexHelper. Those are based directly on the commonmark.js reference parser and are used elsewhere in the library; any changes could have unintended side effects on other features like parsing HTML content. Instead, we should be able to modify how they are used in AttributesHelper::SINGLE_ATTRIBUTE.

That regular expression basically says to match:

  • A CSS class or ID (. or # followed by some characters); or,
  • An attribute name followed by an "attribute value spec" (= then a value)

Making the "attribute value spec" part optional (by adding a ? immediately afterward) should be enough to get the regex to match. Inside the parseAttributes() method, we can check if \explode('=', $attribute, 2); only resulted in 1 item, meaning the value part was omitted and should be set to true behind-the-scenes.

Would you be open to revising your PR to try this approach? I haven't actually tested it, but I think it'll put you on the right path :)

I'm exceptionally happy with the way this project is set up. It was super easy to dig and start working on my own extension because of the outstanding documentation and the easy-to-follow code when I needed to dig deeper and do some source-diving. So thank you ❤️

Thank you so much for that kind feedback! 😊

@colinodell colinodell added this to the v2.5 milestone Jul 31, 2023
@colinodell colinodell added the do not close Issue which won't close due to inactivity label Jul 31, 2023
@colinodell colinodell linked a pull request Jul 31, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not close Issue which won't close due to inactivity enhancement New functionality or behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants