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

Formatting documentation strings in hover popups. #962

Open
deathaxe opened this issue Jan 17, 2021 · 13 comments
Open

Formatting documentation strings in hover popups. #962

deathaxe opened this issue Jan 17, 2021 · 13 comments
Assignees
Labels
enhancement New feature or request hover

Comments

@deathaxe
Copy link

The JSON language server supports markdown formatted docstrings, which enables us to write nice looking popups such as:

grafik

Can LemMinX do something like that as well?

It appears all <xs:documentation> contents is streamed as unformatted plain text with all kinds of whitespace and newlines etc. removed. So even plain text formatted docstring as the one from the following example result in poor user experience at the moment.

Result
grafik

Source

 <xs:annotation>
  <xs:documentation>
   See http://www.w3.org/XML/1998/namespace.html and
   http://www.w3.org/TR/REC-xml for information about this namespace.

    This schema document describes the XML namespace, in a form
    suitable for import by other schema documents.  

    Note that local names in this namespace are intended to be defined
    only by the World Wide Web Consortium or its subgroups.  The
    following names are currently defined in this namespace and should
    not be used with conflicting semantics by any Working Group,
    specification, or document instance:

    base (as an attribute name): denotes an attribute whose value
         provides a URI to be used as the base for interpreting any
         relative URIs in the scope of the element on which it
         appears; its value is inherited.  This name is reserved
         by virtue of its definition in the XML Base specification.

    lang (as an attribute name): denotes an attribute whose value
         is a language code for the natural language of the content of
         any element; its value is inherited.  This name is reserved
         by virtue of its definition in the XML specification.
  
    space (as an attribute name): denotes an attribute whose
         value is a keyword indicating what whitespace processing
         discipline is intended for the content of the element; its
         value is inherited.  This name is reserved by virtue of its
         definition in the XML specification.

    Father (in any context at all): denotes Jon Bosak, the chair of 
         the original XML Working Group.  This name is reserved by 
         the following decision of the W3C XML Plenary and 
         XML Coordination groups:

             In appreciation for his vision, leadership and dedication
             the W3C XML Plenary on this 10th day of February, 2000
             reserves for Jon Bosak in perpetuity the XML name
             xml:Father
  </xs:documentation>
 </xs:annotation>

What the client receives is:

:: --> LemMinX textDocument/hover(583): {'textDocument': {'uri': 'file:///C:/Apps/Sublime%20Text/Data/Packages/Markdown%20Extras/Snippets/Link.sublime-snippet'}, 'position': {'line': 2, 'character': 5}}
:: <<< LemMinX 583: {'contents': {'value': 'See http://www.w3.org/XML/1998/namespace.html and http://www.w3.org/TR/REC-xml for information about this namespace. This schema document describes the XML namespace, in a form suitable for import by other schema documents. Note that local names in this namespace are intended to be defined only by the World Wide Web Consortium or its subgroups. The following names are currently defined in this namespace and should not be used with conflicting semantics by any Working Group, specification, or document instance: base (as an attribute name): denotes an attribute whose value provides a URI to be used as the base for interpreting any relative URIs in the scope of the element on which it appears; its value is inherited. This name is reserved by virtue of its definition in the XML Base specification. lang (as an attribute name): denotes an attribute whose value is a language code for the natural language of the content of any element; its value is inherited. This name is reserved by virtue of its definition in the XML specification. space (as an attribute name): denotes an attribute whose value is a keyword indicating what whitespace processing discipline is intended for the content of the element; its value is inherited. This name is reserved by virtue of its definition in the XML specification. Father (in any context at all): denotes Jon Bosak, the chair of the original XML Working Group. This name is reserved by the following decision of the W3C XML Plenary and XML Coordination groups: In appreciation for his vision, leadership and dedication the W3C XML Plenary on this 10th day of February, 2000 reserves for Jon Bosak in perpetuity the XML name xml:Father\r\n\r\nSource: [sublime-snippet.xsd](file:/C:/Apps/Sublime%20Text/Data/Packages/LSP-lemminx/catalogs/sublime/sublime-snippet.xsd)', 'kind': 'markdown'}, 'range': {'start': {'line': 2, 'character': 3}, 'end': {'line': 2, 'character': 13}}}

Note: I injected the content into an XSD called sublime-snippet.xsd which I am working on in order to provide intelligence features to write Sublime Text / TextMate resources.

@deathaxe
Copy link
Author

deathaxe commented Jan 18, 2021

Hmm.

With a bit of luck and playing around I managed to create something like:

grafik

But the required XML code looks a bit uncommon. Some tokens need to be escaped even twice (e.g.: &amp;lt;).

<xs:documentation>
	```xml&lt;br&gt;&amp;lt;key&amp;gt;...&lt;/key&amp;gt;&lt;br&gt;```&lt;br&gt;

	&lt;p&gt;The Property List key with the main tmPreferences keys.&lt;/p&gt;
	&lt;p&gt;&lt;u&gt;Valid values are:&lt;/u&gt;&lt;/p&gt;
	&lt;ul&gt;
	&lt;li&gt;name&lt;/li&gt;
	&lt;li&gt;scope&lt;/li&gt;
	&lt;li&gt;settings&lt;/li&gt;
	&lt;li&gt;uuid&lt;/li&gt;
	&lt;/ul&gt;
