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

WIth backends set to both LSP and treesitter, it fallbacks to treesitter initially #68

Closed
rockyzhang24 opened this issue Mar 18, 2022 · 14 comments
Assignees
Labels
bug Something isn't working

Comments

@rockyzhang24
Copy link

rockyzhang24 commented Mar 18, 2022

Describe the bug
First I have this setting backends = {"lsp", "treesitter"}. If I am correct, I think this means use LSP as the backend and fallback to treesitter if LSP is unavailable.

I opened a buffer like init.vim (with viml LSP server enabled), and then I open aerial window. aerial fallback to treesitter, i.e., instead of showing symbols from LSP, it only showed symbols from treesitter. To make all the symbols from LSP to be shown, I have to make some changes like inserting an empty line or deleting a char. I think the expected behavior should be that all symbols from LSP will be shown as soon as I open aerial. Because I already have viml LSP server enabled, it shouldn't use treesitter.

If set backends = {"lsp"}, then all symbols from LSP will be shown immediately when aerial window is opened, i.e., no need to change text before showing the symbols.

System information

  • OS: macOS (the latest version)
  • Neovim version: NVIM v0.7.0-dev+1274-gc1b98cfa5
  • AerialInfo:
Aerial Info                                                                                                                                                                                                
-----------                                                                                                                                                                                                
Filetype: vim                                                                                                                                                                                              
Configured backends:                                                                                                                                                                                       
  lsp (supported) (attached)                                                                                                                                                                               
  treesitter (supported)                                                                                                                                                                                   
  markdown (not supported) [Filetype is not markdown]                                                                                                                                                      
Show symbols: all symbols 
  • Aerial config:
require("aerial").setup {

  -- Mappings
  on_attach = function(bufnr)
    -- Toggle aerial window
    vim.api.nvim_buf_set_keymap(bufnr, 'n', '\\s', '<Cmd>AerialToggle!<CR>', {}) -- "s" for symbols
    -- Jump forwards/backwards
    vim.api.nvim_buf_set_keymap(bufnr, 'n', '<Leader>{', '<cmd>AerialPrev<CR>', {})
    vim.api.nvim_buf_set_keymap(bufnr, 'n', '<Leader>}', '<cmd>AerialNext<CR>', {})
    -- Jump up the tree
    vim.api.nvim_buf_set_keymap(bufnr, 'n', '<Leader>[[', '<cmd>AerialPrevUp<CR>', {})
    vim.api.nvim_buf_set_keymap(bufnr, 'n', '<Leader>]]', '<cmd>AerialNextUp<CR>', {})
    -- Fuzzy finding symbols (it respects backends and filter_kind)
    vim.api.nvim_buf_set_keymap(bufnr, 'n', '<Leader>fs', '<cmd>Telescope aerial<CR>', {})
  end,

  backends = { "lsp", "treesitter", "markdown"},
  close_behavior = "auto",
  min_width = 40,
  max_width = 40,
  show_guides = true,

  -- Set it to false to display all symbols
  filter_kind = false,
}

Screenshots
I made a show demo video below

Screen.Recording.2022-03-18.at.14.20.46.mov

Thank you very much.

@rockyzhang24 rockyzhang24 added the bug Something isn't working label Mar 18, 2022
@rockyzhang24 rockyzhang24 changed the title Have to make some changes to make LSP backend work when setting backend to both LSP and treesitter After setting backends to both LSP and treesitter, I have to make changes on text first to show all the symbols from LSP Mar 18, 2022
@rockyzhang24 rockyzhang24 changed the title After setting backends to both LSP and treesitter, I have to make changes on text first to show all the symbols from LSP WIth backends set to both LSP and treesitter, it fallbacks to treesitter initially Mar 18, 2022
stevearc added a commit that referenced this issue Mar 19, 2022
The specific issue was because the viml lsp doesn't provide diagnostics
when a file is opened, so it wouldn't trigger a symbol fetch. The simple
fix is to always fetch symbols on attach.
@stevearc
Copy link
Owner

Should be fixed now. Thanks for the report!

@rockyzhang24
Copy link
Author

rockyzhang24 commented Mar 19, 2022

Updated and the issue sill exists.

Before this fix, this issue happened on not only the viml buffer, but on other filetype buffers such as go. I tested on the fzf repo and opened ./main.go, and I noticed this issue as well. Also, the some issue happened on JS buffer. In my post above, I just used viml buffer as an example.

Just now I updated aerial and I opened ./main.go in fzf, it seemed this issue was resolved for go buffer. However, when I tested it on init.vim, the issue was still there.

Thank you.

@stevearc
Copy link
Owner

Just to confirm: you are still seeing the behavior where you have to make changes to the buffer in order to see the LSP symbols?

Because treesitter is so much faster than LSP, we do expect to see the treesitter symbols first, but the LSP symbols should take over after a second or so.

I unfortunately can't reproduce the issue anymore on any files (go, vim, etc) so I won't be able to debug it myself. The first thing you should check is that we're actually fetching the symbols here

And once you confirm that, make sure the symbols callback is being called and not hitting an early return

M.symbol_callback = function(_err, result, context, _config)

@rockyzhang24
Copy link
Author

rockyzhang24 commented Mar 23, 2022

Just to confirm: you are still seeing the behavior where you have to make changes to the buffer in order to see the LSP symbols?

Yes, I have to make some changes (like insert a new line by o) to make LSP symbols to be shown, but just for viml. I tried go, lua and js, no issues there.

Because treesitter is so much faster than LSP, we do expect to see the treesitter symbols first, but the LSP symbols should take over after a second or so.

For viml buffer, I tried to open aerial for a while but still no LSP symbols shown, just treesitter symbols there. I did a further test for a lua buffer. You know in neovim the initialization of lua LSP server takes quite a while. I opened a lua buffer and then I opened aerial immediately. It showed "No symbols" when the LSP server was initializing and no treesitter symbols. After that, all the LSP symbols were shown.

It's so weird it only happens for viml.

Btw, here is my aerial setup.

Thank you very much.

@stevearc
Copy link
Owner

The fact that it's only the vim language server makes me suspicious. Try upgrading it to make sure you're on the latest version. Also, since I can't reproduce it locally the best I can do is give you some ideas of how to debug it yourself. Try putting some print statements in the locations mentioned in the previous comment. The results should help narrow it down a bit.

@rockyzhang24
Copy link
Author

Okay. I will try that later. Thank you for you help. Btw, if I remove "treesitter" from backends and only left "lsp" in it, it worked perfectly and no any issues.

@stevearc
Copy link
Owner

Interesting. One workaround you can use for now is to specifically exclude the treesitter backend for vim files

require('aerial').setup({
  backends = {
    ['_'] = {'lsp', 'treesitter', 'markdown'},
    vim = {'lsp'},
  }
})

@rockyzhang24
Copy link
Author

rockyzhang24 commented Mar 23, 2022

  1. In M.fetch_symbols function, what I tried is
M.fetch_symbols = function()
  vim.lsp.buf.document_symbol()
  print("hello")
end

And I can definitely see the output "hello" for a vim buffer.

  1. In M.symbol_callback function, I added a print() as well
M.symbol_callback = function(_err, result, context, _config)
  local client_id = context.client_id
  if not result or vim.tbl_isempty(result) then
    return
  end

  print("hello")

  local bufnr = context.bufnr
  -- Don't update if there are diagnostics errors, unless config option is set
  -- or we have no symbols for this buffer
  if
    not config.lsp.update_when_errors
    and data:has_symbols(bufnr)
    and get_error_count(bufnr, client_id) > 0
  then
    return
  end

  -- Debounce this callback to avoid unnecessary re-rendering
  if results[bufnr] == nil then
    vim.defer_fn(function()
      local r = results[bufnr]
      results[bufnr] = nil
      M.handle_symbols(r, bufnr)
    end, 100)
  end
  results[bufnr] = result
end

I couldn't see "hello" output, so it hit the first return.

Any ideas? Thank you.

@stevearc
Copy link
Owner

I suspect that it's a race condition in the vimls server. It could be that we request the symbols so soon after the file is opened that it's still parsing, and instead of waiting until parsing is complete it returns an empty list. If that's the case then it would be resolved by fetching the symbols slightly later.

Try replacing


with

vim.defer_fn(M.fetch_symbols, 10)

However that wouldn't explain why it works when the treesitter backend is disabled. So perhaps it's something else entirely

@rockyzhang24
Copy link
Author

rockyzhang24 commented Mar 23, 2022

I made this change and it still didn't work. Next, I tried 100 instead of 10, and the issue was still there. Then I changed it to 1000. This time, treesitter symbols were shown first and then the LSP symbols were displayed immediately. I recorded a short demo below.

Screen.Recording.2022-03-22.at.23.03.52.mov

Thank you.

Btw, my treesitter setup is here.
and my lspconfig setup is here.

@rockyzhang24
Copy link
Author

Hi @stevearc any updates for this? Thanks a lot.

rockyzhang24 added a commit to rockyzhang24/dotfiles that referenced this issue Apr 6, 2022
Cannot set multiple backends for vim due to an issue
See stevearc/aerial.nvim#68
@stevearc
Copy link
Owner

stevearc commented Apr 7, 2022

This is a bug in the language server. I've submitted a fix iamcco/vim-language-server#72

@rockyzhang24
Copy link
Author

Great. Thank you.

@rockyzhang24
Copy link
Author

This issue was solved after your PR to vim language server was merged. Thank you very much.
I will close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants