renoise / definitions

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

fix return type for song:track() #26

Closed unlessgames closed 2 months ago

unlessgames commented 4 months ago

Song:track() can return either a Track or a GroupTrack, otherwise the lsp can complain for accessing members and group_collapsed. Would be nice to be able to force a check for the type like with optionals, but I'm not sure that's possible.

emuell commented 4 months ago

Would be nice to be able to force a check for the type like with optionals

That must be a runtime check, so it actually might be better to keep it return the base class here.

Also surprisingly, this works:

local track = renoise.song():track(1)
if type(track) == "renoise.GroupTrack" then
  -- this is correctly narrowed down as group track now
  track.members[1]:set_column_is_muted(1, true)
end

local other_track = renoise.song():track(2)
if other_track.type == renoise.Track.TRACK_TYPE_GROUP then
  -- here not. it's unfortunately still a renoise.Track, but could be annotated with
  ---type renoise.GroupTrack then manually
  other_track.members[1]:set_column_is_muted(1, true)
end
unlessgames commented 4 months ago

Makes sense. In that case I converted the members to use only renoise.Track[] as well, and added your example about how to narrow the type to both.

emuell commented 4 months ago

Just have double checked this and unfortunately type(some_track) returns "Track" and not "renoise.Track". in Renoise. Need to think about how to get that prefix fixed or added in the API, or if there's another solutions for this.

This definitely will work, but is a bit more bloated:

local track = renoise.song():track(2)
if track.type == renoise.Track.TRACK_TYPE_GROUP then
  ---@type renoise.GroupTrack 
  local group_track = track
  group_track.members[1]:set_column_is_muted(1, true)
end
emuell commented 4 months ago

Same is true (exactly the same problem) for the viewbuilder.views property:

local vb = renoise.ViewBuilder()
vb:text({id = "my_text", text = "qweqwe"})

--- when accessing by id, we need to cast:
---@type renoise.Views.Text
local my_text = vb.views.by_text

type would work here too, but the renoise.Views prefix is missing here too in the runtime type info.

I've checked if it's possible to get the namespace into the type names, but unfortunately it is not , well, without hacking into https://luabind.sourceforge.net/docs.html which we're using for the Lua class bindings.

So ---@type renoise.Views.Text is the only way do such upcasts for now.

emuell commented 4 months ago

... and just like https://github.com/renoise/definitions/pull/21 renoise.View.View then needs to inherit from table:

---@class renoise.Views.View : table
unlessgames commented 4 months ago

Hmm... I guess using @as can also work but it is similarly bloated.

local group_track = track --[[@as renoise.GroupTrack]]
unlessgames commented 4 months ago

But @cast seems the cleanest, at least no need for another variable (also works if there are more lines in the block).

local track = renoise.song():track(2)
if track.type == renoise.Track.TRACK_TYPE_GROUP then
  ---@cast track renoise.GroupTrack 
  track.members[1]:set_column_is_muted(1, true)
end

Maybe this should be added to the docs until a better solution is reached?

emuell commented 4 months ago

---@cast track renoise.GroupTrack

Yes, that's better!

emuell commented 2 months ago

type would work here too, but the renoise.Views prefix is missing here too in the runtime type info.

I've checked if it's possible to get the namespace into the type names, but unfortunately it is not , well, without hacking into https://luabind.sourceforge.net/docs.html which we're using for the Lua class bindings.

So ---@type renoise.Views.Text is the only way do such upcasts for now.

I'm going to close this and open a new PR for this.


Just thought of a possible workaround for the problem mentioned above. We anyway wanted to separate the class Instances from the class that holds the constants.

e.g.

renoise.ApplicationWindow.UPPER_FRAME_TRACK_SCOPES

vs:

renoise.app().window.UPPER_FRAME_TRACK_SCOPES (which should not be possible)

By using ApplicationWindow as the instance @class in LuaLS, and renoise.ApplicationWindow for the constants, we actually could make the if type(window) == "ApplicationWindow" casts work:

So class instances don't have a renoise. prefix, but the table which hold the constants do.

This is less clear than naming it renoise.ApplicationWindowInstance or ApplicationWindowInstance but would solve both problems.