-
Notifications
You must be signed in to change notification settings - Fork 34.3k
Support markdown styling in task descriptions. #121338
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
Support markdown styling in task descriptions. #121338
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall change looks good. Added a few notes.
Please make sure to add some tests too
@@ -235,7 +235,7 @@ ExtensionsRegistry.registerExtensionPoint({ | |||
}, | |||
description: { | |||
type: 'string', | |||
description: localize('walkthroughs.tasks.description', "Description of task. Use markdown-style links fo commands or external links: [Title](command:myext.command), [Title](command:toSide:myext.command), or [Title](https://aka.ms)") | |||
description: localize('walkthroughs.tasks.description', "Description of task. Supports ``preformatted``, __italic__, and **bold** text. Use markdown-style links for commands for external links: [Title](command:myext.command), [Title](command:toSide:myext.command), or [Title](https://aka.ms). Links on their own line will be rendered as buttons.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would pre-formatted text use a single or double backtick? To match markdown, I think it should probably be a single one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't match markdown already, and this is more consistant with the existing... also it seems like markdown is happy with two backtick, even though one also works:
Markdown: one asterisk, two asterisk, one underscore, two underscore, one backtick
, two backtick
Us: *one asterisk*, two asterisk, _one underscore_, two underscore, `one backtick`, two backtick
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok that's probably ok then and reduces the chance of accidentally creating a code block. I was confused because markdown does support **
and __
(but, as you noted, with different meanings than our formatter)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I prefer our approach TBH, I always end up making weird italic markdown sections when trying to type out mathematic expressions in md-enabled inputs, for instance.
@@ -214,6 +219,8 @@ function formatTagType(char: string): FormatType { | |||
return FormatType.Action; | |||
case ']': | |||
return FormatType.ActionClose; | |||
case '`': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a backtick happens in a string that does not set supportPreformattedSegments
, what happens? How does the parser handler FormatType.Invalid
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parser treats invalid tokens as normal chars and appends them to the text content of the node, they'd be invalid tokens before this change too.
Closes #120050