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

Rework documentation parser #446

Merged

Conversation

MangelMaxime
Copy link
Contributor

@MangelMaxime MangelMaxime commented Jul 28, 2019

⚠️ Do not merge yet, this is till a WIP ⚠️

This PR is related to ionide/ionide-vscode-fsharp#1012

Its goal is to rework the documentation parser.

Today, we have more or less 3 parsers:

  1. Inside FsAutoComplete.LspHelpers.Markdown which takes a raw string an apply some regex against it
  2. The same as 1. but copied inside Ionide Util.fs
  3. The TipFormatter which use a combination of regex and XML parsing.

I proposed to @Krzysztof-Cieslak the possibility to only have one implementation and he was ok with that.

ionide/ionide-vscode-fsharp#1012 (comment)

Yes, markdown module need to go away, I think we use it only in Info Panel, but we should be able to apply same transformations on FSAC side


Demo of the current state

demo_gif

or_ionide_demo


One parser to rule them all

So I am removing the parsers 1 and 2 and keeps parser 3.

Mutualize as much code as possible

All the following elements use the same abstraction:

  • code
  • c
  • a
  • para
  • block
  • see
  • xref
  • paramref
  • typeparamref
  • table

This abstraction support both "void and non-void" version of the elements and use the same regex to detect them.

Example:

  • <paramref name="c">c</paramref>
  • <paramref name="c" />

Implementation

We can them in the Formatter function, we decide how to retrieve the data depending on the version of the element.

Non standard features

Support lang inside <code>

ℹ️ I am "the creator" of this feature

You can add a lang attributes on a <code> block. This will alows us to provide coloration in the tooltip.

Support for -or- blocks.

ℹ️ Microsoft is using theses blocks

If the -or- block is on a single line Microsoft doesn't reformat it making it hard to read the different cases.

My implementation will always try to optimise the display if possible (for example, inside a table cell it will leave it as it is).

[...]  -or-  [...]  -or-  [...]

or

[...]
-or-
[...]
-or-
[...]

generates the same output:

>    [...]

*or*

>    [...]

*or*

>    [...]

Preview

or_ionide_demo

Table support

ℹ️ Microsoft is using theses blocks

This is already supported in current version of FSAC.

xref block supports

ℹ️ Microsoft is using theses blocks instead of paramref or typeparamref

Fix invalid URLs

ℹ️ Microsoft is using theses blocks

Replace ~/docs/standard/cross-platform/cross-platform-development-with-the-portable-class-library.md by https://docs.microsoft.com/en-gb/dotnet/standard/cross-platform/cross-platform-development-with-the-portable-class-library

This is needed because of some links used inside of netstandard.xml

Questions

On twitter some people complained that the tooltip could be too verbose, so my question should we make the tooltip less verbose?

If yes, what info should we remove?

Here are the info we can choose from:

        "**Description**" + nl + nl
        + summary
        + Section.fromList "" remarks
        + Section.fromMap "Type parameters" typeParams
        + Section.fromMap "Parameters" parameters
        + Section.fromOption "Returns" returns
        + Section.fromMap "Exceptions" exceptions
        + Section.fromList "Examples" examples
        + Section.fromList "See also" seeAlso

TODO

  • Check that everything is still working after the or block addition
    • Fix multilines code block, its seems I am removing their empty lines and significant EOL in it
  • Add missing bullet list support
    • When used to generates info panel, add bullet type support (this is not possible to do it in raw markdown so we need html to do it)
  • Use the parser to generate the info panel documentation [related to https://github.com/WIP: Rework documentation parser ionide-vscode-fsharp#1259)
  • Add configuration support to the parser in order to produce different outputs
    • When used to generates info panel, add command supports
  • Decide what to show in the tooltip

@MangelMaxime
Copy link
Contributor Author

cc @Krzysztof-Cieslak @baronfel

Hello, I need your help to interpret the list tag documentation.

How should we represent the listheader thing? What the term thing and how to represent it?

Do you have any example usage of the list tag. I search for <list in netstandard.xml but it's not used in it...

@baronfel
Copy link
Contributor

I found a more detailed example from an old blog post (not microsoft-affilated).

After reading the docs you linked and this post, I've come to the following sort of data-model for the list structures available to Xml Documentation:

type Term = string
type Definition = string

type ListStyle = Bulleted | Numbered

type ItemList =
/// a list where the items are just single-strings (ie a markdown list item that looks like: * Some text here)
| Simple of elements: string list * style : ListStyle
/// a list where the items are a term followed by a definition (ie in markdown: * <TERM> - <DEFINITION>)
| Definitions of elements: (Term * Definition) list * style: ListStyle

type ColumnHeader = string

// the length of the rows is equal the the length of the headers list
type ItemTable = Table of headers: ColumnHeader list * rows: (Term list) list

/// the overall xmldoc structure can be parsed into this
type XmlListStructure = 
| List of ItemList
| Table of ItemTable

That is, there are two kinds of 'list item' (single terms and full-definitions), two kinds of list (bulleted and numbered) and then a way to represent tables.

I think all of these could be translated into markdown equivalents fairly mechanically:

  • simple lists would translate to either: * <element> or 1. <element> depending on style
  • definitions lists would translate to either * **<term>** - <definition> or 1. **<term> - <definition> depending on style
  • tables would translate to markdown tables in a one-to-one fashion.

This is all assuming we get full-markdown in the tooltips/info panel.

@MangelMaxime
Copy link
Contributor Author

@baronfel Thank you for your analysis.

I will try to implement the list parser now that I have examples and better comprehension of the block :)

