Android-FileBrowser-FilePicker icon indicating copy to clipboard operation
Android-FileBrowser-FilePicker copied to clipboard

Admonitions title's escaping quotes

Open facelessuser opened this issue 4 years ago • 12 comments

So, I've been thinking about this recently and curious if you'd be open to a tweak regarding admonition titles.

Admonition title content can be parsed by markdown down, but the current syntax doesn't really allow for quotes inside.

I was thinking, that maybe we could allow quotes to work similarly to code backticks:

!!! class ""Title with "quoted" content""
    body

So, this would allow for instance for users to now use syntax with quotes.

!!! class ""Title with :emoji:{: style="fill: red"}, blah, blah""
    body

Curious about your thoughts on this. Generally, this should be backwards compatible. We'd probably strip white space from lead and tail of titles, but I imagine this wouldn't affect anyone in a noticeable way, but is mainly to address cases like this "" "quoted title" "".

I'd have no problem implementing it, but I wanted to run it by you before I attempt this.

facelessuser avatar Aug 23 '20 18:08 facelessuser

Interesting request. Personally the only time I use titles is for the seealsotype as the class doesn't have the space and the title does (!!! seealso "See Also") Therefore the requested change isn't something I will ever need. That said, I suppose it makes sense. After all, the ReST docs specifically state that "[t]he title may be anything the author desires." That is definitely not true here. Perhaps more importantly, as the ReST directive does not wrap the title in quotes, there is no restriction on quotes there.

However, I'm not crazy about the doubling of double quotes. I wonder if perhaps using matching single/double quotes (like Python) would be sufficient. In other words, wrap the title in whichever type of quote (single/double) is not in the content:

!!! class 'Single quoted title with "double quoted" content'
    body

or

!!! class "Double quoted title with 'single quoted' content"
    body

That shouldn't negatively effect your attr_list, which, in fact, already supports the same quoting method. Actually, I didn't remember we didn't already support this for admonition titles and had to check before responding.

Another option might be to use backslash escaping, although that might break the attr_list depending on how it is implemented.

waylan avatar Aug 23 '20 22:08 waylan

The single/double thing is relatively easy to do too. Replace "(.*?)" with ("|')(.*?)(\1). Of course, you'd need to adjust the group number to fit in the larger regex (or maybe used a named group for readability). But that should at least address the simple cases.

waylan avatar Aug 23 '20 22:08 waylan

Hmm, I think using either ' or " is probably a decent compromise. I guess there are some scenarios that are a bit more awkward, but not impossible to workaround:

!!! class "We can't use "both""

While allowing multiple quote delimiters maybe aren't as aesthetically pleasing, they are definitely more flexible:

!!! class ""Insert an image ![small image](path/img.png "this is 'quoted'")""

With that said, I'll take what I can get 🙂. I doubt I'll often run up against such a wall, I was simply thinking about flexibility.

facelessuser avatar Aug 23 '20 23:08 facelessuser

I think I'd prefer trying to implement backlash escaping before doubling the quotes:

!!! class "Insert an image ![small image](path/img.png \"this is 'quoted'\")"

Of course, for that to work, we couldn't rely on the existing escaping feature of Markdown. We're need to handle it separately in the extension by replacing \" with " in the title before any additional Markdown parsing is done.

waylan avatar Aug 24 '20 02:08 waylan

I'll look into this and see. Parsing the escapes shouldn't be too difficult.

facelessuser avatar Aug 24 '20 02:08 facelessuser

@facelessuser I realize your time has been taken up with other things lately so I thought I'd give you a heads-up. I expect to release version 3.3 once #1026 is resolved and that is only waiting on the assumption that Python 3.9 will be released in the next 5 days. If you want to get this in version 3.3 and can realistically expect to get the work done in a reasonable amount of time, let me know. Otherwise, I'll move forward without it.

waylan avatar Oct 01 '20 19:10 waylan

This is a "nice to have". As I am currently consumed with some other things currently, this may have to wait a bit.

facelessuser avatar Oct 01 '20 20:10 facelessuser

@waylan Cross posting an issue from material-for-mkdocs here: https://github.com/squidfunk/mkdocs-material/issues/2700

Personally the only time I use titles is for the seealsotype as the class doesn't have the space and the title does (!!! seealso "See Also")

Would it be possible to special case case !!! seealso to use "See Also" as its title by default? This seems much more readable than the current default of "Seealso".

johnthagen avatar May 26 '21 12:05 johnthagen

Would it be possible to special case case !!! seealso to use "See Also" as its title by default? This seems much more readable than the current default of "Seealso".

Probably not. seealso is a Material theme specific class. We add no intelligence for splitting a class name into separate words. If we had a special case for seealso, others will want special handling for othercustomclasses etc.

facelessuser avatar May 26 '21 14:05 facelessuser

Note that seealso was only added to Material for MkDocs to allow for an easy migration from the readthedocs theme, which probably kept it for compatibility with its Sphinx deriviative. For this reason, this modifier was only kept for legacy purposes. It doesn't make sense to keep it, due to its incorrect rendering.

I went ahead and deprecated it: https://squidfunk.github.io/mkdocs-material/reference/admonitions/#note

squidfunk avatar May 26 '21 15:05 squidfunk

@squidfunk, thanks for pointing that out. I think that is a useful point.

facelessuser avatar May 26 '21 15:05 facelessuser

@facelessuser @squidfunk Thanks for the discussion and update. We will remove our usages of !!! seealso.

johnthagen avatar May 26 '21 15:05 johnthagen

@waylan with the recent creation of the pymdownx.blocks extensions, I have no desire to update title handling in Admonitions. If you'd like to close this, I am okay with that.

facelessuser avatar Sep 02 '23 18:09 facelessuser

I agree. If a user wants more advanced functionality, blocks is the way to go.

waylan avatar Sep 05 '23 13:09 waylan