renoise / definitions

LuaCATS definitions for the Renoise Lua API
4 stars 4 forks source link

fixes for Document #21

Closed unlessgames closed 4 months ago

unlessgames commented 4 months ago

Fixed a few types that were wrong in document.lua.

Also, before having the template on line 124, user-defined preferences couldn't be annotated without warning from the LSP

For example


---@class MyPrefs : renoise.Document.DocumentNode
---@field my_string renoise.Document.ObservableString

---@type MyPrefs
prefs = renoise.Document.create("ScriptingToolPreferences")({ my_string = "abc" })
--^^^^ the LSP would complains here that DocumentNode couldn't be assigned to MyPrefs

Not sure how the fix works exactly since there is no "template typing" in other places where DocumentNode is defined but it makes the warning go away while still providing the same useful type hints later on.

emuell commented 4 months ago

Not sure how the fix works exactly since there is no "template typing" in other places where DocumentNode is defined but it makes the warning go away while still providing the same useful type hints later on.

Same for function renoise.Document.instantiate(model_name) end

But this breaks the following:

-- no @type here!
local my_prefs = renoise.Document.create("ScriptingToolPreferences"){my_string = "abc"}
my_prefs:add_property("bla", 1) --^Undefined field `add_property`.Lua Diagnostics.(undefined-field)
-- my prefs is not a renoise.Document.DocumentNode anymore

Probably the @type cast works because renoise.Document.DocumentNode<ObservableTypes> is now an invalid and thus any type?

See also https://github.com/LuaLS/lua-language-server/issues/2468

The fix mentioned there (inherit the base class from table) also seems to work in our case, but I'm not sure why and if it breaks other things:

---A document node is a sub component in a document which contains other
---documents or observables.
---@class renoise.Document.DocumentNode : table
--- ...
unlessgames commented 4 months ago

Ah, I missed that! Converted to using table instead. While it might break something, the current state is evidently more broken so I guess it would be better for now to have it this way. I left a TODO comment to make it easier to spot later.