close
Skip to content

Include tilde (~) in markdown syntax tokens#146417

Merged
jrieken merged 3 commits intomicrosoft:mainfrom
babakks:escape-tildes-in-md
Dec 13, 2022
Merged

Include tilde (~) in markdown syntax tokens#146417
jrieken merged 3 commits intomicrosoft:mainfrom
babakks:escape-tildes-in-md

Conversation

@babakks
Copy link
Copy Markdown
Contributor

@babakks babakks commented Mar 31, 2022

This PR fixes #146219

babakks added 2 commits March 31, 2022 10:24
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>
@geekley
Copy link
Copy Markdown

geekley commented Apr 1, 2022

@babakks Shouldn't : and ; be backslash-escaped as well? To prevent links and HTML character entities from being treated specially? E.g.:

https\://example.com
&amp\;

https://example.com
&amp;

@geekley
Copy link
Copy Markdown

geekley commented Apr 1, 2022

Also, IMO \n would be better escaped by "  \n" (2 spaces then newline) rather than "\n\n" (2 newlines).
The former makes a newline, the latter starts a new paragraph (this distinction doesn't seem to exist in github-flavored markdown, since it treats every newline as a newline; but it does in classical/vscode markdown).

Though I admit this is minor and also debatable.

EDIT: Huh? I noticed there should be now a parameter for that, but I don't see it in 1.65.0. Maybe it didn't make it into the API yet? The commit is from Nov/2020 though...

@babakks
Copy link
Copy Markdown
Contributor Author

babakks commented Apr 2, 2022

@geekley I see your point, but I think we'd better off sticking with the standard markdown definition on escapable characters (here) which does not include ; or : (though, for example, Github supports escaping them).

@geekley
Copy link
Copy Markdown

geekley commented Apr 2, 2022

@babakks
OK, but that definition doesn't include ~ either. In fact ~ is not special in original markdown, so VSCode is using some other "flavor". And I did test all three backslash-escaping chars in VSCode and they do work.

If you want to ensure reliable behavior, you'd have to know which markdown spec VSCode actually uses. I assume it's likely it may be using CommonMark, which is a much more "proper" standard than the original -- and, at least on latest version I saw now, it does include escaping for those characters and many others as you can see here:
https://spec.commonmark.org/0.30/#backslash-escapes

If you still don't want to rely on backslash escaping, there's also the option of preceding them with a comment (I tested that too on all three): <!---->: and <!---->;
In the ~ case, it also did seem to work, but if you want to be extra safe it might be prudent to surround it on both sides (since it can be used on start or end of strikethrough): <!---->~<!---->

But I think the best solution would be confirming whether it's using CommonMark and which version, and then include all escaping of that spec's version, specially because we may be missing some other edge cases.


EDIT: In fact, even in the oldest version listed, the set of escapable chars in CommonMark was the same, it seems:
https://spec.commonmark.org/0.5/#backslash-escapes
So, if VSCode's markdown comes from, for example, a npm dependency on the popular markdown-it (I didn't check if that's the case), then that set of chars is surely the right one to use.

@geekley
Copy link
Copy Markdown

geekley commented Apr 2, 2022

Ah OK, VSCode API does state that it's using CommonMark:
@types/vscode v1.65.0 index.d.ts

    /**
     * Human-readable text that supports formatting via the [markdown syntax](https://commonmark.org).
     *
     * Rendering of {@link ThemeIcon theme icons} via the `$(<name>)`-syntax is supported
     * when the {@linkcode supportThemeIcons} is set to `true`.
     *
     * Rendering of embedded html is supported when {@linkcode supportHtml} is set to `true`.
     */
    export class MarkdownString {

So, the full list of chars should be:

!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~

!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~

@babakks
Copy link
Copy Markdown
Contributor Author

babakks commented Apr 4, 2022

@geekley I think you're right. it makes sense to update the character set.

@jrieken Do you agree?

@jrieken jrieken added this to the January 2023 milestone Dec 13, 2022
@jrieken
Copy link
Copy Markdown
Member

jrieken commented Dec 13, 2022

I generally agree but let's keep this PR focused on the ~ issue

@jrieken jrieken merged commit 45d2254 into microsoft:main Dec 13, 2022
@babakks babakks deleted the escape-tildes-in-md branch December 13, 2022 08:59
@geekley
Copy link
Copy Markdown

geekley commented Dec 14, 2022

I generally agree but let's keep this PR focused on the ~ issue

@jrieken @babakks Is there an open issue/PR for the rest of the CommonMark chars that should be escaped, so that I can subscribe to it?

@github-actions github-actions Bot locked and limited conversation to collaborators Jan 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vscode API bug: MarkdownString appendText does not escape tilde

4 participants