Backslash in hover popup is not shown

[jfcherng]

Describe the bug

As the title.

Original response payload:

:: <<< LSP-intelephense 22: {'range': {'start': {'line': 4, 'character': 6}, 'end': {'line': 4, 'character': 9}}, 'contents': {'kind': 'markdown', 'value': '__A\\B\\C\\Foo__\n\n```php\n<?php\nclass Foo { }\n```'}}

To Reproduce

Steps to reproduce the behavior:

1. Have the following codes with intelephense.

<?php

namespace A\B\C;

class Foo {}

2. Hover on Foo

Expected behavior

The backslashes show as-is.

Screenshots

Environment (please complete the following information):

• OS: Win10 x64 22H2
• Sublime Text version: 4147
• LSP version: 4070-1.21.0
• Language servers used: intelephense

[rchl]

Minimal reproduce:

mdpopups.show_popup(view, mdpopups.md2html(view, mdpopups.format_frontmatter({'markdown_extensions': [{'pymdownx.escapeall': {}}]}) + '__A\\B\\C\\Foo__'))

I'll have to throw it at @facelessuser :)

[facelessuser]

Please reference the documentation for the plugin: https://facelessuser.github.io/pymdown-extensions/extensions/escapeall/.

If you have any questions or confusion about its behavior after reading, please feel free to ask. I feel like the documentation is pretty clear, but I'm happy to update if there is anything unclear.

[rchl]

Right, makes sense.

The PR that added pymdownx.escapeall doesn't reference any issues but I believe it was meant to fix sublimelsp/LSP-css#16 and the LSP-css case seems to be working now without pymdownx.escapeall (probably fixed in the language server itself) so I would lean towards just removing that extension.

[rchl]

Though the hardbreak option of pymdownx.escapeall was later enabled in #1591 to fix a different case. And that case still seems to need this workaround.

So it appears that we might still need pymdownx.escapeall for hardbreak case while we don't want it for processing anything else. Which doesn't seem to be possible right now.

[facelessuser]

🤷🏻 If you are using unescaped HTML characters, you run the risk of things breaking. In this case, HTML entities or escapes for HTML characters should most likely be used, but EscapeAll is a great alternative. Something like <color> (referencing the case above, should not be expected to just work.

The original Markdown spec does not include < into the default list of characters that can be escaped: https://daringfireball.net/projects/markdown/syntax#backslash. I assume the idea was that that was what HTML entities were for.

Anyways, some or even all CommonMark parsers may support escaped \< now, not sure what they are currently spec'd to do in this regard, but historically, that was not the case.

EscapeAll has a very specific goal in what it is doing. I wanted \ to always escape so I didn't have to think about whether a plugin allowed a certain character to be escaped or not, especially when you are using a number of custom extensions. It isn't trying to match CommonMark as it existed well before CommonMark started making its appearance.

[rchl]

On the other hand, pyright doesn't seem to be formatting the content in a way that needs this workaround anymore:

{
  "label": "AMQPConnectionWorkflow",
  "data": {
    "workspacePath": "/usr/local/workspace/sublime-packages/LSP",
    "symbolLabel": "AMQPConnectionWorkflow",
    "filePath": "/usr/local/workspace/sublime-packages/LSP/plugin/completion.py",
    "position": {
      "character": 5,
      "line": 19
    }
  },
  "kind": 7,
  "documentation": {
    "value": "```python\nclass AMQPConnectionWorkflow()\n```\n---\nImplements Pika's default workflow for performing multiple TCP/\\[SSL\\]/AMQP\nconnection attempts with timeouts and retries until one succeeds or all\nattempts fail.\n\nThe workflow:  \n    while not success and retries remain:\n        1. For each given config (pika.connection.Parameters object):  \n            A. Perform DNS resolution of the config's host.\nB. Attempt to establish TCP/\\[SSL\\]/AMQP for each resolved address  \n               until one succeeds, in which case we're done.\n        2. If all configs failed but retries remain, resume from beginning  \n           after the given retry pause. NOTE: failure of DNS resolution\nis equivalent to one cycle and will be retried after the pause\nif retries remain.",
    "kind": "markdown"
  },
  "sortText": "05.0000.AMQPConnectionWorkflow"
}

So it seems like we can drop it after all.

[rchl]

To summarize, both of the cases that needed pymdownx.escapeall don't seem to be an issue anymore (likely fixed in the relevant servers). We might find this issue popping up again in other cases if we drop the pymdownx.escapeall extension but it feels like it's the way to go for now anyway.

[rchl]

@facelessuser as I've pointed out in here and in #2163 (comment), removing the escapeall extension completely might give us some trouble in the future. We wouldn't want to use it for escaping all escape sequences but we would likely still want to use the hardbreak functionality.

In case we'd need that (I guess we'll run without escapeall until we find some issues), would you be open to adding an option to escapeall to disable escaping of normal escape sequences so that we could only enable hardbreak option? Or maybe create new extension with only the hardbreak functionality?

[facelessuser]

It is certainly possible, you'd have to create an issue upstream. I haven't been updating the Sublime version of Python Markdown or Pymdown Extensions (which contains the EscapeAll extension) as I've been waiting for Package Control to finally update to allow Python 3.8 dependencies.

I'd really, really like to see Package Control update things for Python 3.8 before updating all the dependencies as I don't want to backport all the current changes back to Python 3.3, but maybe we could backport this specific change if I'm going to be waiting another 2 years for Package Control 😩.

Create an issue on the Pymdown Extensions repo and I can look into it and figure out how we might move forward.