</xs:documentation>

Things get a bit better by wrapping the content into CDATA.

<xs:documentation>
    <![CDATA[
    ```xml<br>&lt;key&gt;...&lt;/key&gt;<br>```

    <p>The Property List key with the main tmPreferences keys.</p>
    <p>Valid values are:</p>
    <ul>
    <li>name</li>
    <li>scope</li>
    <li>settings</li>
    <li>uuid</li>
    </ul>
    ]]>
</xs:documentation>

I haven't found any other XSD file doing so, which makes me think it is not intended to be needed.

Haven't investigated in detail what's going on, but it seems

  1. the xs:documentation content is parsed as normal XML
  2. only the pure text part is interpreted as html and converted to markdown

In short. I am not familiar with XML standard with regards to handling such doc strings but it looks like there is one conversion step too much taking place.

What I'd expect to produce the desired result is:

<xs:documentation>
	```xml
	<key>...</key>
	```

	The Property List key with the main tmPreferences keys.

	Valid values are:

	- name
	- scope
	- settings
	- uuid
</xs:documentation>

or maybe tags which are to be sent as is escaped with entities.

<xs:documentation>
	```xml
	&lt;key>...&lt;/key>
	```

	The Property List key with the main tmPreferences keys.

	Valid values are:

	- name
	- scope
	- settings
	- uuid
</xs:documentation>

@angelozerr
Copy link
Contributor

I see that you did some experimentation, nice!

