replace multiple space into single space on markdown text in hover

[msivasubramaniaan]

Signed-off-by: msivasubramaniaan msivasub@redhat.com

What does this PR do?

This PR shows the description without escape sequence char even description has indentation on hover

What issues does this PR fix or reference?

redhat-developer/vscode-yaml#886

Is it tested? How?

Yes. Added UT

[coveralls]

Coverage: 83.248% (+0.005%) from 83.243% when pulling d0f43fc on 886-yaml-underscore-being-escaped-in-description-attribute-when-hovering-over-an-attribute into ba7452e on main.

[fabiodouek]

Hi, thanks a lot for looking into this issue.
I had a quick look into this PR. It seems that the adopted solution is to replace multiple spaces by a single space, which in fact is a workaround to the issue.
However, in my view, spaces should be preserved. In my specific case, in the json schema, the description attribute has python code snippets and json snippets. If the spaces are not preserved, the code/json will be unreadable.
Could we preserve the spaces and address the backslash issue?

Thanks,
Fabio.

[msivasubramaniaan]

Hi, thanks a lot for looking into this issue. I had a quick look into this PR. It seems that the adopted solution is to replace multiple spaces by a single space, which in fact is a workaround to the issue. However, in my view, spaces should be preserved. In my specific case, in the json schema, the description attribute has python code snippets and json snippets. If the spaces are not preserved, the code/json will be unreadable. Could we preserve the spaces and address the backslash issue?

Thanks, Fabio.

Thanks for your response. I will try to preserve the space from string

[fabiodouek]

Hi, thanks a lot for looking into this issue. I had a quick look into this PR. It seems that the adopted solution is to replace multiple spaces by a single space, which in fact is a workaround to the issue. However, in my view, spaces should be preserved. In my specific case, in the json schema, the description attribute has python code snippets and json snippets. If the spaces are not preserved, the code/json will be unreadable. Could we preserve the spaces and address the backslash issue?
Thanks, Fabio.

Thanks for your response. I will try to preserve the space from string

Thanks a lot. Please let me know how it goes, Hopefully the spaces can be preserved, otherwise it would have a big impact for our users.

[msivasubramaniaan]

Hi, thanks a lot for looking into this issue. I had a quick look into this PR. It seems that the adopted solution is to replace multiple spaces by a single space, which in fact is a workaround to the issue. However, in my view, spaces should be preserved. In my specific case, in the json schema, the description attribute has python code snippets and json snippets. If the spaces are not preserved, the code/json will be unreadable. Could we preserve the spaces and address the backslash issue?
Thanks, Fabio.

Thanks for your response. I will try to preserve the space from string

Thanks a lot. Please let me know how it goes, Hopefully the spaces can be preserved, otherwise it would have a big impact for our users.

Hello @fabiodouek,

As I am preserving the space from string. Please refer the attachment

[fabiodouek]

msivasubramaniaan

Hi @msivasubramaniaan , that's great, thanks a lot for that.
No pressure, but do you know roughly when this fix will be released?

Thanks,
Fabio.

[msivasubramaniaan]

Hello @fabiodouek

You can expect the release 1st week of March. The pre-release by end of next week.

[rgrunber]

I don't think this is the right approach. The language server is behaving exactly per the spec. The problem is with the client not being able to handle the response.

The language server returns a { contents: MarkupContent, range: Range } which is fine. The VS Code client expects a { contents: Array<MarkdownString | MarkedString>, range: Range } .

https://github.com/microsoft/vscode-languageserver-node/blob/release/client/7.0.0/client/src/common/protocolConverter.ts#L361 is probably what gets called to convert our hover LS response into a VS Code hover so I think we just need to figure out what's going wrong there.

FWIW, I see that vscode-java properly deals with the hover and doesn't unescape the underscores.

[msivasubramaniaan]

Added pre launch check

[msivasubramaniaan]

@rgrunber
Tabs and whitespace have a special meaning in Markdown. So converted spaces into non braking space as a workaround here.

[rgrunber]

I think this is pretty reasonable, so feel free to merge when ready. If in the future we end up having to do more replacement (beyond just the indentation string), it might be good to create a separate helper method.

[polyzen]

Seems this broke presentation in Neovim:

 # # # #   g i t l a b \ - c i   | |   g i t l a b \ - c i 
 
   
 
 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 ) 

[rgrunber]

