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

Extract TipFormatters into its own package #1316

Open
4 tasks done
MangelMaxime opened this issue Aug 13, 2024 · 3 comments
Open
4 tasks done

Extract TipFormatters into its own package #1316

MangelMaxime opened this issue Aug 13, 2024 · 3 comments

Comments

@MangelMaxime
Copy link
Contributor

Details

Hello,

I am working on improving F# formatting API documentation and one of the main element is to improve the Tooltip formatting.

This leads to having to duplicate TipFormatter code between FSAC and F# Formatting. Would you be ok if we make it a standalone package?

The portion of the TipFormatter that I would like to export is only the portion responsible for converting a XMLDoc into a formatted text.

It should correspond more or less to this portion of the file:

let inline nl<'T> = Environment.NewLine
let logger = LogProvider.getLoggerByName "TipFormatter"
module private Section =
let inline addSection (name: string) (content: string) =
if name <> "" then
nl + nl + "**" + name + "**" + nl + nl + content
else
nl + nl + content
let fromKeyValueList (name: string) (content: list<KeyValuePair<string, string>>) =
if List.isEmpty content then
""
else
content
|> Seq.map (fun kv ->
let text =
if kv.Value.Contains("\n") then
kv.Value.Split('\n')
|> Seq.map (fun line -> "> " + line.TrimStart())
|> String.concat Environment.NewLine
|> (+) nl // Start the quote block on a new line
else
kv.Value
"* `" + kv.Key + "`" + ": " + text)
|> String.concat nl
|> addSection name
let fromOption (name: string) (content: string option) = if content.IsNone then "" else addSection name content.Value
let fromList (name: string) (content: string seq) =
if Seq.isEmpty content then
""
else
addSection name (content |> String.concat nl)
module private Format =
let tagPattern (tagName: string) =
sprintf
"""(?'void_element'<%s(?'void_attributes'\s+[^\/>]+)?\/>)|(?'non_void_element'<%s(?'non_void_attributes'\s+[^>]+)?>(?'non_void_innerText'(?:(?!<%s>)(?!<\/%s>)[\s\S])*)<\/%s\s*>)"""
tagName
tagName
tagName
tagName
tagName
type TagInfo =
| VoidElement of attributes: Map<string, string>
| NonVoidElement of innerText: string * attributes: Map<string, string>
type FormatterInfo =
{ TagName: string
Formatter: TagInfo -> string option }
let private extractTextFromQuote (quotedText: string) = quotedText.Substring(1, quotedText.Length - 2)
let extractMemberText (text: string) =
let pattern = "(?'member_type'[a-z]{1}:)?(?'member_text'.*)"
let m = Regex.Match(text, pattern, RegexOptions.IgnoreCase)
if m.Groups.["member_text"].Success then
m.Groups.["member_text"].Value
else
text
let private getAttributes (attributes: Group) =
if attributes.Success then
let pattern = """(?'key'\S+)=(?'value''[^']*'|"[^"]*")"""
Regex.Matches(attributes.Value, pattern, RegexOptions.IgnoreCase)
|> Seq.cast<Match>
|> Seq.map (fun m -> m.Groups.["key"].Value, extractTextFromQuote m.Groups.["value"].Value)
|> Map.ofSeq
else
Map.empty
type AttrLookup = Map<string, string> -> Option<string>
let private cref: AttrLookup = Map.tryFind "cref"
let private langword: AttrLookup = Map.tryFind "langword"
let private href: AttrLookup = Map.tryFind "href"
let private lang: AttrLookup = Map.tryFind "lang"
let private name: AttrLookup = Map.tryFind "name"
let rec private applyFormatter (info: FormatterInfo) text =
let pattern = tagPattern info.TagName
match Regex.Match(text, pattern, RegexOptions.IgnoreCase) with
| m when m.Success ->
if m.Groups.["void_element"].Success then
let attributes = getAttributes m.Groups.["void_attributes"]
let replacement = VoidElement attributes |> info.Formatter
match replacement with
| Some replacement ->
text.Replace(m.Groups.["void_element"].Value, replacement)
// Re-apply the formatter, because perhaps there is more
// of the current tag to convert
|> applyFormatter info
| None ->
// The formatter wasn't able to convert the tag
// Return as it is and don't re-apply the formatter
// otherwise it will create an infinity loop
text
else if m.Groups.["non_void_element"].Success then
let innerText = m.Groups.["non_void_innerText"].Value
let attributes = getAttributes m.Groups.["non_void_attributes"]
let replacement = NonVoidElement(innerText, attributes) |> info.Formatter
match replacement with
| Some replacement ->
// Re-apply the formatter, because perhaps there is more
// of the current tag to convert
text.Replace(m.Groups.["non_void_element"].Value, replacement)
|> applyFormatter info
| None ->
// The formatter wasn't able to convert the tag
// Return as it is and don't re-apply the formatter
// otherwise it will create an infinity loop
text
else
// Should not happened but like that we are sure to handle all possible cases
text
| _ -> text
let private codeBlock =
{ TagName = "code"
Formatter =
function
| VoidElement _ -> None
| NonVoidElement(innerText, attributes) ->
let lang =
match lang attributes with
| Some lang -> lang
| None -> "forceNoHighlight"
// We need to trim the end of the text because the
// user write XML comments with a space between the '///'
// and the '<code>' tag. Then it mess up identification of new lines
// at the end of the code snippet.
// Example:
// /// <code>
// /// var x = 1;
// /// </code>
// ^ This space is the one we need to remove
let innerText = innerText.TrimEnd()
// Try to detect how the code snippet is formatted
// so render the markdown code block the best way
// by avoid empty lines at the beginning or the end
let formattedText =
match
innerText.StartsWith("\n", StringComparison.Ordinal), innerText.EndsWith("\n", StringComparison.Ordinal)
with
| true, true -> sprintf "```%s%s```" lang innerText
| true, false -> sprintf "```%s%s\n```" lang innerText
| false, true -> sprintf "```%s\n%s```" lang innerText
| false, false -> sprintf "```%s\n%s\n```" lang innerText
Some formattedText
}
|> applyFormatter
let private example =
{ TagName = "example"
Formatter =
function
| VoidElement _ -> None
| NonVoidElement(innerText, _) ->
let formattedText =
nl
+ nl
// This try to keep a visual consistency and indicate that this
// "Example section" is part of it parent section (summary, remarks, etc.)
+ """Example:"""
+ nl
+ nl
+ innerText
Some formattedText
}
|> applyFormatter
let private codeInline =
{ TagName = "c"
Formatter =
function
| VoidElement _ -> None
| NonVoidElement(innerText, _) -> "`" + innerText + "`" |> Some }
|> applyFormatter
let private link text uri = $"[`%s{text}`](%s{uri})"
let private code text = $"`%s{text}`"
let private anchor =
{ TagName = "a"
Formatter =
function
| VoidElement attributes ->
match href attributes with
| Some href -> Some(link href href)
| None -> None
| NonVoidElement(innerText, attributes) ->
match href attributes with
| Some href -> Some(link innerText href)
| None -> Some(code innerText) }
|> applyFormatter
let private paragraph =
{ TagName = "para"
Formatter =
function
| VoidElement _ -> None
| NonVoidElement(innerText, _) -> nl + innerText + nl |> Some }
|> applyFormatter
let private block =
{ TagName = "block"
Formatter =
function
| VoidElement _ -> None
| NonVoidElement(innerText, _) -> nl + innerText + nl |> Some }
|> applyFormatter
let private see =
let formatFromAttributes (attrs: Map<string, string>) =
match cref attrs with
// crefs can have backticks in them, which mess with formatting.
// for safety we can just double-backtick and markdown is ok with that.
| Some cref -> Some $"``{extractMemberText cref}``"
| None ->
match langword attrs with
| Some langword -> Some(code langword)
| None -> None
{ TagName = "see"
Formatter =
function
| VoidElement attributes -> formatFromAttributes attributes
| NonVoidElement(innerText, attributes) ->
if String.IsNullOrWhiteSpace innerText then
formatFromAttributes attributes
else
match href attributes with
| Some externalUrl -> Some(link innerText externalUrl)
| None -> Some $"`{innerText}`" }
|> applyFormatter
let private xref =
{ TagName = "xref"
Formatter =
function
| VoidElement attributes ->
match href attributes with
| Some href -> Some(link href href)
| None -> None
| NonVoidElement(innerText, attributes) ->
if String.IsNullOrWhiteSpace innerText then
match href attributes with
| Some href -> Some(link innerText href)
| None -> None
else
Some(code innerText) }
|> applyFormatter
let private paramRef =
{ TagName = "paramref"
Formatter =
function
| VoidElement attributes ->
match name attributes with
| Some name -> Some(code name)
| None -> None
| NonVoidElement(innerText, attributes) ->
if String.IsNullOrWhiteSpace innerText then
match name attributes with
| Some name ->
// TODO: Add config to generates command
Some(code name)
| None -> None
else
Some(code innerText)
}
|> applyFormatter
let private typeParamRef =
{ TagName = "typeparamref"
Formatter =
function
| VoidElement attributes ->
match name attributes with
| Some name -> Some(code name)
| None -> None
| NonVoidElement(innerText, attributes) ->
if String.IsNullOrWhiteSpace innerText then
match name attributes with
| Some name ->
// TODO: Add config to generates command
Some(code name)
| None -> None
else
Some(code innerText) }
|> applyFormatter
let private fixPortableClassLibrary (text: string) =
text.Replace(
"~/docs/standard/cross-platform/cross-platform-development-with-the-portable-class-library.md",
"https://docs.microsoft.com/en-gb/dotnet/standard/cross-platform/cross-platform-development-with-the-portable-class-library"
)
/// <summary>Handle Microsoft 'or' formatting blocks</summary>
/// <remarks>
/// <para>We don't use the formatter API here because we are not handling a "real XML element"</para>
/// <para>We don't use regex neither because I am not able to create one covering all the possible case</para>
/// <para>
/// There are 2 types of 'or' blocks:
///
/// - Inlined: [...] -or- [...] -or- [...]
/// - Blocked:
/// [...]
/// -or-
/// [...]
/// -or-
/// [...]
/// </para>
/// <para>
/// This function can convert both styles. If an 'or' block is encounter the whole section will always result in a multiline output
/// </para>
/// <para>
/// If we pass any of the 2 previous example, it will generate the same Markdown string as a result (because they have the same number of 'or' section). The result will be:
/// </para>
/// <para>
/// > [...]
///
/// *or*
///
/// > [...]
///
/// *or*
///
/// > [...]
/// </para>
/// </remarks>
let private handleMicrosoftOrList (text: string) =
let splitResult = text.Split([| "-or-" |], StringSplitOptions.RemoveEmptyEntries)
// If text doesn't contains any `-or-` then we just forward it
if Seq.length splitResult = 1 then
text
else
splitResult
|> Seq.map (fun orText ->
let orText = orText.Trim()
let lastParagraphStartIndex = orText.LastIndexOf("\n")
// We make the assumption that an 'or' section should always be defined on a single line
// From testing against different 'or' block written by Microsoft it seems to be always the case
// By doing this assumption this allow us to correctly handle comments like:
//
// <block>
// Some text goes here
// </block>
// CaseA of the or section
// -or-
// CaseB of the or section
// -or-
// CaseC of the or section
//
// The original comments is for `System.Uri("")`
// By making the assumption that an 'or' section is always single line this allows us to detect the "<block></block>" section
// orText is on a single line, we just add quotation syntax
if lastParagraphStartIndex = -1 then
sprintf "> %s" orText
// orText is on multiple lines
// 1. We first extract the everything until the last line
// 2. We extract on the last line
// 3. We return the start of the section and the end of the section marked using quotation
else
let startText = orText.Substring(0, lastParagraphStartIndex)
let endText = orText.Substring(lastParagraphStartIndex)
sprintf "%s\n> %s" startText endText)
// Force a new `-or-` paragraph between each orSection
// In markdown a new paragraph is define by using 2 empty lines
|> String.concat "\n\n*or*\n\n"
/// <summary>Remove all invalid 'or' block found</summary>
/// <remarks>
/// If an 'or' block is found between 2 elements then we remove it as we can't generate a valid markdown for it
///
/// For example, <td> Some text -or- another text </td> cannot be converted into a multiline string
/// and so we prefer to remove the 'or' block instead of having some weird markdown artifacts
///
/// For now, we only consider text between <td></td> to be invalid
/// We can add more in the future if needed, but I want to keep this as minimal as possible to avoid capturing false positive
/// </remarks>
let private removeInvalidOrBlock (text: string) =
let invalidOrBlockPattern =
"""<td(\s+[^>])*>(?'or_text'(?:(?!<td)[\s\S])*-or-(?:(?!<\/td)[\s\S])*)<\/td(\s+[^>])*>"""
Regex.Matches(text, invalidOrBlockPattern, RegexOptions.Multiline)
|> Seq.cast<Match>
|> Seq.fold
(fun (state: string) (m: Match) ->
let orText = m.Groups.["or_text"]
if orText.Success then
let replacement = orText.Value.Replace("-or-", "or")
state.Replace(orText.Value, replacement)
else
state)
text
let private convertTable =
{ TagName = "table"
Formatter =
function
| VoidElement _ -> None
| NonVoidElement(innerText, _) ->
let rowCount = Regex.Matches(innerText, "<th\s?>").Count
let convertedTable =
innerText
.Replace(nl, "")
.Replace("\n", "")
.Replace("<table>", "")
.Replace("</table>", "")
.Replace("<thead>", "")
.Replace("</thead>", (String.replicate rowCount "| --- "))
.Replace("<tbody>", nl)
.Replace("</tbody>", "")
.Replace("<tr>", "")
.Replace("</tr>", "|" + nl)
.Replace("<th>", "|")
.Replace("</th>", "")
.Replace("<td>", "|")
.Replace("</td>", "")
nl + nl + convertedTable + nl |> Some
}
|> applyFormatter
type private Term = string
type private Definition = string
type private ListStyle =
| Bulleted
| Numbered
| Tablered
/// ItemList allow a permissive representation of an Item.
/// In theory, TermOnly should not exist but we added it so part of the documentation doesn't disappear
/// TODO: Allow direct text support without <description> and <term> tags
type private ItemList =
/// A list where the items are just contains in a <description> element
| DescriptionOnly of string
/// A list where the items are just contains in a <term> element
| TermOnly of string
/// A list where the items are a term followed by a definition (ie in markdown: * <TERM> - <DEFINITION>)
| Definitions of Term * Definition
let private itemListToStringAsMarkdownList (prefix: string) (item: ItemList) =
match item with
| DescriptionOnly description -> prefix + " " + description
| TermOnly term -> prefix + " " + "**" + term + "**"
| Definitions(term, description) -> prefix + " " + "**" + term + "** - " + description
let private list =
let getType (attributes: Map<string, string>) = Map.tryFind "type" attributes
let tryGetInnerTextOnNonVoidElement (text: string) (tagName: string) =
match Regex.Match(text, tagPattern tagName, RegexOptions.IgnoreCase) with
| m when m.Success ->
if m.Groups.["non_void_element"].Success then
Some m.Groups.["non_void_innerText"].Value
else
None
| _ -> None
let tryGetNonVoidElement (text: string) (tagName: string) =
match Regex.Match(text, tagPattern tagName, RegexOptions.IgnoreCase) with
| m when m.Success ->
if m.Groups.["non_void_element"].Success then
Some(m.Groups.["non_void_element"].Value, m.Groups.["non_void_innerText"].Value)
else
None
| _ -> None
let tryGetDescription (text: string) = tryGetInnerTextOnNonVoidElement text "description"
let tryGetTerm (text: string) = tryGetInnerTextOnNonVoidElement text "term"
let rec extractItemList (res: ItemList list) (text: string) =
match Regex.Match(text, tagPattern "item", RegexOptions.IgnoreCase) with
| m when m.Success ->
let newText = text.Substring(m.Value.Length)
if m.Groups.["non_void_element"].Success then
let innerText = m.Groups.["non_void_innerText"].Value
let description = tryGetDescription innerText
let term = tryGetTerm innerText
let currentItem: ItemList option =
match description, term with
| Some description, Some term -> Definitions(term, description) |> Some
| Some description, None -> DescriptionOnly description |> Some
| None, Some term -> TermOnly term |> Some
| None, None -> None
match currentItem with
| Some currentItem -> extractItemList (res @ [ currentItem ]) newText
| None -> extractItemList res newText
else
extractItemList res newText
| _ -> res
let rec extractColumnHeader (res: string list) (text: string) =
match Regex.Match(text, tagPattern "listheader", RegexOptions.IgnoreCase) with
| m when m.Success ->
let newText = text.Substring(m.Value.Length)
if m.Groups.["non_void_element"].Success then
let innerText = m.Groups.["non_void_innerText"].Value
let rec extractAllTerms (res: string list) (text: string) =
match tryGetNonVoidElement text "term" with
| Some(fullString, innerText) ->
let escapedRegex = Regex(Regex.Escape(fullString))
let newText = escapedRegex.Replace(text, "", 1)
extractAllTerms (res @ [ innerText ]) newText
| None -> res
extractColumnHeader (extractAllTerms [] innerText) newText
else
extractColumnHeader res newText
| _ -> res
let rec extractRowsForTable (res: (string list) list) (text: string) =
match Regex.Match(text, tagPattern "item", RegexOptions.IgnoreCase) with
| m when m.Success ->
let newText = text.Substring(m.Value.Length)
if m.Groups.["non_void_element"].Success then
let innerText = m.Groups.["non_void_innerText"].Value
let rec extractAllTerms (res: string list) (text: string) =
match tryGetNonVoidElement text "term" with
| Some(fullString, innerText) ->
let escapedRegex = Regex(Regex.Escape(fullString))
let newText = escapedRegex.Replace(text, "", 1)
extractAllTerms (res @ [ innerText ]) newText
| None -> res
extractRowsForTable (res @ [ extractAllTerms [] innerText ]) newText
else
extractRowsForTable res newText
| _ -> res
{ TagName = "list"
Formatter =
function
| VoidElement _ -> None
| NonVoidElement(innerText, attributes) ->
let listStyle =
match getType attributes with
| Some "bullet" -> Bulleted
| Some "number" -> Numbered
| Some "table" -> Tablered
| Some _
| None -> Bulleted
(match listStyle with
| Bulleted ->
let items = extractItemList [] innerText
items
|> List.map (itemListToStringAsMarkdownList "*")
|> String.concat Environment.NewLine
| Numbered ->
let items = extractItemList [] innerText
items
|> List.map (itemListToStringAsMarkdownList "1.")
|> String.concat Environment.NewLine
| Tablered ->
let columnHeaders = extractColumnHeader [] innerText
let rows = extractRowsForTable [] innerText
let columnHeadersText =
columnHeaders
|> List.mapi (fun index header ->
if index = 0 then
"| " + header
elif index = columnHeaders.Length - 1 then
" | " + header + " |"
else
" | " + header)
|> String.concat ""
let separator =
columnHeaders
|> List.mapi (fun index _ ->
if index = 0 then "| ---"
elif index = columnHeaders.Length - 1 then " | --- |"
else " | ---")
|> String.concat ""
let itemsText =
rows
|> List.map (fun columns ->
columns
|> List.mapi (fun index column ->
if index = 0 then
"| " + column
elif index = columnHeaders.Length - 1 then
" | " + column + " |"
else
" | " + column)
|> String.concat "")
|> String.concat Environment.NewLine
Environment.NewLine
+ columnHeadersText
+ Environment.NewLine
+ separator
+ Environment.NewLine
+ itemsText)
|> Some }
|> applyFormatter
/// <summary>
/// Unescape XML special characters
///
/// For example, this allows to print '>' in the tooltip instead of '&gt;'
/// </summary>
let private unescapeSpecialCharacters (text: string) =
text
.Replace("&lt;", "<")
.Replace("&gt;", ">")
.Replace("&quot;", "\"")
.Replace("&apos;", "'")
.Replace("&amp;", "&")
let applyAll (text: string) =
text
// Remove invalid syntax first
// It's easier to identify invalid patterns when no transformation has been done yet
|> removeInvalidOrBlock
// Start the transformation process
|> paragraph
|> example
|> block
|> codeInline
|> codeBlock
|> see
|> xref
|> paramRef
|> typeParamRef
|> anchor
|> list
|> convertTable
|> fixPortableClassLibrary
|> handleMicrosoftOrList
|> unescapeSpecialCharacters
[<RequireQualifiedAccess>]
type FormatCommentStyle =
| Legacy
| FullEnhanced
| SummaryOnly
| Documentation
// TODO: Improve this parser. Is there any other XmlDoc parser available?
type private XmlDocMember(doc: XmlDocument, indentationSize: int, columnOffset: int) =
/// References used to detect if we should remove meaningless spaces
let tabsOffset = String.replicate (columnOffset + indentationSize) " "
let readContentForTooltip (node: XmlNode) =
match node with
| null -> null
| _ ->
let content =
// Normale the EOL
// This make it easier to work with line splitting
node.InnerXml.Replace("\r\n", "\n") |> Format.applyAll
content.Split('\n')
|> Array.map (fun line ->
if
not (String.IsNullOrWhiteSpace line)
&& line.StartsWith(tabsOffset, StringComparison.Ordinal)
then
line.Substring(columnOffset + indentationSize)
else
line)
|> String.concat Environment.NewLine
let readChildren name (doc: XmlDocument) =
doc.DocumentElement.GetElementsByTagName name
|> Seq.cast<XmlNode>
|> Seq.map (fun node -> Format.extractMemberText node.Attributes.[0].InnerText, node)
|> Seq.toList
let readRemarks (doc: XmlDocument) = doc.DocumentElement.GetElementsByTagName "remarks" |> Seq.cast<XmlNode>
let rawSummary = doc.DocumentElement.ChildNodes.[0]
let rawParameters = readChildren "param" doc
let rawRemarks = readRemarks doc
let rawExceptions = readChildren "exception" doc
let rawTypeParams = readChildren "typeparam" doc
let rawReturns =
doc.DocumentElement.GetElementsByTagName "returns"
|> Seq.cast<XmlNode>
|> Seq.tryHead
let rawExamples =
doc.DocumentElement.GetElementsByTagName "example"
|> Seq.cast<XmlNode>
// We need to filter out the examples node that are children
// of another "main" node
// This is because if the example node is inside a "main" node
// then we render it in place.
// So we don't need to render it independently in the Examples section
|> Seq.filter (fun node ->
[ "summary"; "param"; "returns"; "exception"; "remarks"; "typeparam" ]
|> List.contains node.ParentNode.Name
|> not)
let readNamedContentAsKvPair (key, content) = KeyValuePair(key, readContentForTooltip content)
let summary = readContentForTooltip rawSummary
let parameters = rawParameters |> List.map readNamedContentAsKvPair
let remarks = rawRemarks |> Seq.map readContentForTooltip
let exceptions = rawExceptions |> List.map readNamedContentAsKvPair
let typeParams = rawTypeParams |> List.map readNamedContentAsKvPair
let examples = rawExamples |> Seq.map readContentForTooltip
let returns = rawReturns |> Option.map readContentForTooltip
let seeAlso =
doc.DocumentElement.GetElementsByTagName "seealso"
|> Seq.cast<XmlNode>
|> Seq.map (fun node -> "* `" + Format.extractMemberText node.Attributes.[0].InnerText + "`")
override x.ToString() =
summary
+ nl
+ nl
+ (parameters
|> Seq.map (fun kv -> "`" + kv.Key + "`" + ": " + kv.Value)
|> String.concat nl)
+ (if exceptions.Length = 0 then
""
else
nl
+ nl
+ "Exceptions:"
+ nl
+ (exceptions
|> Seq.map (fun kv -> "\t" + "`" + kv.Key + "`" + ": " + kv.Value)
|> String.concat nl))
member __.ToSummaryOnlyString() =
// If we where unable to process the doc comment, then just output it as it is
// For example, this cover the keywords' tooltips
if String.IsNullOrEmpty summary then
doc.InnerText
else
"**Description**" + nl + nl + summary
member __.HasTruncatedExamples = examples |> Seq.isEmpty |> not
member __.ToFullEnhancedString() =
let content =
summary
+ Section.fromList "Remarks" remarks
+ Section.fromKeyValueList "Type parameters" typeParams
+ Section.fromKeyValueList "Parameters" parameters
+ Section.fromOption "Returns" returns
+ Section.fromKeyValueList "Exceptions" exceptions
+ Section.fromList "See also" seeAlso
// If we where unable to process the doc comment, then just output it as it is
// For example, this cover the keywords' tooltips
if String.IsNullOrEmpty content then
doc.InnerText
else
"**Description**" + nl + nl + content
member __.ToDocumentationString() =
"**Description**"
+ nl
+ nl
+ summary
+ Section.fromList "Remarks" remarks
+ Section.fromKeyValueList "Type parameters" typeParams
+ Section.fromKeyValueList "Parameters" parameters
+ Section.fromOption "Returns" returns
+ Section.fromKeyValueList "Exceptions" exceptions
+ Section.fromList "Examples" examples
+ Section.fromList "See also" seeAlso
member this.FormatComment(formatStyle: FormatCommentStyle) =
match formatStyle with
| FormatCommentStyle.Legacy -> this.ToString()
| FormatCommentStyle.SummaryOnly -> this.ToSummaryOnlyString()
| FormatCommentStyle.FullEnhanced -> this.ToFullEnhancedString()
| FormatCommentStyle.Documentation -> this.ToDocumentationString()

