Generalize support of negative string offsets #1431
Conversation
flaupretre
force-pushed
the
string_offsets
branch
2 times, most recently
from
d0713ee
to
96ef4b6
Compare
flaupretre
force-pushed
the
string_offsets
branch
2 times, most recently
from
6006471
to
4ee363d
Compare
|
Hi @flaupretre! What is the status of this PR? Thanks! |
|
@raoulvdberge I'm working on another project these days, but I think it will be on time for inclusion in 7.1. Anyway, help is always welcome (see todo above). |
|
Even though the PR is incomplete in terms of |
|
@jerrygrey It is too late for 7.0.0. Maybe 7.0.1 but I think it won't be considered as important enough to be introduced in a patch release. That's why I'm targeting 7.1. Feel free to start a discussion on the mailing list if you think it should be merged before. |
|
Hi @flaupretre I consider this more a "feature", and since this touches the core language, I don't think it is something we should add in a patch release. So the most optimal target where you can find the most support is indeed 7.1. On a personal note, I love this feature as I write a lot of small parsers and state machines where I work on string offsets in one way or another, so I only welcome this improvement into PHP. |
|
@flaupretre Yeah, 7.1 seems to be the ideal release. It is just a shame this feature didn't make it into 7.0. |
|
@flaupretre "It's a shame" is a saying, meaning it is unfortunate, not to be ashamed, and I don't mean you or anyone else should be ashamed. I also don't know C very well to help out in that way, but I do know PHP very well (I still remember writing my first scripts at age 10, a good 15 years ago now, PHP4 was big back then) and I help out where I can like reporting bugs and helping out with the test scripts. |
flaupretre
force-pushed
the
string_offsets
branch
4 times, most recently
from
c589eb0
to
f6abd06
Compare
flaupretre
force-pushed
the
string_offsets
branch
3 times, most recently
from
c402ca6
to
4bd8f1a
Compare
flaupretre
changed the title
|
RFC is 'under discussion'. |
|
@flaupretre Yay 🎉 I really hope it doesn't get voted down though. |
flaupretre
force-pushed
the
string_offsets
branch
from
a031289
to
4bd8f1a
Compare
|
Starting 2-week RFC voting period. Vote ends Monday, March 7th 00:00 UTC. |
|
@flaupretre Awesome! Thank you for your work on this! 👍 |
|
Vote is over. RFC is approved (28 Yes, 0 No). Will now be merged in the master (7.1) branch. Thanks to all. |
| @@ -4,7 +4,7 @@ Test iconv_strpos() function : usage variations - pass different data types as $ | |||
| <?php | |||
| extension_loaded('iconv') or die('skip'); | |||
| function_exists('iconv_strpos') or die("skip iconv_strpos() is not available in this build"); | |||
| if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only"); | |||
| if (PHP_INT_SIZE != 8) die("skip this test is for 32bit platform only"); | |||
There was a problem hiding this comment.
This change doesn't looks right (and a number of similar ones).
Negative string offsets are counted from the end of the container string.
…arameters. following the 'substr()' model.
Also add missing test for this function
flaupretre
force-pushed
the
string_offsets
branch
from
3a13507
to
adf0e7d
Compare
|
Just rebased PR on latest master to solve conflict with #1761 |
|
This PR is now merged via range aa2cf3a...f65486f. I've squashed some of the changes where it seemed reasonable. Thanks for all your work on this! |
This PR implements the changes proposed in the 'Generalize support of negative string offsets' RFC.
Please refer to the RFC for more information.