redhat-developer / yaml-language-server

Language Server for YAML Files
MIT License
1.1k stars 264 forks source link

fix(indentation): hover behavior when indentation not set #863

Closed Kasama closed 1 year ago

Kasama commented 1 year ago

What does this PR do?

The previous behavior for hover replaces indentations with the   entity, because Markdown treats whitespace at the beginning of a line in a special way. However, when the indentation setting wasn't set, the regex that does that replace was set to empty ("") and so would include an   entity between every character.

This went under the radar because the previous PR #844 also changed the tests setup to include a .withIndentation call, which made sure all tests were passing.

In this PR I'm reverting that to the default behavior for most tests, but overriding the default only on the test introduced in #844.

What issues does this PR fix or reference?

The issue is referenced on a comment of PR #844. here

Is it tested? How?

I have tested this fix with a neovim setup as described in the original problem report in the PR comment.

At the risk of simply repeating myself, below is the full set of reproduction steps:

Open for steps 1. Download neovim (I used v0.8.3) 2. Create a new folder `cd $(mktemp)` 3. create a file named `init.lua` with the following contents: ```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' }, -- replace with local installation as necessary root_dir = vim.fn.fnamemodify('./.gitlab-ci.yaml', ':p:h'), settings = settings }) end }) vim.keymap.set('n', '', vim.lsp.buf.hover, {}) ``` 5. create a file called `.gitlab-ci.yaml` with the following contents ```yaml stages: - hello ``` 6. run neovim with `nvim --noplugin -u init.lua .gitlab-ci.yaml`. 7. give it a couple of seconds to spin up `yaml-language-server` and press `` with the cursor on top of the `stages` word. ![image](https://user-images.githubusercontent.com/1714520/227104127-7cc0adb5-59a0-4b8a-9d1d-8ce64394a83e.png) 8. With this PR patch, the result is the following ![image](https://user-images.githubusercontent.com/1714520/227812133-55ee5b7b-0b6a-4599-82dc-b2eaff4f37e8.png)
polyzen commented 1 year ago

Works for me. Backporting to the Arch Linux package ❤️

Jomik commented 1 year ago

Oh yes please. I am having this problem :)

gorkem commented 1 year ago

can you rebase the PR?

Kasama commented 1 year ago

It seems the actual issue has been fixed at #831 here, but I think the test changes are still relevant.

WYT?

I have rebased it, anyway

gorkem commented 1 year ago

CI failed with a prettier error. I did not notice that #831 fixed this on the side but did not provide any tests. This is a good complement PR, and I would like to merge it if you can update it for the prettier fix

Kasama commented 1 year ago

fixed. It seems my auto-format wasn't setup to use the project's config :smile:

coveralls commented 1 year ago

Coverage Status

Coverage: 83.304%. Remained the same when pulling ab85743b1cb40fb13daf2316a2ce48270cec2b24 on Kasama:fix/indentation-support into cf3f79202524ba979f1f4011636b85d08d81bdcb on redhat-developer:main.

Kasama commented 1 year ago

Nice, would be great to have a release soon, so we can have this fix rolled out!

Thanks for the help!