facelessuser / sublime-markdown-popups

Markdown popup dependency for Sublime
https://facelessuser.github.io/sublime-markdown-popups/
Other
112 stars 13 forks source link

Markdown parsing from LSP/Solargraph #139

Closed dportalesr closed 1 year ago

dportalesr commented 1 year ago

I have this code:

# @return [Array<String, Integer>] The resulting array.
def payment
  ...
end

This is how the LSP popup looks on hover in a non-fresh install of ST4 (b4150) using solargraph

deleteme-listing_search_resolver rb-20230802-021828

And this is how it looks in a fresh install of ST3 (b3211)

deleteme-abp-backend-20230802-021849

Notice the escaped "<" for the return value. Also, it seems that ST4 is not recognizing the markdown list in that same section, whereas ST3 seems to be doing it (not sure if the line break is added because of the content).

I also tried it with Zed editor which also uses solargraph as LSP provider. This is what I expect to see:

deleteme-abp-backend-20230802-023009 The additional spacing/line breaks between the sections are a nice addition but I don't really expect them in ST.

Let me know if this could be an issue from mdpopups issue or the LSP package or whatever. Maybe this could be solved by https://github.com/facelessuser/sublime-markdown-popups/issues/103 ?

dportalesr commented 1 year ago

From LSP log in ST:

[03:13:10.343] <<< ruby (3) (duration: 18ms): {'contents': {'kind': 'markdown', 'value': 'Payments::PaymentAffordability#payment\n\n`=^ Array<String, Integer>`\n\nReturns:\n* [Array\\<String, Integer\\>] The resulting array.\n\nVisibility: public'}}

mdpopup debug log:

mdpopups: =====HTML OUTPUT=====
mdpopups: 
<p>
 Payments::PaymentAffordability#payment
</p>
<p>
 <code class="highlight">
  <span style="color: #b8c1c5;">
   =^ Array&lt;String, Integer&gt;
  </span>
 </code>
</p>
<p>
 Returns:
* [Array\&lt;String, Integer&gt;] The resulting array.
</p>
<p>
 Visibility: public
</p>
<div class="actions">
 <a href='subl:lsp_run_text_command_helper {"view_id":24,"args":{"point":3882},"command":"lsp_symbol_definition"}'>
  Definition
 </a>
 <a class="icon" href='subl:lsp_run_text_command_helper {"view_id":24,"args":{"point":3882,"side_by_side":true},"command":"lsp_symbol_definition"}'>
  ◨
 </a>
 |
 <a href='subl:lsp_run_text_command_helper {"view_id":24,"args":{"point":3882},"command":"lsp_symbol_references"}'>
  References
 </a>
 |
 <a href='subl:lsp_run_text_command_helper {"view_id":24,"args":{"point":3882},"command":"lsp_symbol_rename"}'>
  Rename
 </a>
</div>
<div class="lsp_popup--spacer">
</div>
facelessuser commented 1 year ago

I would take this up with the relevant LSP repo. MdPopups currently uses Python Markdown. This is an old-school Markdown parser and is not a CommonMark parser, nor will it ever be. CommonMark added a number of assumptions to its rules which creates a difference in what you observe. I can explain the differences in this case.

In old-school Markdown parsers (at least the ones used as a reference for Python Markdown), when a paragraph is started, all content under that paragraph is treated like a paragraph. This is a single paragraph, not a paragraph and a list.

Returns:
* [Array\<String, Integer\>] The resulting array.

CommonMark adds some new rules to try and be "smarter" or guess the intent of a user, so it assumes if a line in a paragraph starts with * followed by a space that the user must have meant for it to be a list, so it treats it as a list. Python Markdown will not do this. You must separate the list from the paragraph.

Returns:

* [Array\<String, Integer\>] The resulting array.

Python Markdown is not alone in this behavior as can be seen here. Obviously many of the newer CommonMark parsers will treat it as a list, and there are a few others non-CommonMark parsers that do this despite others not doing it, this being one of the reasons CommonMark was created, to unify the Markdown experience.

So, then people ask why we don't just use a CommonMark parser. Well, partly because dependencies in Sublime Text still run on Python 3.3, and I do not want to backport a CommonMark parser to Python 3.3. Once a new, official Package Control is released that can support Python dependencies in Python 3.8, we could consider adding an additional parser that could be selected. We would still keep Python Markdown around for backwards compatibility and because, despite it's perceived faults, it is extremely flexible for custom things. Unfortunately, Python 3.8 is almost end of its life as well. It would be nice if Sublime Text supported 3.8 dependencies before it starts to require significant backporting for dependencies to 3.8.

dportalesr commented 1 year ago

Thank you very much for the comprehensive explanation.

So the markdown parsing is one issue won't be resolved anytime soon, it seems. What do you think about the additional/different escaping of the HTML characters (which somehow it ends showing just the first one)? Is there something that can be done there?

facelessuser commented 1 year ago

The original Markdown only stated that critical characters were escaped, such as > which are used as blockquotes. < was not part of the Markdown syntax, so it required no escaping. To have HTML brackets generally not parsed as actual HTML tags, it was expected that the user would use HTML entities.

Can MdPopups recognize backslash-escaped angle brackets? Sure, if the right extensions are enabled.

Screenshot 2023-08-02 at 3 09 42 PM

You need to take this up with the LSP folks.

dportalesr commented 1 year ago

Gotcha!