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

Making LuaLS correctly detect all the entities #17

Closed
wants to merge 1 commit into from

Conversation

mjrogozinski
Copy link

Removing diagnostics section made LSP
to see all the fields inside of vim.

Additional entries in luarc make LSP detect all entities from busted and luassert library.

For some reason it did not work out of the box in my environment.

Removing diagnostics section made LSP
to see all the fields inside of vim.
@ColinKennedy
Copy link
Owner

Have you run make llscheck? On my side it errors a few hundred times with Undefined global vim. The GitHub workflow on this PR says passing but that's only because it actually runs a different ~.github/workflows/.luarc.json file. I'd imagine if the GitHub workflow used the same .luarc.json it would give the same errors

Copy link
Owner

@ColinKennedy ColinKennedy left a comment

Choose a reason for hiding this comment

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

You mentioned LSPs - So this PR is an attempt to get better autocomplete and what not in Neovim, right? In truth I don't have that working on my personal configuration currently so I didn't notice any issues (I only run diagnostics via the terminal).

Do you have a resource or tutorial that you can recommend I follow along so I can see the issues that you're seeing? I have lspconfig so I'm aware of the describe / after / etc not found but nothing so far related to auto-completing vim.* entries.

Comment on lines +7 to +9
"${3rd}/busted/library",
"${3rd}/luv/library",
"${3rd}/luassert/library",
Copy link
Owner

Choose a reason for hiding this comment

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

${3rd} will be deprecated in future versions of lua-language-server so I'd like to avoid using it.

Try running git submodule update --init --recursive and then changing this file to

    "workspace.library": [
        "$PWD/.dependencies/busted/library",
        "$PWD/.dependencies/luassert/library",
        "lua"
    ],

That works for me. If it works for you to let's replace it here.

"lua/?/init.lua",
"?.lua",
"?/init.lua",
"$XDG_DATA_HOME/nvim/lazy",
Copy link
Owner

@ColinKennedy ColinKennedy Oct 27, 2024

Choose a reason for hiding this comment

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

What benefit is this line $XDG_DATA_HOME/nvim/lazy doing exactly? The other lines are evident to me except for this one.

@ColinKennedy
Copy link
Owner

Oh also @mjrogozinski would you please also update the ~/.github/workflows/.luarc.json with your changes? (The only reason they're separate files is because GitHub is weird about subfolders so I had to add "workspace.ignoreDir": [".lua", ".luarocks"],.

If you do that and then re-push, the llscheck workflow will run and we can see how it fares on a vanilla Neovim install

@ColinKennedy
Copy link
Owner

ColinKennedy commented Nov 12, 2024

@mjrogozinski I reproduced the LSP issues that you mentioned in this PR but fixed it in a different way that should future-proof the code better. That PR is over here - #19

I checked llscheck locally + llscheck CI. Both are working. I also don't see any LSP-related issues anymore.

Please let me know if the latest tagged release, v1.3.2, works for you. Thank you for raising this issue!

@mjrogozinski
Copy link
Author

@mjrogozinski I reproduced the LSP issues that you mentioned in this PR but fixed it in a different way that should future-proof the code better. That PR is over here - #19

I checked llscheck locally + llscheck CI. Both are working. I also don't see any LSP-related issues anymore.

Please let me know if the latest tagged release, v1.3.2, works for you. Thank you for raising this issue!

@ColinKennedy I have checked your solution on my setup. Everything works perfectly (as long as you clone the repo with submodules, of course). This is exactly the way I wanted the project to behave.

Thank you!
I agree that your solution is better, so let's close this issue.

@ColinKennedy
Copy link
Owner

Glad to hear it's working for you! Please let me know if you have any other suggestions

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.

2 participants