| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
This is backwards compatible as if a TermClass is passed to the method it will take precedence over the property
update docblocks. also removes usage of _get_terms property, as this could return wrong results depending on arguments passed to the method
|
@aaemnnosttv this is great work. Thanks much for your work on this (and with tests!). I had a question on the TimberTerm::from('My Term Name') — can you provide an example of how/where you would use this in an example scenario (is this for usage in PHP or Twig or both?) thanks! |
Sorry, something went wrong.
|
Thanks Jared! I've really been digging Timber, so I'm happy to contribute. Regarding the named constructor, I actually use this in the tests. In addition to just being maybe a bit more expressive than using new, the static method makes it possible to use as a callback. Perhaps the best example might be using it as a callback for transforming an array of terms, into TimberTerms. // some function/method..
// get all non-empty terms for categories and tags
$terms = get_terms(['post_tag','category']);
return array_map('TimberTerm::from', $terms);Whereas without that, you would need to use a closure or something to map over that in order to new TimberTerm.. Tests are another reason (and probably the reason I thought to add it) where you can see above, we're able to convert all of those array items to TimberTerms without using a foreach, which would unnecessarily pollute the test method with variables that were really not necessary. It's just a little helper which can be used to help with readability, but also with reducing the amount of code needed to do certain things. :) |
Sorry, something went wrong.
|
Awesome, something I hadn't thought of before. Would it make sense to apply the same method to posts and users as well? |
Sorry, something went wrong.
Add support for TimberPost->terms( array ) & more
|
Yeah I think the same use case would apply to all of them. I definitely thought of extending that to those as well, but as I said, I wanted to keep this PR limited ;) I could put together another one if you're interested? |
Sorry, something went wrong.
|
@aaemnnosttv yes please! Thanks for making it a separate PR though, one of the toughest things can be wading into code that touches too many pieces at-once. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR addresses a few things (sorry about that) but I tried to limit the reach of the code changed to only that which is relevant. I think the commit messages give a pretty good summary, but here's the gist of it: