-
Notifications
You must be signed in to change notification settings - Fork 13
RegExp Match Indices Stage-3 performance mitigation #47
Comments
Given that RegExp match indices is at Stage 3, (1) is the simplest and cheapest solution that doesn't require an overhaul of the entire specification. The flag would be easily accessible to
To me (2) and (3) seem like a non-starters due to the complexity of the change as well as the related subclassing hazards. (4) also seems acceptable to me, if unfortunate, as I would gladly pay the penalty for re-execution of the RegExp to leverage a capability that doesn't currently exist. I'm option to other options (6) if there are any that make sense given the current stage of the proposal. If we choose to go with (1), we still need to bikeshed the flag. Some I've discussed this with are unhappy with the idea of an upper-case RegExp flag, so If I were to use ranked-choice voting, my top 3 would be:
|
Using |
"d" and "o" flags both already exist for Perl regular expressions, although even their own documentation denigrates them severely: https://perldoc.perl.org/perlre#Modifiers . None of the flag options seem great to me, though. I would personally be fine with the stay-the-course (4), and unhappy with the symmetry-breaking (2) and (3). |
Given that Perl perl documentation denigrates both the Option (2) has ergonomic issues for some fonts and (3) would eliminate the use in RegExp literals. The (4) stay-the-course will likely hinder the uptake of this feature. |
@ljharb: @msaboff: A native implementation of (4) is likely still faster than a polyfill version of (4) like in https://github.com/rbuckton/regexp-match-indices, and better than the alternative of not having the feature when you need it. Option (4) at least provides a path for the possibility of a future where (5) might be possible without deprecating a flag. Overall though, I'm leaning towards (1) using the |
I assume you mean Option (1) using |
@msaboff: Yes, I am talking about (1) using the |
I've created #49 as a PR against this proposal to get a better idea of what kind of changes we would need to make in the specification to use Option (1) with a |
Options:
.indices
to the result:I
to indicate indices (upper-caseI
does not conflict with lower-casei
).d
since bothi
is already used andn
means something else in other RegExp engines.n
at a future meeting.o
to indicate offsets, and rename the.indices
property to.offsets
.RegExp
, such as.execIndices
.String.prototype.match
orString.prototype.matchAll
unless we extendString
as well.matchIndices
andmatchIndicesAll
toString
, we further complicateRegExp
subclassing by either needinga
@@matchIndices
and@@matchIndicesAll
or needing to extend the existing@@match
and@@matchAll
.@@match
and@@matchAll
.RegExp
twice.The text was updated successfully, but these errors were encountered: