implement regex sticky support #1181
Conversation
|
Nice 👍 Will look over the changes coming week Could you link this PR to the matching issue? |
|
Looks good to me |
| @@ -1432,19 +1440,16 @@ built-ins/RegExp 925/1464 (63.18%) | |||
| prototype/source/value-empty.js | |||
| prototype/source/value-line-terminator.js | |||
| prototype/source/value-u.js | |||
| prototype/sticky 8/8 (100.0%) | |||
| prototype/sticky/cross-realm.js {unsupported: [cross-realm]} | |||
| prototype/sticky/length.js | |||
There was a problem hiding this comment.
BTW: this test + the next 2 fail because the 'sticky' property you added is of the wrong type.
Having said that, so are all the existing properties like multiline for example.
Not sure if we should continue as is, just do this new property properly (and fix the other ones later) or fix them all now
There was a problem hiding this comment.
@p-bakker what do you mean by wrong type?
Do you mean this disabled test?
org.mozilla.javascript.tests.NativeRegExpTest.flagsPropery()
There was a problem hiding this comment.
See #963: the global/multiline/ignoreCase properties and now the new sticky property ought to be accessor properties, but they are implemented as data properties
There was a problem hiding this comment.
Got no problem with that.
There was a problem hiding this comment.
Once you merge I'll rebase my Regex unicode flag WIP and create a draft PR for it (not 100% complete yet)
There was a problem hiding this comment.
Merged, fix for the proprties is also on the way
Closes #961