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

Add support for GitHub alert blocks #776

Merged
merged 7 commits into from Mar 14, 2024
Merged

Add support for GitHub alert blocks #776

merged 7 commits into from Mar 14, 2024

Conversation

xoofx
Copy link
Owner

@xoofx xoofx commented Mar 4, 2024

Fixes #774

This extension does not support roundtrip.

@xoofx xoofx requested a review from MihaZupan March 4, 2024 08:27
@xoofx
Copy link
Owner Author

xoofx commented Mar 4, 2024

@MihaZupan I added you as a reviewer. First time in years that I haven't implemented an extension to markdig 😅

@coveralls
Copy link

coveralls commented Mar 4, 2024

Pull Request Test Coverage Report for Build 8276766130

Details

  • 136 of 156 (87.18%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 93.197%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Markdig/Extensions/Alerts/AlertBlockRenderer.cs 37 38 97.37%
src/Markdig/MarkdownExtensions.cs 5 7 71.43%
src/Markdig/Extensions/Alerts/AlertInlineParser.cs 60 77 77.92%
Totals Coverage Status
Change from base Build 8092414745: -0.07%
Covered Lines: 26086
Relevant Lines: 27363

💛 - Coveralls

Copy link
Collaborator

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple notes, but this looks good to me.

It feels a bit odd that we're hardcoding SVGs, but I don't know of a good alternative. Users can customize the logic themselves anyway.

src/Markdig/MarkdownExtensions.cs Outdated Show resolved Hide resolved
src/Markdig/Extensions/Alerts/AlertBlockRenderer.cs Outdated Show resolved Hide resolved
src/Markdig/Extensions/Alerts/AlertInlineParser.cs Outdated Show resolved Hide resolved
src/Markdig/Extensions/Alerts/AlertInlineParser.cs Outdated Show resolved Hide resolved
src/Markdig/Extensions/Alerts/AlertInlineParser.cs Outdated Show resolved Hide resolved
@jorge-moreira
Copy link

jorge-moreira commented Mar 14, 2024

Hi Alexandre,

I was trying your code and I found a situation where the AlertBlock might not be parsed.

For this example, I added a code block inside the alert:

> [!NOTE]
> Highlights information that users should take into account, even when skimming.
> Testing rendering for multiple lines
> ```csharp
> var test = "I can also add code to panels
> ```
> `Inline code testing` 

This is the result on GitHub markdown:

Note

Highlights information that users should take into account, even when skimming.
Testing rendering for multiple lines

var test = "I can also add code to panels

Inline code testing

Although, it will never be rendered, since is not parsed as an AlertBlock.

The same goes to if I want to add a nested quote block on the alert:

> [!NOTE]
> Highlights information that users should take into account, even when skimming.
>  > Nested quoteblock
>  > more info
>  > other

Expected result:

Note

Highlights information that users should take into account, even when skimming.

Nested quoteblock
more info
other

If you need more info regarding this, I'm available to help.

@xoofx
Copy link
Owner Author

xoofx commented Mar 14, 2024

I was trying your code and I found a situation where the AlertBlock might not be parsed.

Thanks, good catch, I have a fix. I mistakenly forgot that inline parser happens after all blocks have been parsed.

@xoofx xoofx merged commit b62a12d into master Mar 14, 2024
2 checks passed
@xoofx
Copy link
Owner Author

xoofx commented Mar 14, 2024

Thanks all for the review!

@xoofx xoofx deleted the alert-blocks branch March 14, 2024 07:18
@jorge-moreira
Copy link

Hi @xoofx

I was trying the new version and it seems that the paragraphs within the AlertBlock are not being rendered.

Markdown example:

> [!NOTE]
> Highlights information that users should take into account, even when skimming.
> 
> Testing rendering for multiple lines
> 
> `Inline code testing`
> 
> Other line
> 
> > Nested quote
> >
> > Final nested quote line
> 
> Final line of alert

Expected:

Note

Highlights information that users should take into account, even when skimming.

Testing rendering for multiple lines

Inline code testing

Other line

Nested quote

Final nested quote line

Final line of alert

HTML rendered result:

<div class="markdown-alert markdown-alert-note">
<p class="markdown-alert-title"><svg viewBox="0 0 16 16" version="1.1" width="16" height="16" aria-hidden="true"><path d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8Zm8-6.5a6.5 6.5 0 1 0 0 13 6.5 6.5 0 0 0 0-13ZM6.5 7.75A.75.75 0 0 1 7.25 7h1a.75.75 0 0 1 .75.75v2.75h.25a.75.75 0 0 1 0 1.5h-2a.75.75 0 0 1 0-1.5h.25v-2h-.25a.75.75 0 0 1-.75-.75ZM8 6a1 1 0 1 1 0-2 1 1 0 0 1 0 2Z"></path></svg>Note</p>
<p>Highlights information that users should take into account, even when skimming.</p>
<p></p>
<p></p>
<p></p>
<blockquote>
<p></p>
<p></p>
</blockquote>
<p></p>
</div>

Do you want me to open an issue with this info?

@xoofx
Copy link
Owner Author

xoofx commented Mar 14, 2024

Do you want me to open an issue with this info?

Yes please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for GitHub Markdown Alerts
5 participants