Having a separate library means:

  • Both project can benefit from improvements made to the XMLDoc formatter

  • Add tests to capture future regressions.

    I think there are currently no tests regarding XMLDoc formatting in FSAC (at least I never wrote ones when I worked a lot with that portion of the code)

Extracting that portion of the code should not be too much of big deal because it doesn't depends on FSC or things complex like that. It just want an XMLElement and return a string / class instance.

It could be named something like Ionide.XMLDocFormatter? I worked on that code several times and volunteer to scaffold that project.

Checklist

  • I have looked through existing issues to make sure that this feature has not been requested before
  • I have provided a descriptive title for this issue
  • I am aware that even valid feature requests may be rejected if they do not align with the project's goals
  • I or my company would be willing to contribute this feature
@TheAngryByrd
Copy link
Member

I think this is a good idea. Some other questions/thoughts.

  • Should this be a separate repository?
    • If so, just another repository in the giant chain to update when FCS comes out with a new version.
  • How much do we need to care about backwards compatibility/API stability?
  • I know we have some issues with our tooltips being a bit too tied to VSCode display. Would this library need to know/allow for other formatting outputs? (Not extremely familiar with the TipFormatter code so maybe this is already addressable)

@MangelMaxime
Copy link
Contributor Author

  • Should this be a separate repository?

    • If so, just another repository in the giant chain to update when FCS comes out with a new version.

I would prefer it to be a separate repository because it is easier to maintain (at least for me). I already have all the infrastructure needed to handle automatic release based on Git history etc.

I only want to expose from Ionide.XMLDocFormatter something like XMLDoc -> string.

This way it is the job of FSAC and F# Formatting to resolve the XMLDoc location based on the project cracker / FCS API.

So I don't believe this library will be impacted by FCS release. It will just need Regex and XML API, so I don't think it will have anything dependencies beside NetStandard.

  • How much do we need to care about backwards compatibility/API stability?

Only a portion of TipFormatter will be extracted and it should be backwards compatible. The main compatibility stuff to keep are the different styles of ToolTips we want to supports:

[<RequireQualifiedAccess>]
type FormatCommentStyle =
  | Legacy // Full text version of the tooltip
  | FullEnhanced // This is the default used by FSAC at the moment (markdown output with stripped exemples)
  | SummaryOnly // Markdown with summary content only
  | Documentation // Used by InfoPanel and probably F# Formatting (markdown with full content)

We could debate if Legacy is still needed, but every time I asked to remove it I was asked to not too 😅.

  • I know we have some issues with our tooltips being a bit too tied to VSCode display. Would this library need to know/allow for other formatting outputs? (Not extremely familiar with the TipFormatter code so maybe this is already addressable)

