-
Notifications
You must be signed in to change notification settings - Fork 856
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
attr_list breaks if the value contains braces, requires HTML-encoding them #1390
Comments
Interesting issue. When the feature was implemented, we did not foresee any sort of formatted text being passed in as a value, which explains why you need to jump through hoops to make it work. For example, we might expect a class name or a value for an HTML title attribute. In either case, there would be no markup. Although, I suppose a title attribute could conceivably contain curly braces. I'll have to think about this. Regarding the specific feature of mkdocs-material; I find it very strange that the feature even exists. I can't imagine any scenario where anyone would want that specific feature. Of course, use cases should not be limited by my imagination (or lack thereof), but it does give me pause. I will certainly not be adding specific support for newlines or other advanced formatting. The only thing I would consider here at all is to maybe allow curly braces within quoted values. The tricky thing about this is that the initial regex does not parse the keys and values. It simply grabs the entire string (between the curly braces) and then passes that on to a separate key/value parser. We currently grab that string with I'm curious how Superfences handles this scenario. Any input from @facelessuser or @squidfunk is welcome. |
Thanks for the evaluation, @waylan.
A user wanted to explicitly set the text for the clipboard button, which we implemented in squidfunk/mkdocs-material#6086. We consider this to be highly experimental and are considering removing this functionality again, if it turns out to be complicated for users to use. If you follow the issue, you can learn that the use case is that some users have comments in their code blocks which they don't want to be copied. If this turns out to be too involved to implement, or it can only be partially solved (e.g. curly braces are okay, but everything else needs to be escaped) we will remove the feature again, since we don't consider the necessity for escape everything with HTML entities to be very user-friendly. It's too prone to errors. |
SuperFences uses With that said, I can see how and why someone would desire sane handling. I do think it is possible to craft a solution that could consume strings properly. But I also would probably not care if it was decided |
@squidfunk thanks for the explanation. I commented on your issue with a suggestion for a different way to solve your problem. Whether you go that route or another is your choice. But the best you can hope for from us here is to allow curly braces in values. All other limitations will remain. @facelessuser the one question I was hoping you would answer you didn't. The |
@waylan It passes whatever is in the attribute to If use the following,
When using Superfences, such an attribute would cause parsing to fail as well. If |
If I were to fix this, this seems sufficient: -((\{(?P<attrs>[^\}\n]*)\})|
+((\{(?P<attrs>[^\n]*)\})| As it is currently written, the only thing that may come after the attr_list on the opening line is |
@facelessuser the >>> from markdown.extensions.attr_list import get_attrs
>>> get_attrs('data-test="{}"')
[('data-test', '{}')] Like |
@waylan I was comparing the actual processing of the values in Markdown: import markdown
MD = R"""
## test {data-test="{}"}
"""
print(markdown.markdown(MD, extensions=['attr_list'])) <h2>test {data-test="{}"}</h2> In general, we were being consistent with how |
I think as far as SuperFences is concerned, we have a clear starting indicator ( For normal inline patterns, In the worst case, if we find undesirable examples of it being too greedy, we could craft a more explicit rule for ourselves. |
The change has been made in SuperFences. |
This is what gives me pause. While it is an easy fix for fenced code blocks, it introduces an inconsistency with all other attr_lists. And to adjust the others would be a much more involved process. |
Yep, it is definitely more involved. While I think possible, and maybe even doable as a single (much more complex) pattern, it is not nearly as simple. |
FYI, we removed this feature from our documentation. The current implementation is too fragile and will likely lead to many problems when being used by non-technical users. We don't feel comfortable providing support for it at the current stage. |
Well, regardless, the next version of pymdown-extensions will handle braces better in code block attributes. |
@squidfunk thanks for the update. Given the fact that (1) there is no officially supported need for it, (2) the proposed modification would introduce a inconsistency between attr_lists on fenced code blocks compared to all other elements, and (3) Superfences provides a working solution for those who really want it, I'm inclined to not make a change to fenced code blocks at this time. That said, if someone were to submit a PR which consistently altered the attr_list behavior across all elements to allow curly braces, I would be wiling to give it consideration. Therefore, I'm not going to close this but will give it the [someday-maybe] label. |
I believe it satisfies this requirement |
This issue originated from squidfunk/mkdocs-material#6177 where I was exploring the "override copied text" feature.
The following code block breaks Markdown parsing:
It doesn't quite make sense to me as everything is already inside double quotes.
Instead, HTML-encoding the braces (and you can add newlines) solves the problem:
Using HTML-encoding is not intuitive to most users. Maybe this could get changed to Python escapes?
The text was updated successfully, but these errors were encountered: