fsprojects / FSharp.Formatting

F# tools for generating documentation (Markdown processor and F# code formatter)
https://fsprojects.github.io/FSharp.Formatting/
Other
462 stars 155 forks source link

Add pre tag for comment summary. #843

Closed nojaf closed 7 months ago

nojaf commented 9 months ago

Fixes https://github.com/fsprojects/FSharp.Formatting/issues/736. @kMutagene would you mind giving this a spin on your local codebase?

dsyme commented 9 months ago

Hmmm is using pre really right? Any chance of a before/after visual on the generated docs? If I look at the docs for, say, Diffsharp, it would look wrong to use pre-formatted text I'm sure:

image

It looks like the original request was in the context of summary text written as markdown. In theory, formatted text should be written using an explicity <summary> and <code></code> and <para></para> sections etc.

I agree with your original analysis that if people are writing docs as markdown, they should use the original flag provided for that. Perhaps you could provide a flag for the behaviour implemented here? But it shouldn't be the default

dsyme commented 9 months ago

Also, the Summary will be formatted HTML at this point, and include <b>...</b> and other elements, converted from the XML doc via https://github.com/fsprojects/FSharp.Formatting/blob/main/src/FSharp.Formatting.ApiDocs/GenerateModel.fs#L1827

Arguably what should be happening is the F# compiler supports a --markdowndocs option, which interprets /// text as a limited subset of markdown and emits an XML doc that corresponds to the intent of the markdown, or else emits a single special <markdown>...</markdown> section with the raw markdown (not totally sure which is better - the former would mean consuming F# tooling doesn't need to be updated - the latter would require updates to all consuming tooling)

nojaf commented 9 months ago

Hmm, I still think this makes sense, to be honest. Consider:

/// To use this from C#
///
///     [lang=csharp]
///     var a = new MyClass()
///
/// To use this from F#
///
///     let a = FsLib.MyClass()
///
type MyClass() =
    member x.Nothing = 0

This now (on main) generates:

  <div class="fsdocs-xmldoc">
    <div>
      <p class="fsdocs-summary">

 To use this from C#

     [lang=csharp]
     var a = new MyClass()

 To use this from F#

     let a = FsLib.MyClass()

      </p>
    </div>
  </div>

Wrapping this in a summary tag does not affect the output:

/// <summary>
/// To use this from C#
///
///     [lang=csharp]
///     var a = new MyClass()
///
/// To use this from F#
///
///     let a = FsLib.MyClass()
/// </summary>
type MyClass() =
    member x.Nothing = 0

same HTML:

  <div class="fsdocs-xmldoc">
    <div>
      <p class="fsdocs-summary">

 To use this from C#

     [lang=csharp]
     var a = new MyClass()

 To use this from F#

     let a = FsLib.MyClass()

      </p>
    </div>
  </div>

The diffsharp example you mention is a little different: https://github.com/DiffSharp/DiffSharp/blob/29af24ad69a96aace8a49420996861b2a3ca0fa9/src/DiffSharp.Core/DiffSharp.fs#L977-L979

The summary/remark gives:

/// <summary>
/// To use this from C#
///
///     [lang=csharp]
///     var a = new MyClass()
/// </summary>
/// <code>
/// To use this from F#
///
///     let a = FsLib.MyClass()
/// </code>
/// <remarks>Yes, ok new paragraph</remarks>
type MyClass() =
    member x.Nothing = 0
  <div class="fsdocs-xmldoc">
    <div>
      <p class="fsdocs-summary">

 To use this from C#

     [lang=csharp]
     var a = new MyClass()

      </p>
    </div>
    <p class="fsdocs-remarks">
      Yes, ok new paragraph
    </p>
  </div>

Which still doesn't take any newlines into account for the individual paragraph element.

nojaf commented 7 months ago

This was indeed not the way to go.