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

Add typing to widgets and gui #4577

Merged
merged 29 commits into from
May 17, 2024
Merged

Add typing to widgets and gui #4577

merged 29 commits into from
May 17, 2024

Conversation

vallode
Copy link
Contributor

@vallode vallode commented May 9, 2024

This PR aims to add type support to widgets.lua and gui.lua and as a consequence extend the typing of a few other files as well (namely some globals and class.lua).

There are some conventions being established here, feel free to critique anything and everything for the sake of establishing a good foundation.

When defining new classes we should follow this rough pattern for a good experience:

---@class moduleName.ClassNameAttrs: moduleName.ParentClassAttrs
---@field foo integer This is never nil
---@field bar? integer This can be nil

---@class moduleName.ClassNameInitTable: moduleName.ClassNameAttrs
---@field foo integer This needs to be provided during init
-- This also inherits all attributes from ClassNameAttrs but they are optional.

---@class moduleName.ClassName: moduleName.ParentClass, moduleName.ClassNameAttrs
---@field super moduleName.ParentClass
---@field ATTRS moduleName.ClassNameAttrs|fun(attributes: moduleName.ClassNameAttrs)
---@overload fun(init_table: moduleName.ClassNameInitTable): self
ClassName = defclass(ClassName, Widget)

There are a few caveats:

  • defclass(ClassName) plays weird with LuaLS and self parameters in the module file will have to be manually cast as the class, i.e:
---@param self moduleName.ClassName
---@param init_table moduleName.ClassNameInitTable
function ClassName:init(init_table) end```
- `subviews` indexing has no autocomplete or type support so subviews accessed need to be cast manually, i.e
```lua
---@type widgets.List
local list = self.subviews.list

@vallode
Copy link
Contributor Author

vallode commented May 10, 2024

pre-commit.ci autofix

@vallode
Copy link
Contributor Author

vallode commented May 10, 2024

@myk002 gui.lua is probably ready for a first pass to start setting in stone some of the conventions we might use in the future. Namespacing is one issue dfhack.gui.Painter vs gui.Painter. As well as where to keep things like dfhack.color and dfhack.pen, inline whereever fits or maybe a more generic types.lua file also works. I'll let you look over it before I continue further with typing all the methods in gui and widgets.

Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

looks like a great start! class names look good to me

Comment on lines 22 to 25
---@class dfhack.class: class.common_methods, class.class_obj
---@field super any
---@field ATTRS fun(attributes: table)
---@overload fun(attributes: table): self
Copy link
Member

Choose a reason for hiding this comment

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

is this attached to anything? or does it get combined into defclass below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This overload is attached to the class itself, it's mostly boilerplate.

library/lua/class.lua Outdated Show resolved Hide resolved
library/lua/class.lua Outdated Show resolved Hide resolved
library/lua/dfhack.lua Outdated Show resolved Hide resolved
@@ -8,6 +8,20 @@ local utils = require('utils')
local dscreen = dfhack.screen
local getval = utils.getval

---@class dfhack.pen
---@field ch? string
Copy link
Member

Choose a reason for hiding this comment

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

I think this is more commonly an integer, though a string character also works (iirc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is probably a candidate for an alias/class like dfhack.pen or dfhack.truthy.

---@field pen? pen
---@field dpen? pen
---@field hpen? pen
---@field on_activiate? function
Copy link
Member

Choose a reason for hiding this comment

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

specifically, a function that takes no params and returns no value

Comment on lines 1453 to 1455
---@field line? any
---@field x1? any
---@field x2? any
Copy link
Member

Choose a reason for hiding this comment

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

these three are internal; do they need to be declared here?

Comment on lines 1459 to 1461
---@field text_pen? any
---@field text_dpen? any
---@field text_hpen? any
Copy link
Member

Choose a reason for hiding this comment

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

colors or Pens

---@field auto_width? boolean
---@field on_click? function
---@field on_rclick? function
---@field scroll_keys? widgets.Keys
Copy link
Member

Choose a reason for hiding this comment

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

scroll_keys is a map from keybinding strings to either an integer or a string

library/lua/gui/widgets.lua Outdated Show resolved Hide resolved
library/lua/dfhack.lua Outdated Show resolved Hide resolved
This was causing issues in parsing and I didn't want to change the parser in the definitions repository for this single instance.
@vallode
Copy link
Contributor Author

vallode commented May 12, 2024

@myk002 getting closer to completion, feel free to ignore widgets.lua below TextButton I ran out of steam a bit. Also feel free to leave this for a day or two before I get it closer to being 100% done. Cheers!

@vallode vallode marked this pull request as ready for review May 14, 2024 07:52
@vallode
Copy link
Contributor Author

vallode commented May 14, 2024

pre-commit.ci autofix

---@alias dfhack.truthy
---| true
---| integer
---| string Not an empty string ""
Copy link
Member

Choose a reason for hiding this comment

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

empty strings are truthy. only nil and false is falsey

if you really want to be complete, you can add userdata to this list

library/lua/gui.lua Outdated Show resolved Hide resolved
local function show_view(view,vis)
if view then
view.visible = vis
end
end

---@param tab table
Copy link
Member

Choose a reason for hiding this comment

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

tab? table<T>

Copy link
Contributor Author

@vallode vallode May 17, 2024

Choose a reason for hiding this comment

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

As mentioned on Discord, I think this doesn't work. Full example would be something like:

---@generic T
---@param tab? table<T>
---@param idx integer
---@return T
local function map_opttab(tab,idx)
    if tab then
        return tab[idx]
    else
        return idx
    end
end

---@type string[]
local foo
local bar = map_opttab(foo, 0)

Where bar remains unknown

local function show_view(view,vis)
if view then
view.visible = vis
end
end

---@param tab table
---@param idx integer
---@return table|integer
Copy link
Member

Choose a reason for hiding this comment

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

T|integer

library/lua/gui/widgets.lua Outdated Show resolved Hide resolved
@@ -1814,13 +2110,48 @@ ToggleHotkeyLabel.ATTRS{
-- List --
----------

---@class widgets.ListChoice
---@field text string
Copy link
Member

Choose a reason for hiding this comment

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

this can also be a list of text tokens, like Label

@myk002 myk002 merged commit a787baa into DFHack:develop May 17, 2024
14 checks passed
@vallode vallode deleted the widgets-types branch May 18, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants