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

LSP: docs/hover handler doesn't handle HTML escape sequences #22757

Closed
Kasama opened this issue Mar 23, 2023 · 5 comments
Closed

LSP: docs/hover handler doesn't handle HTML escape sequences #22757

Kasama opened this issue Mar 23, 2023 · 5 comments
Labels
bug issues reporting wrong behavior lsp
Milestone

Comments

@Kasama
Copy link
Contributor

Kasama commented Mar 23, 2023

Problem

Related to this issue

When the Language server responds a markdown payload for a textDocument/hover request containing html escape sequences (in this example, many   sequences), the stylize_markdown function doesn't handle it, causing it to be written as-is to the floating window's buffer.

Example response:

{
  "contents": {
    "kind": "markdown",
    "value": " # # # #   g i t l a b \\ - c i \n \n T h e   n a m e   o f   o n e   o r   m o r e   j o b s   t o   i n h e r i t   c o n f i g u r a t i o n   f r o m \\ . \n \n S o u r c e :   [ c i . j s o n ] ( h t t p s : / / g i t l a b . c o m / g i t l a b - o r g / g i t l a b / - / r a w / m a s t e r / a p p / a s s e t s / j a v a s c r i p t s / e d i t o r / s c h e m a / c i . j s o n ) "
  },
  "range": {
    "end": {
      "character": 9,
      "line": 2377
    },
    "start": {
      "character": 2,
      "line": 2377
    }
  }
}

Example hover on neovim
image

It would be great to have neovim decode those html sequences, similar to what this luarocks package does.


A workaround for me is to use a wrapper function around vim.lsp.handlers.hover to remove those sequences manually

local function hover_wrapper(err, request, ctx, config)
  local bufnr, winnr = vim.lsp.handlers.hover(err, request, ctx, config)
  if (bufnr == nil or winnr == nil) then
    return
  end
  local contents = vim.api.nvim_buf_get_lines(bufnr, 0, -1, false)
  contents = vim.tbl_map(
    function(line)
      return string.gsub(line, " ", "")
    end,
    contents
  )
  vim.api.nvim_buf_set_option(bufnr, 'modifiable', true)
  vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, contents)
  vim.api.nvim_buf_set_option(bufnr, 'modifiable', false)
  vim.api.nvim_win_set_height(winnr, #contents)

  return bufnr, winnr
end

vim.lsp.handlers["textDocument/hover"] = hover_wrapper

Adding this to init.lua fixes the appearance of the hover for this language server, but there are others that probably have similar issues.

Steps to reproduce using "nvim -u minimal_init.lua"

  1. create a new temporary directory cd $(mktemp -d) and create an init.lua file with the following contents
local pattern = 'yaml'
local cmd = { 'yaml-language-server', '--stdio' }
-- Add files/folders here that indicate the root of a project
local root_markers = { '.gitlab-ci.yaml' }
-- Change to table with settings if required
local settings = {
  yaml = {
    schemas = {
      ["https://gitlab.com/gitlab-org/gitlab/-/raw/master/app/assets/javascripts/editor/schema/ci.json"] = ".gitlab-ci.y*l",
    },
    hover = true,
  }
}

vim.api.nvim_create_autocmd('FileType', {
  pattern = pattern,
  callback = function(args)
    local match = vim.fs.find(root_markers, { path = args.file, upward = true })[1]
    local root_dir = match and vim.fn.fnamemodify(match, ':p:h') or nil
    vim.lsp.start({
      name = 'yamlls',
      cmd = cmd,
      root_dir = root_dir,
      settings = settings
    })
  end
})

vim.keymap.set('n', '<space>', vim.lsp.buf.hover, {})
  1. create a .gitlab-ci.yaml file with the following contents
stages:
  - hello
  1. open it with neovim nvim --noplugin -u init.lua .gitlab-ci.yaml
  2. wait a couple of seconds for yaml-language-server to be launched and press <space> with the cursor over the word stages

Expected behavior

I expected neovim to be able to parse html entities in a markdown hover response to properly display them in the floating buffer.

Neovim version (nvim -v)

NVIM v0.8.3 Build type: Release

Language server name/version

yaml-language-server 1.12.0 https://github.com/redhat-developer/yaml-language-server/releases/tag/1.12.0

Operating system/version

Linux Kasama 6.2.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 18 Mar 2023 01:06:36 +0000 x86_64 GNU/Linux

Log file

No response

@Kasama Kasama added bug issues reporting wrong behavior lsp labels Mar 23, 2023
@Kasama
Copy link
Contributor Author

Kasama commented Mar 23, 2023

I also volunteer to write a PR for this, but there are a couple of questions I'd like to have answered first.

  • Is this something we want to do?
  • Do we want to include the luarocks package that already handles these entities?
    • if not, should we include all entities that it handles, or just a subset of common ones? i.e. &gt;, &lt;, &quot;

@clason
Copy link
Member

clason commented Mar 23, 2023

Is that even valid Markdown? Neovim's policy is to strictly implement the specification and not cater to every off-spec quirk that language servers may have "because VS Code handles it fine".

(And the near-term goal is to replace this function with one that uses the tree-sitter Markdown parser.)

@justinmk
Copy link
Member

the near-term goal is to replace this function with one that uses the tree-sitter Markdown parser

Yes 👍

Do we want to include the luarocks package that already handles these entities?

No (see above), but a small code change to workaround the issue is fine as a short-term solution, imo.

@justinmk justinmk added this to the backlog milestone Mar 23, 2023
@justinmk justinmk changed the title stylize markdown doesn't handle HTML escape sequences LSP: docs/hover handler doesn't handle HTML escape sequences Mar 23, 2023
@Kasama
Copy link
Contributor Author

Kasama commented Mar 23, 2023

Is that even valid Markdown? Neovim's policy is to strictly implement the specification and not cater to every off-spec quirk that language servers may have "because VS Code handles it fine".

Indeed, I don't think this is valid in "normal" markdown even though it works on github-flavored markdown and some others.

I'll try to continue the discussion on the language server side to see why the returned value is like this to begin with.

(And the near-term goal is to replace this function with one that uses the tree-sitter Markdown parser.)

nice.

No (see above), but a small code change to workaround the issue is fine as a short-term solution, imo.

Got it. Would something like this be a good, small, workaround for now?

-- in function M.stylized_markdown in runtime/lua/vim/lsp/util.lua
stripped = vim.tbl_map(
  function(line)
    line = string.gsub(line, '&gt;', '>')
    line = string.gsub(line, '&lt;', '<')
    line = string.gsub(line, '&quot;', '"')
    line = string.gsub(line, '&apos;', "'")
    line = string.gsub(line, '&ensp;', ' ')
    line = string.gsub(line, '&emsp;', ' ')
    line = string.gsub(line, '&amp;', '&')
    return line
  end,
  stripped
)

@clason
Copy link
Member

clason commented Mar 23, 2023

Yes, that looks fine.

Kasama added a commit to Kasama/neovim that referenced this issue Mar 23, 2023
Implements neovim#22757

During handling of textDocument/hover, if the language server returns a
payload containing markdown with some html escape sequences, the neovim
handler will convert common ones to characters to display in the
floating window's buffer.

Signed-off-by: Kasama <robertoaall@gmail.com>
Kasama added a commit to Kasama/neovim that referenced this issue Mar 23, 2023
Implements neovim#22757

During handling of textDocument/hover, if the language server returns a payload containing markdown with some html escape sequences, the neovim handler will convert common ones to characters to display in the floating window's buffer.

Signed-off-by: Kasama <robertoaall@gmail.com>
Kasama added a commit to Kasama/neovim that referenced this issue Mar 23, 2023
Implements neovim#22757

During handling of textDocument/hover, if the language server returns a payload
containing markdown with some html escape sequences, the neovim handler will
convert common ones to characters to display in the floating window's buffer.

Signed-off-by: Kasama <robertoaall@gmail.com>
Kasama added a commit to Kasama/neovim that referenced this issue Mar 23, 2023
Implements neovim#22757

During handling of textDocument/hover, if the language server returns a payload
containing markdown with some html escape sequences, the neovim handler will
convert common ones to characters to display in the floating window's buffer.

Signed-off-by: Kasama <robertoaall@gmail.com>
Kasama added a commit to Kasama/neovim that referenced this issue Mar 23, 2023
Implements neovim#22757

During handling of textDocument/hover, if the language server returns a payload
containing markdown with some html escape sequences, the neovim handler will
convert common ones to characters to display in the floating window's buffer.

Signed-off-by: Kasama <robertoaall@gmail.com>
Kasama added a commit to Kasama/neovim that referenced this issue Mar 25, 2023
Implements neovim#22757

During handling of textDocument/hover, if the language server returns a payload
containing markdown with some html escape sequences, the neovim handler will
convert common ones to characters to display in the floating window's buffer.

Signed-off-by: Kasama <robertoaall@gmail.com>
Kasama added a commit to Kasama/neovim that referenced this issue Mar 25, 2023
Implements neovim#22757

During handling of textDocument/hover, if the language server returns a payload
containing markdown with some html escape sequences, the neovim handler will
convert common ones to characters to display in the floating window's buffer.

Signed-off-by: Kasama <robertoaall@gmail.com>
justinmk pushed a commit that referenced this issue Mar 25, 2023
Problem:
LSP docs hover (textDocument/hover) doesn't handle HTML escape seqs in markdown.

Solution:
Convert common HTML escape seqs to a nicer form, to display in the float.
closees #22757

Signed-off-by: Kasama <robertoaall@gmail.com>
craigmac pushed a commit to craigmac/neovim that referenced this issue Apr 4, 2023
Problem:
LSP docs hover (textDocument/hover) doesn't handle HTML escape seqs in markdown.

Solution:
Convert common HTML escape seqs to a nicer form, to display in the float.
closees neovim#22757

Signed-off-by: Kasama <robertoaall@gmail.com>
folke pushed a commit to folke/neovim that referenced this issue May 22, 2023
Problem:
LSP docs hover (textDocument/hover) doesn't handle HTML escape seqs in markdown.

Solution:
Convert common HTML escape seqs to a nicer form, to display in the float.
closees neovim#22757

Signed-off-by: Kasama <robertoaall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues reporting wrong behavior lsp
Projects
None yet
Development

No branches or pull requests

3 participants