Skip to content

Twig filter#2124

Merged
jarednova merged 9 commits intotimber:masterfrom
palmiak:twig_filter
Jan 1, 2020
Merged

Twig filter#2124
jarednova merged 9 commits intotimber:masterfrom
palmiak:twig_filter

Conversation

@palmiak
Copy link
Copy Markdown
Collaborator

@palmiak palmiak commented Dec 19, 2019

Ticket: #2010

Issue

Twig and Timber have filter called filter and now we have a problem.

Solution

Adds new filter called wp_list_filter and adds backward compatibility and warning to normal filter.

Impact

User's that use the filter and with string or array as second param will get a warning.

Usage Changes

The twig filter works a bit different - https://twig.symfony.com/doc/3.x/filters/filter.html.

Considerations

Testing

Not yet

@palmiak palmiak requested a review from gchtr December 19, 2019 11:13
@palmiak palmiak requested a review from jarednova as a code owner December 19, 2019 11:13
@palmiak palmiak added the wip Work in Progress label Dec 19, 2019
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.5%) to 93.326% when pulling 16408f9 on palmiak:twig_filter into 5c22f62 on timber:master.

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 19, 2019

Coverage Status

Coverage decreased (-0.05%) to 93.809% when pulling 27fd694 on palmiak:twig_filter into 3eaddbc on timber:master.

Copy link
Copy Markdown
Member

@jarednova jarednova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@palmiak — this looks great! It'll make it easy to bring into 2.x. My only Q is the naming of the new function, see my comment

lib/Helper.php Outdated
*/
public static function filter_array( $list, $args, $operator = 'AND' ) {
if ( ! is_array($args) ) {
public static function wp_filter_array( $list, $args, $operator = 'AND' ) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change this to wp_list_filter to match the Twig filter name? One less name to manage (and confuse me)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea - pushing the fix in a moment.

@jarednova
Copy link
Copy Markdown
Member

I'm going to close my associated PRs in favor of this one

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 24, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@fddbe49). Click here to learn what that means.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2124   +/-   ##
=========================================
  Coverage          ?   95.04%           
  Complexity        ?     1582           
=========================================
  Files             ?       50           
  Lines             ?     3753           
  Branches          ?        0           
=========================================
  Hits              ?     3567           
  Misses            ?      186           
  Partials          ?        0
Impacted Files Coverage Δ Complexity Δ
lib/Twig.php 99.47% <100%> (ø) 85 <0> (?)
lib/Helper.php 96.52% <66.66%> (ø) 76 <4> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fddbe49...27fd694. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wip Work in Progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants