FelixKratz / SbarLua

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

Adding 'graph' to the Lua API. #8

Closed Pix-xiP closed 9 months ago

Pix-xiP commented 10 months ago

Hey again!

Finally had some time to take another swing at the original graph - addressing: #6

I've split it into its own if to be able to give it more concise error messages - I also added a way to translate the LUA_T to human readable text that I thought might be more useful in the error message to tell whoever is using the API exactly what part they might have goofed.

I think my current code forces a user to give the graph an explicit name - I'm unsure if you wanna do it that way, figured I'd open the PR and we could discuss if you want to change the approach etc.

Should be able to do this - using your helper c program as an example:

local cpu_sys = sbar.add("graph", "cpu.sys", 75, {
    position = "right",
    width = 0,
    graph = {
        color = colours.rose_pallete.love,
        fill_color = colours.rose_pallete.love,
    },
    background = {
        height = 30,
        drawing = true,
        color = colours.basics.transparent,
    },
})

I haven't completely cleaned the stuff I was using to dev, so if you're happy with this, I'll make another commit to remove all my printing stuff and comments!

FelixKratz commented 10 months ago

Looks good so far, I think what we need for the graph to be complete is to add the --push domain, where we can push data points to the graph: https://felixkratz.github.io/SketchyBar/config/components#data-graph----draws-an-arbitrary-graph-into-the-bar That would make sense to include as a member in the item table such that we can do someting like

cpu_sys:push({0.1, 0.2, 0.4})

or equivalently

sbar.push(cpu_sys, {0.1, 0.2, 0.4})

I think giving a name to the graph should stay optional, as it is possible to entirely handle the graph in lua once the push field is available and then the name will not be needed. As soon as the graph is populated by another process, e.g. the cpu helper process, a name is indeed needed. The way to go about this is to check wether the second argument is a string or an integer and then branch to the appropriate logic where if the second argument is a number already, we auto-generate the name of the item and if it is a string, we take it as the name of the item. Of course, the property list will be the optional third argument instead of the fourth argument if the name is omitted.

If you need help with this I am available for assistance. Thanks again for your work on this, it is very welcome.

Pix-xiP commented 9 months ago

Okay with those changes graph names are now optional once again

And I've tested both methods of pushing to a graph ( .push({}) and :push({})) which seem to work with and without a name so its all controllable from the lua if required.

On the topic of that helper, is there a good way to start it from the lua? I was originally using it for testing but had to run it manually from terminal as I don't think os.excute will run it in the background even with a &.

Thanks for letting me work on this, has been a great learning experience :D

Pix-xiP commented 9 months ago

Sorry about that, I autopilot committed cause it was late x(

I've removed the binaries, the run script, my additional header for dev logging. I merged your changes in to that merge. You may wanna give it a once over just in case :>

Sorry for the trouble again!

FelixKratz commented 9 months ago

Thanks for the contribution!

Pix-xiP commented 9 months ago

Is there anything else that needs to be worked on / added?

I was enjoying just spending burning some spare time on extending it.

FelixKratz commented 9 months ago

The slider component also receives an additional width argument, just like the graph: https://felixkratz.github.io/SketchyBar/config/components#slider----a-draggable-progression-indicator It currently is handled like all the other items because it does not throw any errors, however, I would like to map the api of sketchybar more closely with this item type as well. So if you want you could make the sliders available properly in lua.

Another idea that I was playing with is to add some kind of package manager system, e.g. like Lazy in nvim, where it would be easier to install external packages. Those packages could include compiled .so libraries like this one here (e.g. I have a module for communication and handling of yabai in lua) and lua files which could contain helper functions and preconfigured plugins, which would be configurable through a setup function call in the package manager.

I was thinking something like this:

local yabai = sbar.package("FelixKratz/YabaiLua")
local current_window = yabai.m("query windows --window")

local battery = sbar.package("FelixKratz/battery.sbar")
battery.setup({
  icon = "..."
})

where the package manager would handle the downloading, installing and loading of the plugin modules automatically from the specified github repo. I already have a fully working yabai module for example that I am currently using privately. But I think this would be something that I would like to design myself.

Pix-xiP commented 9 months ago

I like the package manager idea, I think that could be very cool. Given you're thinking compiled .so libraries I assume the packages would be written in C / any language that can output an .so file with a C ABI?

I'll take a swing at the slider in the meantime so I have something to tweak.

Happy to help with the package manager if you want it, or to extend it later once you've got a set framework designed :>

FelixKratz commented 9 months ago

The package manager would be able to load all kinds of modules so compiled .so libraries and regular lua files as well. But I think most useful will be the C based .so files to make system interaction easier. E.g. the CPU helper for the graph could be such plugin, simply providing the cpu load info in a lightweight way.

I will try to create a minimal proof of concept architecture for this sometime soon.