NuGet / NuGetGallery

NuGet Gallery is a package repository that powers https://www.nuget.org. Use this repo for reporting NuGet.org issues.
https://www.nuget.org/
Apache License 2.0
1.55k stars 643 forks source link

Readme: Add support for NOTE, TIP, WARNING, IMPORTANT, and CAUTION notations #10125

Open kzu opened 3 months ago

kzu commented 3 months ago

Related Problem

No response

The Elevator Pitch

Highlighting important parts of a package documentation is crucial to creating a more engaging experience.

In GitHub, the following readme markup:


> [!NOTE]
> This is a note

> [!TIP]
> This is a tip

> [!WARNING]
> This is a warning

> [!IMPORTANT]
> This is important

> [!CAUTION]
> This is a caution

Renders as follows image

This is how the gallery renders that today: https://www.nuget.org/packages/Observable/0.2.0#readme-body-tab

image

Source for the examples: https://github.com/kzu/Observable

Additional Context and Details

Bringing nuget.org more aligned with GitHub markdown support is a good thing for OSS authors since it allows us to keep a unified readme in both places, lowering maintenance costs and avoiding duplication just to satisfy nuget.org restrictions on formatting.

JonDouglas commented 3 months ago

Looks like this is possible w/ markdig today:

https://github.com/xoofx/markdig/blob/master/src/Markdig.Tests/Specs/AlertBlockSpecs.md

erdembayar commented 2 months ago

See https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1605 for Note and Tip ones.

image

kzu commented 2 months ago

I could give it a shot at sending a PR if it would help this ship sooner than later 🙏 . Just let me know and I can figure it out.

Thanks!

erdembayar commented 2 months ago

Sure, you're welcome to create a PR. Please start with just one instead of all. If the first one is accepted, then we can proceed with the others.

Happy coding! 🧑‍💻

erdembayar commented 2 months ago

Please note that sometimes we ask for a spec review before starting implementation if it is going to change current behavior (e.g. how customers perceive), or if it will introduce any breaking changes or security concerns (for example, rendering requiring a new package, URL, or network call). Something like this: https://github.com/NuGet/Home/pull/13673

kzu commented 2 months ago

Oh boy is this going to be "interesting":

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  <PropertyGroup>
    <UseNuGetBuildExtensions>true</UseNuGetBuildExtensions>
    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>

I thought I was never going to see that legacy format ever again :(

erdembayar commented 2 months ago

Oh boy is this going to be "interesting":

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  <PropertyGroup>
    <UseNuGetBuildExtensions>true</UseNuGetBuildExtensions>
    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>

I thought I was never going to see that legacy format ever again :(

It shouldn't be hard to migrate, there're tools for doing that, recently some community members migrated some for us (https://github.com/NuGet/NuGetGallery/pull/10000)

kzu commented 2 months ago

And, dev branch build fails due to https://github.com/NuGet/NuGetGallery/pull/10166. 😿

Added this to the relevant .csproj.user for now:

    <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
kzu commented 2 months ago

Quick Q @erdembayar: does the nuget.org use markdig rendering? (starting from ConvertsMarkdownToHtml)

kzu commented 2 months ago

Ok, with an update to Markdig and a one-liner .UseAlertBlocks(), the following inline data passes for the mentioned test:

[InlineData(
    """
    > [!NOTE]
    > This is a note
    """,
    """
    <div class="markdown-alert markdown-alert-note alert alert-primary" dir="auto">
    <p class="markdown-alert-title" dir="auto"><svg class="octicon octicon-info mr-2" 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 dir="auto">This is a note</p>
    </div>
    """,
    false, true)]

That matches (almost exactly, except for alert alert-primary in the outer div) the html in github.com.

All that remains is where to put the styles.

zivkan commented 3 weeks ago

IMO this should be validated with the NuGet Client team to ensure that it'll be possible to support in Visual Studio. Markdown features that only work on nuget.org and not Visual Studio won't be good for the ecosystem.

cc @jgonz120