-
Notifications
You must be signed in to change notification settings - Fork 29
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
Issue/46 - Fix magic methods not working as intended #47
base: master
Are you sure you want to change the base?
Conversation
This commit introduces a private data() array, and a public method for classes that extend Base to call to setup the defaults. This is necessary to make sure that all default class variables make their way into the data array so that magic methods can access them like they can any other variable.
This commit is necessary to make sure that all object properties are correctly setup in the private data array.
The above upstream WordPress core change was merged into WordPress 5.5! |
So should we assume that anything using BerlinDB should be on 5.5 or greater? |
Once this is merged, yes. |
* Change from array to object * Introduce unset_vars() to parlay off of set_vars() * Introduce validate_key() to reduce some code duplication in magic methods * Switch from is_callable() to method_exists() so that it works as intended with __call() * Make sure magic un/set methods operate on the correct variables * Avoid setting object array keys directly, triggering byref debug notices * Some general code & docs improvements to related methods * Add 'join' to query & request clauses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just adding a note that it would be a good idea to add a note in the Readme that BerlinDB currently requires a minimum of WordPress 5.5. This may be a detriment to some plugins that want to be compatible further than that.
👍
It doesn't yet, but it will when #46 is resolved, which is in the Ongoing milestone for pretty much this reason. Based on the WordPress functions that Berlin uses, its minimum is probably 3.3 thanks to it calling That's a pretty big jump from 3.3 to 5.5, so I'm not super eager to merge this. 😬 |
Yep, totally understandable. |
This pull request changes quite a bit about how classes store their properties.
$this->data
arrayBase
have internal logic for allowingget_
functions to work as intended$this->foo
ends up in$this->data['foo']
so that it can always be magically overridden, or notAll of this is required to make overrides work as intended. But then, WordPress functions that use
WP_List_Util
will then stop working, because they usearray_key_exists()
which will bypass PHP magic methods completely.That means this PR either requires:
Fixes #46.
Related to: https://core.trac.wordpress.org/ticket/50095