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

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

Merged
merged 1 commit into from
May 3, 2023

Conversation

Kasama
Copy link
Contributor

@Kasama Kasama commented Mar 26, 2023

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:
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', '<space>', vim.lsp.buf.hover, {})
  1. create a file called .gitlab-ci.yaml with the following contents
stages:
  - hello
  1. run neovim with nvim --noplugin -u init.lua .gitlab-ci.yaml.

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

image

  1. With this PR patch, the result is the following
    image

polyzen
polyzen previously approved these changes Apr 24, 2023
@polyzen
Copy link
Contributor

polyzen commented Apr 24, 2023

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

@Jomik
Copy link

Jomik commented May 1, 2023

Oh yes please. I am having this problem :)

@gorkem
Copy link
Collaborator

gorkem commented May 2, 2023

can you rebase the PR?

@Kasama
Copy link
Contributor Author

Kasama commented May 2, 2023

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
Copy link
Collaborator

gorkem commented May 2, 2023

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

The previous behavior of hover replaces indentations with the &emsp;
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 &emsp; entity between every character.

Signed-off-by: Kasama <robertoaall@gmail.com>
@Kasama
Copy link
Contributor Author

Kasama commented May 2, 2023

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

@coveralls
Copy link

coveralls commented May 2, 2023

Coverage Status

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

Copy link
Collaborator

@gorkem gorkem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gorkem gorkem merged commit 4dd3479 into redhat-developer:main May 3, 2023
4 checks passed
@Kasama
Copy link
Contributor Author

Kasama commented May 3, 2023

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

Thanks for the help!

@Kasama Kasama deleted the fix/indentation-support branch May 3, 2023 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants