lua-language-server icon indicating copy to clipboard operation
lua-language-server copied to clipboard

[Bug]: Addon Not Working

Open TurtleP opened this issue 2 years ago • 25 comments

How are you using the lua-language-server?

Visual Studio Code Extension (sumneko.lua)

Which OS are you using?

Windows

What is the issue affecting?

Annotations, Libraries

Expected Behaviour

The addon's definitions and comments should be displayed.

Actual Behaviour

The library's comments above functions are being displayed.

Reproduction steps

  1. Download and add this for an addon:

  2. Download batteries and place them inside of libraries/batteries: image

  3. Add require("libraries.batteries"):export() to the top of your lua file

  4. Hovering :export() shows the library's documented info and not from the addon

Additional Notes

I might have the addon set up incorrectly, but I am genuinely unsure. I've looked at a few of the addon samples and instructions, but can't seem to entirely figure it out properly. My settings.json for the files I'm testing with looks like the following:

{
    "Lua.workspace.library": [
        "G:/GitHub/Lua/batteries/library/batteries"
    ]
}

Log File

No response

TurtleP avatar Mar 02 '23 20:03 TurtleP

Hello 👋

Try G:/GitHub/Lua/batteries/library for the library path

carsakiller avatar Mar 03 '23 01:03 carsakiller

Doesn't seem to fix the issue, either.

TurtleP avatar Mar 03 '23 17:03 TurtleP

Can you provide more details about how your workspace looks, please?

Your screenshot shows the definition files in the addon. How does the workspace where you are trying to enable them look? Do you have any global settings? Can you provide a screenshot of the documentation that you are currently receiving?

There is a lot that can go wrong, so there is a lot to cover 🙂

carsakiller avatar Mar 03 '23 22:03 carsakiller

The screenshot I provided is how my workspace looks. I'm trying to test that the addon is working there separately to get the definitions. I don't think I have any global settings. I'm not even receiving any documentation at all.

TurtleP avatar Mar 03 '23 23:03 TurtleP

So you are trying to enable the library while the library workspace is open?

carsakiller avatar Mar 04 '23 00:03 carsakiller

Yeah, pretty much. I'm not sure why it doesn't enable even though I ask for it to.

TurtleP avatar Mar 04 '23 06:03 TurtleP

Setting workspace.library to G:/GitHub/Lua/batteries/libraries, assuming that points to libraries/ in your screenshot should work, looks like the path was just wrong. You may also need to set diagnostics.libraryFiles.

carsakiller avatar Mar 04 '23 12:03 carsakiller

From what I understand, workspace.libraries points to where the definitions of the library, not the library itself. I could be wrong, of course, but if I wanted to point it to the library itself, that path would be G:\GitHub\LOVE\tests\batteries\libraries. As a result, my current settings.json looks like:

{
    "Lua.diagnostics.libraryFiles": "Enable",
    "Lua.workspace.library": [
        "G:/GitHub/Lua/batteries/library",
        "${3rd}/love2d/library"
    ],
    "Lua.runtime.special": {
        "love.filesystem.load": "loadfile"
    }
}

and still no definitions from the stuff pops up.

TurtleP avatar Mar 05 '23 17:03 TurtleP

I see, batteries can either be required in pieces or exported entirely into the global scope. I am not sure how this could be documented to support both methods, I only know how to document either a global or module.

carsakiller avatar Mar 05 '23 17:03 carsakiller

I would think that just having the documentation set up would be fine, no? Because realistically if you require the library and export globally or in pieces, it should have all the proper definitions when the settings are applied.

require("libraries.batteries"):export() -- should show up as how I documented it
assert:some(...) -- should be documented
("something"):find("thing") -- should be documented
local stringx = require("libraries.batteries.stringx")
stringx.find(...) -- should be documented

TurtleP avatar Mar 05 '23 17:03 TurtleP

I was able to get it to work by not using :export():

local assert = require("libraries.batteries.assert")

I don't know how to make the language server understand that :export() instead includes it globally

carsakiller avatar Mar 05 '23 18:03 carsakiller

Hm.. interesting. Maybe that's a feature that would be needed to be added?

TurtleP avatar Mar 05 '23 18:03 TurtleP

I was able to get it to work by not using :export():

local assert = require("libraries.batteries.assert")

I don't know how to make the language server understand that :export() instead includes it globally

What does your config look like? I tried this and I'm not getting the documentation that I wrote out, only the comments that semi-document from the library itself, still.

according to documentation, this should look like:

{
    "Lua.diagnostics.libraryFiles": "Enable",
    "Lua.workspace.userThirdParty": [
        "G:/GitHub/Lua"
    ],
    "Lua.workspace.library": [
        "G:/GitHub/Lua/batteries/library",
        "${3rd}/love2d/library"
    ],
    "Lua.runtime.special": {
        "love.filesystem.load": "loadfile"
    }
}

but this also doesn't work, see my comment above

TurtleP avatar Mar 05 '23 18:03 TurtleP

Hm yeah, looks like I only have the basic documentation, not your custom stuff. I'm not sure how it would work with :export(), but all I can really suggest is to look at the built-in libraries to see if those help.

carsakiller avatar Mar 07 '23 00:03 carsakiller

Yeah I tried not using :export() and still only get the basic documentation.

I've been looking at all the other examples as well, but there's really nothing different on how I'm doing this. Maybe somehow the language server is taking the comments from the library as priority? That would be weird, though..

TurtleP avatar Mar 07 '23 15:03 TurtleP

fwiw - I have the definitions files hosted on this repo now. They still don't work, but I'm not quite sure as to what may be wrong still after having followed the Wiki closer and all. Maybe @sumneko has some input?

TurtleP avatar Mar 25 '23 13:03 TurtleP

Alright, I've solved the mystery, although there seems to be a bug in the language server. To fix it, I renamed all the functions in the library definitions as batteries.{module}.{function_name}. I also had to make the classes not local. Then I can do the following:

batteries = require("libraries.batteries")

local is_nan = batteries.mathx.isnan(0 / 0)
print("is_nan (0 / 0): " .. tostring(is_nan))

This gives me documentation! However, it looks like, at least in some extra testing:

  • it expects the full function name.
  • it expects the module to not be local bound to the file, e.g. local batteries = require("libraries.batteries")
  • even with a full function name and local require, it doesn't know what the function is
mathx = require("libraries.batteries.mathx")

local is_nan = mathx.isnan(0 / 0)
print("is_nan (0 / 0): " .. tostring(is_nan))

or else it gives me the library's commentary.

TurtleP avatar Mar 25 '23 15:03 TurtleP

Sorry I missed this issue before, I'm looking into it now

sumneko avatar Mar 27 '23 07:03 sumneko

For the origin issue:

  1. require uses path for searching by default, or you can use ---@meta libraries.batteries to define how to require your file.
  2. assert is a local variable in your definition file, so it cannot override global assert

For the latest issue:

  1. I can't find any file with path libraries/batteries. However batteries is a global variable, so you can use it without require.
  2. There is no file with path libraries/batteries/mathx, and mathx is not defined as global variable, so it is unknown.

I think the best way to fix it is change ---@meta to ---@meta <require-name>

sumneko avatar Mar 27 '23 07:03 sumneko

I can't find any file with path libraries/batteries. However batteries is a global variable, so you can use it without require.

This shouldn't be the case. The library has to be required for it to function.

There is no file with path libraries/batteries/mathx, and mathx is not defined as global variable, so it is unknown.

This is also false. I have it in my test environment.

I think the best way to fix it is change ---@meta to ---@meta <require-name>

I don't know how this may solve the issue at hand, though. The problem now is that:

  1. It expects batteries.{module}.{function} to be explicitly written out. Users of this library in particular don't always do that.
  2. Users requiring the module by itself, e.g. functional = require("libraries.batteries.functional") and then trying to do functional.foreach does not yield any documentation except the library's own commentary.
  • If I tried to do batteries.functional.foreach this wouldn't work: attempt to index global 'batteries' (a nil value)

TurtleP avatar Mar 29 '23 14:03 TurtleP

This is also false. I have it in my test environment.

Your example project dose not match your screenshot, please provide a correct example.

sumneko avatar Mar 30 '23 06:03 sumneko

Ah, sorry. When I mean "test environment" I am talking about as if I'm writing actual Lua code in a project, not the definitions. The screenshot I provided has the actual batteries library code in it and the settings point to my definitions files.

The example project you linked is purely the definitions that I am writing out.

TurtleP avatar Mar 30 '23 13:03 TurtleP

Could you please provide a complete example project, include your codes and libraries.

sumneko avatar Apr 24 '23 06:04 sumneko

I made a PR on their repo when this topic was still active that fixed the issues the OP was having. It looked like the library just wasn't configured correctly.

The two changes I made to their library was:

  • moving everything under library into a batteries folder
-- from this
info.json
libary/
  functional.lua
  intersect.lua
  mathx.lua
  ...
-- to this
info.json
library/
  batteries/
    functional.lua
    intersect.lua
    mathx.lua
    ...
  • making all definitions assign to a local table, exporting it, and re-importing it in init.lua
-- from this
batteries.functional = {}

function batteries.functional.identity(value) end

function batteries.functional.foreach(table, callback) end

function batteries.functional.reduce(table, seed, callback) end

-- ...
-- to this

-- in batteries/functional.lua
local functional = {}

function functional.identity(value) end

function functional.foreach(table, callback) end

function functional.reduce(table, seed, callback) end
-- ...
return functional

-- in batteries/init.lua
local batteries = {
  functional = require("batteries.functional"),
  -- ...
}

With the usual configuration of pointing Lua.workspace.library to the library folder, require("batteries") and require("batteries.[module-name]") works.

The only thing this doesn't fix is batteries:export() not injecting everything into the global namespace, which isn't easy without having access to the parse tree, typechecker, or a plugin hook/annotation that can do it for me.

goldenstein64 avatar Apr 26 '23 00:04 goldenstein64

Drop Lua. prefix if you are using .luarc.json and try again.

hinell avatar Nov 24 '23 21:11 hinell