-
-
Notifications
You must be signed in to change notification settings - Fork 705
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
Lua config #2959
base: master
Are you sure you want to change the base?
Lua config #2959
Conversation
This is very cool, thanks a lot for all the work you put into this! Unfortunately I can't get it to compile yet, so I can't test it out for myself. Error
I do have some thoughts on your TODO list, but many of them can wait for later:
In this PR we should try to get a minimal viable product (MVP) for lua configs, where we define a robust config API that fits both lua and INI I haven't gone into the code too much yet since it seems there will be some bigger changes yet to come. Let me know when/if you need any additional feedback :) |
Your compilation error should now be fixed.
I tried that before the first commits but it looks like you can't derive templated functions, thus I came up with manual routing. |
Nice! I'll try it out tomorrow
That's a bummer. What if the subclasses only provide the primitive get functionality that returns a string and the base class has all the (non-virtual) templated methods that handle the conversion? |
Eagerly waiting for your feedback on loading the modules you're used to use.
That would be an option but I find it's kind of shame to use a string API when we can directly read the right type from lua. Luwra handles getting If we want to leverage lua typing and move more polybar types into lua typing, we need find a better way than a string API.
These refactoring would need to happen in another PR not to clutter this one. |
Sorry, I haven't found the time to try it out yet. It does seem to compile though :) We should definitely let our users use lua's data types other than strings for their config values. However, I believe that reading lua's different primitive values as strings and then using Otherwise, the primitive get method would need to return some type of object that represents one out of many types. The code overhead to do this and then pass that object to the That being said, I do think implementing this will be necessary at some point, when we want values on the lua side to get more complex. For example, I imagine that at some point, values like "percentage with offset" can be represented as tables in lua: Let's defer this design decision on how exactly to do this until we actually have to deal with such more complex values. What do you think? I do have some ideas though. Just writing them down because I already racked my brain over this:
I really wish C had proper algebraic data types 😞 |
Alright, I'm testing it out right now and I'm getting really excited about this. It is still quite easy to crash polybar though 😉
These are probably all quite minor issues. The more fundamental issue I have found concerns lists. The current approach works great if we just want to load a list of value (
Not sure how we want to reconcile this in the current implementation. Any ideas? |
Since C 17 is available now, I'm experimenting with another lua wrapper library (sol2) which might make error handling better |
Your points on not supplying the bar name and bar background list or string should be addressed. Concerning the ramps/weight, the object they contain is quite complex:
I think one option is to define all entries
So, I think reading ramps configuration must be changed to use the We are kind of back to the problem of the #2515 where maps can't be read from the config file with the ini API. Adding the functions The parsing of a label would be the first client of these new functions. This might also apply to animations/framerate though I didn't look at them. |
I've been away last week, so I couldn't look at this yet. For the library, I'm fine with anything that has robust maintenance. I just don't to have a situation like I think in the end, the config API will definitely have some way to read an entire map from the config. The question is whether we should implement this in this initial version or not. If not, everything that is reading a list that may have additional properties cannot use Otherwise, feel free to go ahead and implement this as you like. I'm assuming the I have been doing some thinking around returning different types in the config API. For example, reading something like Now the question is how would we be able to retrieve something like My goal with this is to hide as much complexity as possible from the config API clients, behind the scenes, the config API does all the work. |
I just came up with the same solution but named it
With this API, retrieving My next moves here will be more refactoring of
|
Hey, sorry for the communication blackout. Just wanted to let you know that I'm having a busy few weeks, hopefully things slow down in a week or two. Until then I can't promise any in depth reviews unfortunately. I did see your config tests PR though. I appreciate it, that should really help us in keeping the same user-facing semantics when it comes to reading complex config values. |
I totally understand that, thank for letting me know |
What type of PR is this? (check all applicable)
Description
This is the first draft of adding lua configuration file to polybar.
It uses the luwra lib which BSD-3 license seems to be compatible with polybar license.
Lua 5.4 is not embedded yet and is expected to be installed on the building and running machine.
Polybar starts with the
doc/config.lua
file on thebar_test
barThere is still lot of work to do, at least at the top of my head:
Related Issues & Documents
Documentation (check all applicable)