support negative list indeces #245
Conversation
so e.g.
{{ (split_me|split('-'))[-2] }}
works instead of
{% set parts = split_me.split('-') %}{{ parts[parts|length - 2] }}"
Codecov Report
@@ Coverage Diff @@
## master #245 +/- ##
============================================
- Coverage 71.84% 71.78% -0.07%
- Complexity 1396 1400 +4
============================================
Files 220 220
Lines 4383 4402 +19
Branches 693 699 +6
============================================
+ Hits 3149 3160 +11
- Misses 996 1002 +6
- Partials 238 240 +2
Continue to review full report at Codecov.
|
|
It seems easier to first check if the value is resolvable to a list. If so, call |
|
I was trying to tread as lightly as possible -- to leave as much to the superclass as possible. If you're OK with totally taking over, which is what I think you're suggesting, I'll give it a shot. I could use a hand coming up with test cases that cover the non-string parts of toIndex. EDIT: Aaah, I think I see what you're saying....and it still leaves the superclass doing much of the work. |
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException("Cannot parse list index: " + property); | ||
| } | ||
| } else if (property instanceof Character) { |
There was a problem hiding this comment.
I had a comment here that this seems a little too accommodating and if someone is passing a char here, they probably have a bug somewhere else or they really don't know how to use list indexes :)
But, it looks like this just copied from juel. Crazy.
| } catch (IllegalArgumentException e) { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** |
|
@mattcoley like this? 7d79492 |
| @@ -30,12 +31,109 @@ public boolean isReadOnly(ELContext context, Object base, Object property) { | |||
| @Override | |||
| public Object getValue(ELContext context, Object base, Object property) { | |||
| try { | |||
| return super.getValue(context, base, property); | |||
| final Object superValue = super.getValue(context, base, property); | |||
There was a problem hiding this comment.
nit: final within method not needed.
| // Leave the range checking to the superclass. | ||
| index += list.size(); | ||
| } | ||
| return super.getValue(context, base, index); |
There was a problem hiding this comment.
we can just set property = index + list.size() and just have one super.getValue() outside the if
There was a problem hiding this comment.
Sorry, yes. Object can be anything.
|
Yea that approach is a bit cleaner. |
| List<?> list = (List<?>) base; | ||
| if (index < 0) { | ||
| // Leave the range checking to the superclass. | ||
| property = index + list.size(); |
|
Thanks much. Would love to see this merged and a new version released. |
|
Thanks for merging! |
so e.g.
{{ (split_me|split('-'))[-2] }}
works instead of
{% set parts = split_me|split('-') %}{{ parts[parts|length - 2] }}"