$substring length parameter no longer works with negative start positions

[deepilla]

I've just noticed that the $substring() function isn't handling its length argument correctly when the start argument is negative. For example, this expression correctly evaluates to "world":

$substring("Hello world", -5, 5)

But so do all of these expressions:

$substring("Hello world", -5, 4)    // should return "worl"

$substring("Hello world", -5, 3)    // should return "wor"

$substring("Hello world", -5, 2)    // should return "wo"

$substring("Hello world", -5, 1)    // should return "w"

$substring("Hello world", -5, 0)    // should return ""

$substring("Hello world", -5, -1)   // should return ""

I'm guessing that this bug was introduced in the recent fix for UTF-16 surrogate pairs.

[deepilla]

There's a bit more to this issue than I first thought...

The previous version of $substring used JavaScript's substr function. In order to reproduce that behaviour, there are a couple of edge cases that need to be handled (in addition to fixing the bug mentioned above).

1. Negative length values should be treated as zero, i.e. $substring should return an empty string whenever length <= 0.

$substring("Hello world", 0, -6)    // should return "" (currently returns "Hello")

2. Negative start values that go beyond the start of the string should be treated as zero.

$substring("Hello world", -100, 3)  // should return "Hel"

[brodo]

Just so that I understand correctly: The current behavior breaks backwards compatibility?

[deepilla]

Well, the current behaviour isn't the same as the old behaviour. That said, I'm not sure whether the old behaviour was intentional or just a side effect of using substr. Certainly it wasn't being enforced by the tests.

Maybe negative length values shouldn't return an empty string. Maybe they should generate an error. That might be more consistent with other string functions that have a limit argument such as $match, $split, and $replace.