@MangelMaxime MangelMaxime force-pushed the feature/doc_comment_formatter branch from 301e5d2 to 548d488 Compare November 8, 2019 15:47
@MangelMaxime MangelMaxime force-pushed the feature/doc_comment_formatter branch from 548d488 to 7e4570a Compare November 12, 2019 11:02
@MangelMaxime
Copy link
Contributor Author

List support is slowly being added:

Simple bulleted list
Capture d’écran 2019-11-12 à 16 05 18

Simple numbered list
Capture d’écran 2019-11-12 à 16 05 34

Term & definition bulleted list
Capture d’écran 2019-11-12 à 16 05 49

Mix (simple | term & definition) bulleted list
Capture d’écran 2019-11-12 à 16 06 09

@MangelMaxime
Copy link
Contributor Author

Here is the table support

Capture d’écran 2019-11-12 à 17 25 55

@MangelMaxime
Copy link
Contributor Author

Hello @Krzysztof-Cieslak @baronfel @7sharp9

I now have all the syntax covered by the new parser. And would like to tick the todo:

Decide what to show in the tooltip

Should we put everything in the tooltip or only the summary content?

@7sharp9
Copy link
Contributor

7sharp9 commented Nov 18, 2019

I think just type info and summary, the other stuff should go in the doc viewer.

@MangelMaxime
Copy link
Contributor Author

To get the new parser out, I am postponing the commands generation. So now, we only need to decide what to include in the tooltip.

Adding commands for all the tags like cref, see, etc. will not be as easy as initially planned that's also why I am postponing it.

@Krzysztof-Cieslak
Copy link
Member

Decide what to show in the tooltip

Maybe have single option? Something like full vs minimal? With Full we'd show everything, with minimal only summary?

@MangelMaxime
Copy link
Contributor Author

Maybe have single option?

I am not sure to understand, do you want to add a setting so the user can configure it from his editor?

@Krzysztof-Cieslak
Copy link
Member

I think there should be setting tooltipMode with possible values simple or full
For simple display just type info and summary, for full display everything.
Default... I think full would be good default in spirit of rich features that Ionide provides out of the box

@baronfel
Copy link
Contributor

baronfel commented Dec 5, 2019

I'd agree with @Krzysztof-Cieslak. Defaulting to more info is how ionide works, and if a user prefers less information they can set one flag in their user profile settings to get that behavior across all Ionide instances.

@7sharp9
Copy link
Contributor

7sharp9 commented Dec 5, 2019 via email

@MangelMaxime
Copy link
Contributor Author

Hello guys,

sorry for the long silence, I was taking a lot of vacations.

Thank you for the input I will update the PR to include the settings.

@MangelMaxime
Copy link
Contributor Author

MangelMaxime commented Mar 9, 2020

Ok, I added the option to control the tooltip mode.

demo_tooltip_mode

@MangelMaxime MangelMaxime changed the title WIP: Rework documentation parser Rework documentation parser Mar 9, 2020
@MangelMaxime
Copy link
Contributor Author

@baronfel @Krzysztof-Cieslak

This finally is ready for review !!! 🎉 🎉

You should use Ionide from this PR ionide/ionide-vscode-fsharp#1259 in order to test it.

@Krzysztof-Cieslak
Copy link
Member

Well, let's do this. Awesome work Maxime, thanks a lot!

@Krzysztof-Cieslak Krzysztof-Cieslak merged commit 2d8b0ec into ionide:master Mar 10, 2020
@MangelMaxime MangelMaxime deleted the feature/doc_comment_formatter branch August 17, 2022 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants