Adding classes for constructor tables

[unlessgames]

Maybe I went too "smart" with the inheritance here, but I was lazy to type out a lot of duplicated fields across the different views. I might have missed something? In any case, this can be a good starting point to annotate each class explicitly as well.

Also cleaned up a bit of naming and aliased the number<->string functions.

Still needs work:

• commenting each field for the props (or transfering comments form the class defs)
• noting that child views can be added to the tables where appropriate
• remove the write-only fields from the classes that are only applicable when constructing

[emuell]

Maybe I went too "smart" with the inheritance here, but I was lazy to type out a lot of duplicated fields across the different views

Always good to be lazy :) On the other hand, it might be easier for me to maintain this in the future if it follows Renoise's internal view inheritance.

View -> Control -> SomeSpecializedControl
     -> SomeSpecializedView 

But either way this is fine for now. So you decide what's easier to manage for now. Especially because what we have now is already a great improvement.

I won't have time to go over this in detail over the weekend, but I will continue this next week.

Until then, thanks a lot for your very helpful contributions. It really helps a lot!

[unlessgames]

Sure thing!

I made it so the _Properties classes mirror the list you've shared.

[unlessgames]

• Converted every type for views to an alias to make editing the doc comments easier: change the comments for the aliases and it changes for both the class and the constructor table classes.
• Removed the write-only bind/notifier fields from the classes.
• Cleaned up the whole document a bit so it's the same order everywhere, "View class header, aliases, description, properties, functions"
• Moved defaults to the alias descriptions to show up in editor hints for both the constructor tables and classes (added * to force rendering the default on a new line)
• Fixed a few wrong types for properties

It all seems to work nicely in practice.

[unlessgames]

I copied the explanation of the show_custom_dialog into application.lua, since that way it would show up as a hint when calling this function.

• Much of the explanation can be encoded in the type system itself so I left the parts that talk about something being optional to the ? notation in the new KeyEvent class def.
• The keyhandler function had the wrong type here
• TODO I'm not sure if modifiers uses "command" or "control" on Mac, the ModifierStates alias might need to be extended.

After revision, much of the top section from view_builder.lua could be dropped imo.

[emuell]

Converted every type for views to an alias to make editing the doc comments easier: change the comments for the aliases and it changes for both the class and the constructor table classes.

Great idea. That works nicely!

TODO I'm not sure if modifiers uses "command" or "control" on Mac, the ModifierStates alias might need to be extended.

That indeed is platform specific and can be a combination of:

• Windows: "Control"|"Shift"|"Alt"|"WinKey"
• Linux: "Control"|"Shift"|"Alt"|"Meta"
• Mac: "Command"|"Shift"|"Option"|"Control"

So maybe just noting this (combination of XXX) will be enough here?

The keyhandler function had the wrong type here

Apropos. The key handler also supports optional contexts, like all other notifiers/callbacks. This wasn't properly documented before. We could all them as overloads:

---When returning the passed key from the key-handler function, thekey will be 
---passed back to Renoise's key event chain, in order to allow processing global 
---Renoise key-bindings from your dialog. This will not work for modal dialogs. 
---This also only applies to global shortcuts in Renoise, because your dialog will
---steal the focus from all other Renoise views such as the Pattern Editor, etc.
---@alias KeyHandlerFunction fun(dialog: renoise.Dialog, key_event: KeyEvent): KeyEvent?
---@alias KeyHandlerMemberFunction fun(self: NotifierMemberContext, dialog: renoise.Dialog, key: KeyEvent): KeyEvent?
---@alias KeyHandlerMethod1 {[1]:NotifierMemberContext, [2]:KeyHandlerMemberFunction}
---@alias KeyHandlerMethod2 {[1]:KeyHandlerMemberFunction, [2]:NotifierMemberContext}

---Opens a modal dialog with a title, custom content and custom button labels.
---
---See Renoise.ViewBuilder.API for more info.
---@param title string Message box title.
---@param content_view renoise.Views.View Message box content view.
---@param button_labels string[]? Default: {"Ok"}
---@param key_handler KeyHandlerFunction? Optional notifier function for keyboard events in the dialog.
---@param key_handler_options KeyHandlerOptions? Optional table with the fields: ```{ "send_key_repeat": true/false, "send_key_release": true/false }```
---@overload fun(title: string, content_view: renoise.Views.View, button_labels: string[], key_handler: KeyHandlerMethod1?, key_handler_options: KeyHandlerOptions?): string
---@overload fun(title: string, content_view: renoise.Views.View, button_labels: string[], key_handler: KeyHandlerMethod2?, key_handler_options: KeyHandlerOptions?): string
---@return string label
function renoise.Application:show_custom_prompt(title, content_view, button_labels, key_handler, key_handler_options) end

---Shows a non modal dialog (a floating tool window) with custom content.  
---When no key_handler is provided, the Escape key is used to close the dialog.
---
---@see renoise.ViewBuilder for more info about custom views.
---@param title string Dialog title.
---@param content_view renoise.Views.View dialog content view.
---@param key_handler KeyHandlerFunction? Optional notifier function for keyboard events in the dialog.
---@param key_handler_options KeyHandlerOptions? Optional table with the fields: ```{ "send_key_repeat": true/false, "send_key_release": true/false }```
---@overload fun(title: string, content_view: renoise.Views.View, key_handler: KeyHandlerMethod1?, key_handler_options: KeyHandlerOptions?): renoise.Dialog
---@overload fun(title: string, content_view: renoise.Views.View, key_handler: KeyHandlerMethod1?, key_handler_options: KeyHandlerOptions?): renoise.Dialog
---@return renoise.Dialog
function renoise.Application:show_custom_dialog(title, content_view, key_handler, key_handler_options) end

After revision, much of the top section from view_builder.lua could be dropped imo.

Agree. Nobody will read this anyway. It's great to have this now in the context where it's actually used.

Apart from the "--TODO note possible child views" that's it, isn't it? Really turned out to be the most complex part of the type definition. Thanks again for your help.

---

Change whatever you see that fits here and let me know when you are happy with it. I'll give it another quick test and merge it.

[unlessgames]

I think we can use a union type for the key_handler here instead of an overload.

That indeed is platform specific and can be a combination of:

• Windows: "Control"|"Shift"|"Alt"|"WinKey"
• Linux: "Control"|"Shift"|"Alt"|"Meta"
• Mac: "Command"|"Shift"|"Option"|"Control"

You mean "mac: control | shift | option | command"? Control on other platforms corresponds to control on Mac, right?

I didn't know meta key was also available here (doesn't work for me on Linux). In that case listing all permutations might be excessive.

That being said this API is kinda confusing because you have to know the precedence rules as well (shift > alt > control) or use find on the string instead. Maybe in the future this could be deprecated and abstracted away across platforms? Since I don't think many tools want to do something different for Win/Mac/Linux anyway, so it could be a just table of booleans instead of a string, like { control : boolean, shift : boolean, alt : boolean, meta : boolean} which would be easier to document, check against and handle the cross-platform nature as a dev with a single machine.

Where is the meta key in the precedence order? And is it formatted as "winkey" on Windows?

Tell me what you think of the Modifier explanation I did.

As for TODOs maybe it would be better to move or somehow share the class definition comments (and ASCII art) with the builder functions, since those are what you use more while coding, and they show nothing currently, you'd have to manually look up the class to get to the general explanation of each view type. Now I've put @see links pointing to the class from the constructors to make this a bit better. Unfortunately the @see links from class->constructor didn't seem to show up.

[unlessgames]

I've converted every instance of optional types from the type|nil to the type? syntax to be consistent.

[emuell]

Tell me what you think of the Modifier explanation I did.

Looks good!

I think we can use a union type for the key_handler here instead of an overload.

This also works, but makes the signature harder to read because it's super long.

That being said this API is kinda confusing because you have to know the precedence rules as well

Yes, agree. That should be changed in future.

Where is the meta key in the precedence order? And is it formatted as "winkey" on Windows?

Yes, it's "WinKey". The order is "Shift" < "Alt" < "Control" < "Meta".

[unlessgames]

I found another issue with the definitions.

Since the constructor functions are defined on the ViewBuilder class, you can write the things below, none of which is actually possible, yet the language server will not say anything.

-- text doesn't exist on the ViewBuilder class at runtime
rprint( renoise.ViewBuilder:text { text = "bla" } )

-- views doesn't exist either
rprint( renoise.ViewBuilder.views )

vb = renoise.ViewBuilder()
-- constants don't exist on a ViewBuilder instance
print(vb.DEFAULT_DIALOG_MARGIN)

I fixed this by adding a local dummy class ViewBuilderInstance which makes all the above lines be flagged by the lsp.

[emuell]

I've converted every instance of optional types from the type|nil to the type? syntax to be consistent.

Agree. But I'd prefer to keep things focused on the ViewBuilder here and do this later in a separate commit.

[emuell]

I fixed this by adding a local dummy class ViewBuilderInstance which makes all the above lines be flagged by the lsp.

Cool. That actually is a problem in the entire API and the XXXInstance thing looks like a clever fix. I'll check if we can make this a pattern in the API.

[unlessgames]

Yes, it's "WinKey". The order is "Shift" < "Alt" < "Control" < "Meta".

You mean it's in PascalCase? On Linux all the modifiers are lowercase. Is this different for other platforms or just this one key?

Do you reckon the precedence should be noted as 1 < 2 < 3 instead of 1 > 2 > 3? Guess it could be just with a + sign instead.

[emuell]

You mean it's in PascalCase? On Linux all the modifiers are lowercase. Is this different for other platforms or just this one key?

No, sorry. They are all lower in the API, internally they are not. I've confused things here.

Guess it could be just with a + sign instead.

Yep!

[unlessgames]

I see, if there is nothing else I think this might be it for the view_builder definition.

I've added the nested view hints to the appropriate constructor functions to tick off that TODO.

[emuell]

Looks great to me. If you're happy with it, let's merge it.

Thanks again for all your help. I couldn't have done it alone with all the details!

My TODOs for the next release regarding the viewbuilder:

• add new "modifier_flags" property to KeyEvent and make old "modifiers" deprecated:

  { 
  command: boolean,  -- Command key on macOS else Control
  shift  boolean, 
  alt  boolean, 
  meta: boolean  -- Control key on macOS, Windows key on Windows, Super key on Linux
  } 

• add new "views" property to renoise.Views.View to specify child/sub views in space/rack/aligned as property, instead of mixing array/key-value notation.

Anything else?

[unlessgames]

Happy to help!

Can't think of anything else to improve right now, so merging would be fine.

 command: boolean,  -- Command key on macOS else Control
 shift  boolean, 
 alt  boolean, 
 meta: boolean  -- Control key on macOS, Windows key on Windows, Super key on Linux
}

*control : boolean I suppose to align with the naming convention of the majority

Anything else?

Technically these can be worked around right now but I would love to have

• ViewInstance.views which would return all the child views added to a particular view. Would be nice paired with the mentioned views property on the constructor table as it could mirror that on the created instance.
• ViewInstance:clear_children() a convenience function which would allow us to remove every child of the view without having to hold a reference to each. Although having a views property on the created instances would make this easier to implement, so it's not as needed if that exists.