tablerow: Avoid accidental special case for constant nil cols #1644
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
#1633 recently introduced a weird special case behaviour for a nil literal for the tablerow columns, which differs from an expression that evaluates to
nil
. This is because@attributes['cols'].nil?
would match anil
literal expression, but wouldn't match an expression that dynamically evaluates tonil
.The included regression test demonstrates the problem, where
{% tablerow i in (1..2) cols:nil %}{{ tablerowloop.col_last }}{% endtablerow %}
doesn't produces the same output as{% tablerow i in (1..2) cols:var %}{{ tablerowloop.col_last }}{% endtablerow %}
whenvar
isnil
.Solution
It seems like the intended behaviour is to default 'cols' to the length of the slice being iterated over, as
@attributes.key?
does for the 'offset' and 'limit' attributes. So I updated the expression to default 'cols' to also use@attributes.key?
.