@polyzen , I'm guessing Neovim doesn't handle Markdown coming from the hovers ? @msivasubramaniaan , could #837 solve this issue ?

[polyzen]

https://neovim.io/doc/user/lsp.html#lsp-util
convert_input_to_markdown_lines({input}, {contents})

Not really sure how it works.

[rgrunber]

It mentions : ... Converts any of MarkedString | MarkedString[] | MarkupContent into a list of lines containing valid markdown.

However, if you look at MarkupContent, its kind field determines whether the string value is markdown or plain text. In our case, markdown is always sent back.

yaml-language-server/src/languageservice/services/yamlHover.ts

Line 93 in c657e66

kind: MarkupKind.Markdown,

.

The other thing to check is if we respect https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#hoverClientCapabilities , which allows the clients to set their preferred format through contentFormat.

[polyzen]

If value: contents, is used as before, it renders again:

[msivasubramaniaan]

@polyzen , I'm guessing Neovim doesn't handle Markdown coming from the hovers ? @msivasubramaniaan , could #837 solve this issue ?

Yes @rgrunber I hope #837 will resolve.

@polyzen Can you please share the init.vim for better understanding of plugin which you have used and configuration on the hover things.

[Kasama]

I'm facing the same issue as polyzen

I'm guessing Neovim doesn't handle Markdown coming from the hovers

Actually neovim handles markdown from hovers and parses it a bit for better stylization. The thing is that it doesn't actually handle HTML escape sequences, so the   sequences are left as-is.

Consider the following content returned from a textDocument/hover request:

{
  "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
    }
  }
}

The return itself is weird, containing   sequences between every character.

In neovim we can treat this by adding a wrapper function to handle the textDocument/hover responses

local function hover_wrapper(err, request, ctx, config)
  local bufnr, winnr = vim.lsp.handlers.hover(err, request, ctx, config)
  local contents = vim.api.nvim_buf_get_lines(bufnr, 0, -1, false)
  contents = vim.tbl_map(
    function(line)
      return string.gsub(line, " ", " ")
    end,
    contents
  )
  contents = vim.lsp.util.trim_empty_lines(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)
end

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

Even so, by replacing " " with a space " ", the content is still pretty weird, with spaces around every character.

while replacing it with nothing "" yields the expected result

---

I'm not sure I understand the reasoning behind replacing indentations with   or why it's causing it to be inserted between every character, but I believe this is a bug with this implementation.

Aside from that, I also think that neovim can benefit from properly handling HTML escape sequences, I'll create an issue there as well about this.

[Kasama]

Oh yeah, I almost forgot. To reproduce this, here's a minimal init.lua

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 = "yaml",
  callback = function(args)
    vim.lsp.start({
      name = 'yamlls',
      cmd = { 'yaml-language-server', '--stdio' },
      root_dir = vim.fn.fnamemodify('./.gitlab-ci.yaml', ':p:h'),
      settings = settings
    })
  end
})

vim.keymap.set('n', '<space>', vim.lsp.buf.hover, {})

• run cd $(mktemp -d), and copy the contents above to a file called init.lua.

• create a file called .gitlab-ci.yaml with the following contents

stages:
  - hello

• run neovim (v0.8.3 in my case) with nvim --noplugin -u init.lua .gitlab-ci.yaml.

• give it a couple of seconds to spin up yaml-language-server and press <space> with the cursor on top of the stages word.

• you can then apply the patch I showed in my last message to reproduce the "correct" behavior. (change the gsub call to replace " " with "")

[EdgeJ]

One small tweak to @Kasama's fix that will account for instances where there is no LSP information for a hover (e.g. a yaml file without an accompanying schema):

 local function hover_wrapper(err, request, ctx, config)
  local bufnr, winnr = vim.lsp.handlers.hover(err, request, ctx, config)
  if bufnr then
    local contents = vim.api.nvim_buf_get_lines(bufnr, 0, -1, false)
    contents = vim.tbl_map(
      function(line)
        return string.gsub(line, " ", "")
      end,
      contents
    )
    contents = vim.lsp.util.trim_empty_lines(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)
  end
end

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

Prevents issues like:

No information available
Error executing vim.schedule lua callback: ../.config/nvim/lsp.lua:82 Expected lua number
stack traceback:
    [C]: in function 'nvim_buf_get_lines'
    .../.config/nvim/lsp.lua:82: in function 'handler'