This library will not need to know/allow for other formatting outputs. It will just need for FSAC/F# Formatting to tell which version of the Tooltip they want (see above).

Currently the tooltip is composed of:

  1. Signature
  2. XMLDoc comment
  3. Open the documentation link
  4. FullName info
  5. Assembly info

The portion I want to extract is XMLDoc comment. While the part too tied to VSCode right now is Open the documentation link because it generates a VSCode command link which is not standard across LSP Servers.

This is part of FSAC LSPServer:

match TipFormatter.tryFormatTipEnhanced tooltipResult.ToolTipText formatCommentStyle with
| TipFormatter.TipFormatterResult.Success tooltipInfo ->
let response =
{ Contents =
U3.C3
[|
// Display the signature as a code block
tooltipResult.Signature
|> TipFormatter.prepareSignature
|> (fun content -> U2.C2 { Language = "fsharp"; Value = content })
U2.C1 tooltipInfo.DocComment
match tooltipResult.SymbolInfo with
| TryGetToolTipEnhancedResult.Keyword _ -> ()
| TryGetToolTipEnhancedResult.Symbol symbolInfo ->
TipFormatter.renderShowDocumentationLink
tooltipInfo.HasTruncatedExamples
symbolInfo.XmlDocSig
symbolInfo.Assembly
|> U2.C1
// Display each footer line as a separate line
yield! tooltipResult.Footer |> TipFormatter.prepareFooterLines |> Array.map U2.C1 |]

@baronfel
Copy link
Contributor

I'm generally in favor of this - my main comments are aligned with Jimmy. We need to keep an ability to define custom renderings for e.g. VSCode-specific clients vs NeoVim clients, for example.

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

No branches or pull requests

3 participants