FelixKratz / SbarLua

A Lua API for SketchyBar
GNU General Public License v3.0
151 stars 11 forks source link

Adding 'event' and 'trigger' to LUA Api #7

Closed Pix-xiP closed 10 months ago

Pix-xiP commented 10 months ago

Hi!

Love your work, would like to contribute if possible since I've been using SketchyBar and SketchyVim for a while now!

I'm not sure if more work needed to be done to make this work for graphs but seems to work for events I basically pulled the brew shell version from your dotfiles to test and it triggers!

local brew_event = sbar.add("event", "brew_update")
local brew = sbar.add("item", {
  -- some config
})

brew:subscribe("brew_update, some_func)

Would be happy to help with more things if you're open to more contributions

FelixKratz commented 10 months ago

Thanks for the PR. Contributions are always welcome!

I didn't yet add these kinds of items yet because they can/must receive additional input.

The event for example takes an optional argument which can be a DistributedNotification identifier as stated here: https://felixkratz.github.io/SketchyBar/config/events#creating-custom-events Additionally, it should not be possible to pass the event a config table in the add function. So it would probably be a good idea to handle the add call for the event explicitly (like I already did for the bracket to allow the optional additional argument). Then it is easier to check the arguments and print precise error messages.

The graph (and the slider...) take an additional integer in the regular sketchybar api, which determines the width of the graph/slider, so it would be nice to also handle that explicitly so we supply sketchybar with the correct setup information: https://felixkratz.github.io/SketchyBar/config/components#data-graph----draws-an-arbitrary-graph-into-the-bar

Pix-xiP commented 10 months ago

Okay! I'll skip doing the graph for the time being and work on event!

Break it out into its own explicit case, check that the lua_type is a string I assume for the event name.

And then check for an optional DistributedNotification and stack_push? - this might take me a moment I'm not super familiar with it so if you power through it in the meantime no stress otherwise I'll take a swing at it tomorrow evening :D

FelixKratz commented 10 months ago

Yeah exactly, you essentially only need to swap out the position argument which is hard coded in the item case with the distributed notification name if there is one, else do nothing. Take your time.

Pix-xiP commented 10 months ago

I'm not 100% sure if I've nailed the DistributedNotification part from the docs, I've got nothing that uses it in my current bar that I use but assume it works as such..

If you wanna take a look when you have a free moment and let me know if I've missed anything crucial!

Pix-xiP commented 10 months ago

While I was feeling productive, I added a way to trigger the event from inside of lua as well:

function M:brew_update(env)
    print("BREW UPDATE: ", env.PIX)
end

function M:brew_click()
    sbar.trigger("brew_update", { PIX = "working" })
end

M.brew:subscribe("brew_update", function(env)
    print("Event has been triggered: ", env.PIX)
    M:brew_update(env)
end)
FelixKratz commented 10 months ago

Works like a charm. Thanks for the improvements!