The XSD documentation doesn't provide a very robust specification for the format to use so it's difficult to support all usecases. The basic idea is that this content is normalized and when you must encode XML (I know it's little awful), but if you see xs:documentation for maven for instance you will see that http://maven.apache.org/xsd/maven-4.0.0.xsd

@deathaxe
Copy link
Author

I see. The maven code basically matches my first approach. I had a look on older files which contained the ascii documentation or even unescaped html tags directly. Indeed hard/impossible to handle them all.

It appears the normalized content is then converted from html to markdown?

Placing escaped enough <br> tags seems to work best.

<xs:documentation>
  ```xml                                                    &lt;br>
  &amp;lt;key>...&amp;lt;/key>                              &lt;br>
  ```                                                       &lt;br>
                                                            &lt;br>
  The Property List key with the main tmPreferences keys.   &lt;br>
                                                            &lt;br>
  Valid values are:                                         &lt;br>
                                                            &lt;br>
  - name                                                    &lt;br>
  - scope                                                   &lt;br>
  - settings                                                &lt;br>
  - uuid                                                    &lt;br>
</xs:documentation>

The result seems the same:

:: <<< LemMinX 47: {'range': {'start': {'line': 3, 'character': 2}, 'end': {'line': 3, 'character': 5}}, 'contents': {'value': '```xml\r\n<key>...</key>\r\n```\r\n\r\n\r\nThe Property List key with the main tmPreferences keys.\r\n\r\nValid values are:\r\n\r\n *  name\r\n *  scope\r\n *  settings\r\n *  uuid\r\n\r\nSource: [tmPreferences.xsd](file:/C:/Apps/Sublime%20Text/Data/Packages/LSP-lemminx/schemas/sublime/tmPreferences.xsd)', 'kind': 'markdown'}}

@angelozerr
Copy link
Contributor

It appears the normalized content is then converted from html to markdown?

It's exactly that! We did this choice because the main xs:documentation of xsd files (ex : maven) worked with this strategy.

@deathaxe
Copy link
Author

Well, I guess I got the main part. There is just one little thing I am wondering about, which I found in maven-4.0.0.xsd

It may be not well designed in the xsd but may also be something related with html to markdown conversion.

Maybe let's have a look at the following snippet.

   <xs:attribute name="child.project.url.inherit.append.path" type="xs:string" use="optional">
      <xs:annotation>
        <xs:documentation source="version">4.0.0+</xs:documentation>
        <xs:documentation source="description">
            
            When children inherit from project&apos;s url, append path or not? Note: While the type
            of this field is &lt;code&gt;String&lt;/code&gt; for technical reasons, the semantic type is actually
            &lt;code&gt;Boolean&lt;/code&gt;
            &lt;br&gt;&lt;b&gt;Default value is&lt;/b&gt;: &lt;code&gt;true&lt;/code&gt;
            &lt;br&gt;&lt;b&gt;Since&lt;/b&gt;: Maven 3.6.1
            
          </xs:documentation>
      </xs:annotation>
    </xs:attribute>

The normalized content is plain HTML.

            When children inherit from project&apos;s url, append path or not? Note: While the type
            of this field is <code>String</code> for technical reasons, the semantic type is actually
            <code>Boolean</code>
            <br><b>Default value is</b>: <code>true</code>
            <br><b>Since</b>: Maven 3.6.1        

I guess it is expected for the last two lines to be rendered as separate lines.

grafik

But what actually happens is them being rendered inline because Markdown would need two <br> to make them render at the next line.

grafik

My impression is the documentation being intended to be passed as plain HTML rather than being converted to Markdown?

@angelozerr
Copy link
Contributor

But what actually happens is them being rendered inline because Markdown would need two
to make them render at the next line.

Indeed it's a bug

To fix this problem we should set

options.hardwraps = false;

here

which generates 2 spaces and a line break.

@fbricon do you remember why you did that?

@deathaxe
Copy link
Author

Basically Markdown files may contain any kind of HTML tags. In other words: A valid HTML file is also a valid Markdown file.

So if the documentation content is considered valid HTML, trying to convert it to Markdown is probably not needed, is it?

@fbricon
Copy link
Contributor

fbricon commented Jan 20, 2021

@fbricon do you remember why you did that?

No, I don't, it's mainly inherited from eclipse.jdt.ls.
if changing options.hardwraps provides better results, then do that

@angelozerr
Copy link
Contributor

So if the documentation content is considered valid HTML, trying to convert it to Markdown is probably not needed, is it?

I'm sorry I don't understand? Markdown and HTML don't use the same syntax, no?

No, I don't, it's mainly inherited from eclipse.jdt.ls.

Ok thanks for your feedback.

if changing options.hardwraps provides better results, then do that

I tested quickly with Eclipse IDE, and it breaks the Default in a new line:

image

@deathaxe
Copy link
Author

I'm sorry I don't understand? Markdown and HTML don't use the same syntax, no?

Any HTML tag placed into a Markdown file is to be rendered as is per specification.

Therefore writing # Heading or <h1>Heading</h1> in a *.md file results in same rendering. While the former one is just plain markdown which intends to be human readable as is, the latter one is valid as well.

A markdown renderer should therefore ignore html tags if it converts the document to html or interpret those tags if it wants to display the content on screen.

# Heading results in:

Heading

<h1>Heading</h1> results in:

Heading

@datho7561 datho7561 self-assigned this Jan 21, 2021
@datho7561
Copy link
Contributor

Looks like VS Code does not support rendering HTML tags in their documentation Markdown: microsoft/vscode#40607.

When I removed the code to convert the HTML tags to Markdown equivalents, no documentation appeared at all in VS Code.

I'm thinking one way forward is to render the mixed Markdown/HTML content to just HTML, then convert it back into just Markdown.

datho7561 added a commit to datho7561/lemminx that referenced this issue Feb 10, 2021
Treat `xs:documentation` and `xs:appinfo` as mixed Markdown and HTML,
with the HTML tags escaped using entities.
Unescape the entities, then render the Markdown portions into HTML
using the commonmark-java library.
Finally, use the existing code to convert this HTML into Markdown
that doesn't contain any HTML tags.

Closes eclipse#962

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/lemminx that referenced this issue Feb 10, 2021
Treat `xs:documentation` and `xs:appinfo` as mixed Markdown and HTML,
with the HTML tags escaped using entities.
Unescape the entities, then render the Markdown portions into HTML
using the commonmark-java library.
Finally, use the existing code to convert this HTML into Markdown
that doesn't contain any HTML tags.

Closes eclipse#962

Signed-off-by: David Thompson <[email protected]>
@datho7561 datho7561 added this to the 0.17.0 milestone Apr 13, 2021
datho7561 added a commit to datho7561/lemminx that referenced this issue Apr 14, 2021
Treat `xs:documentation` and `xs:appinfo` as mixed Markdown and HTML,
with the HTML tags escaped using entities.
Unescape the entities, then render the Markdown portions into HTML
using the commonmark-java library.
Finally, use the existing code to convert this HTML into Markdown
that doesn't contain any HTML tags.

Closes eclipse#962

Signed-off-by: David Thompson <[email protected]>
@datho7561 datho7561 modified the milestones: 0.16.1, 0.17.0 May 12, 2021
@datho7561
Copy link
Contributor

At this point, we don't support using Markdown for the XSD documentation. The supported way to provide marked up documentation is to use escaped html tags. I am removing this from the 0.17.0 milestone.

I believe that there are ways that we could address some of the problems mentioned in this issue that would be worth looking into in the future, such as escaping Markdown-like content in the documentation so that the client doesn't try to display it as Markdown.

@datho7561 datho7561 removed this from the 0.17.0 milestone Jun 16, 2021
@Timmmm
Copy link

Timmmm commented Aug 3, 2022

Any HTML tag placed into a Markdown file is to be rendered as is per specification.

Almost no Markdown renderers do that for obvious reasons, including VSCode.

This all seems to be a big mess and I didn't follow all of the above conversation but I can confirm that the CDATA technique mentioned above works fine in VSCode and it's not too ugly:

      <![CDATA[
      <pre>x++ += 4</pre>

      <p>The Property List key with the main tmPreferences keys.</p>
      <p>Valid values are:</p>
      <ul>
      <li>name</li>
      <li>scope</li>
      <li>settings</li>
      <li>uuid</li>
      </ul>
      Also you can just use
      <br><br>
      If you want separate paragraphs.
      ]]>

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hover
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants