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

[fs-widget] Implement an alternative config pattern #277

Conversation

Aire-One
Copy link
Contributor

@Aire-One Aire-One commented Jun 20, 2021

Hello,

(This PR targets your 274-make-beatiful-optional branch)

I didn't have time to do this earlier, but at least, here it is: my take on making beautiful not a hard dependency for the fs-widget. (Also, please note this is untested)

My solution is actually quite easy:

You already have defined the (module level) global config table that holds all the available config properties and their default values. I trusted this table to set the local widget instance configuration table.
In the widget instance constructor, I add a _config table that holds the local version of the global config table. The local _config can be blindly used by the widget instance as "its own definition of configuration" ([1]).
I then replaced all the usage of the config table by the instance level _config table.

The main idea here is that the instance level _config table will be a copy of the global config table. With the exception, it's also checking for the first available value from args, beautiful, then fallback to the default.
(A notable limitation for this is that it will not work with nested table!)

Note: In the Awesome source code, we generally don't do a full copy of the default config table. Instead, we do the args or beautiful or default check every time we need to access the config value. We could have a debate on "what's the best approach: code readability/performance with a local config or save memory by doing the check". I personally think both approaches are equivalent at the end of the day...

[1]: Another approach with more complex widget would be to have something like self._private.config and some meta-table methods to properly implement getters and setters for these properties.

CC: @raven2cz

@streetturtle
Copy link
Owner

I really like this way! I'll test it and merge next week. Then will apply this approach on all the widgets.
Thank you!

@streetturtle streetturtle merged commit 4859e55 into streetturtle:274-make-beautiful-optional Jun 28, 2021
@Aire-One Aire-One deleted the 274-make-beautiful-optional branch June 28, 2021 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants