Last Comment Bug 371932 - Implement ES4's /y option on regular expressions ("sticky" -- start at lastIndex) (js1_7/regexp/yflag.js)
: Implement ES4's /y option on regular expressions ("sticky" -- start at lastIn...
Status: VERIFIED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla1.9alpha3
Assigned To: Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017)
:
: Jason Orendorff [:jorendorff]
Mentors:
http://developer.mozilla.org/es4/prop...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-27 05:54 PST by Axel Hecht [:Pike]
Modified: 2008-01-17 20:23 PST (History)
9 users (show)
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement /y, v1 (9.74 KB, patch)
2007-02-27 21:47 PST, Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017)
crowderbt: review+
Details | Diff | Splinter Review
Implement /y, v2 (11.70 KB, patch)
2007-02-28 09:12 PST, Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017)
brendan: review+
crowderbt: superreview+
Details | Diff | Splinter Review

Description Axel Hecht [:Pike] 2007-02-27 05:54:03 PST
Writing a parser in js for my pet file format, I use regular expressions to do the analysis of the input string.

It'd be nice if .test and .exec would actually provide a start (and maybe even an end) offset, because right now, I'd either have to provide some ^[.\n]{n} to skip the first n chars I already parsed, or chop of each parsed string fragment at the start of the string, both of which look happily inefficient.

That'd be similar to python's match and search params, http://docs.python.org/lib/re-objects.html.

Do we need to have a .match equivalent to specify what '^foo' is supposed to match in that case? I'm currently using the chopping off alternative, with a BOL for each regexp, using .match with an offset would be the API I really need for parsing.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2007-02-27 06:07:41 PST
http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Global_Objects:RegExp#Properties

See |lastIndex|, and note that it's present when the RegExp includes the /g flag.
Comment 2 Brendan Eich [:brendan] 2007-02-27 06:50:22 PST
The ES4/JS2 regexp work includes a /y flag ("sticky") that starts at lastIndex and either matches there without searching forward, or fails.

Given /y, I don't see a need for anything more. What am I missin?

/be
Comment 3 Axel Hecht [:Pike] 2007-02-27 07:42:22 PST
And yes, the /y flag is exactly what I need, together with lastIndex.

I didn't manage to find http://developer.mozilla.org/es4/proposals/extend_regexps.html before, too. I recall that we don't intend to enhance JS for 1.9, right?

I'm kinda torn if I should see myself writing the l20n parser in C/C++ or keep it in js, so if we could support /y early, that'd make it easier to not port the parser to C/C++ and still evaluate the performance impact of it. At least without having obvious computational complexity bugs in it.
Comment 4 Brendan Eich [:brendan] 2007-02-27 07:51:44 PST
We should do /y and the other ES4 regexp extensions in 1.9.  Anyone up for it?

/be
Comment 5 Bob Clary [:bc:] 2007-02-27 09:34:53 PST
The Perl regular expression tests use @- (array of captures starting indices) and @+ (array of captures ending indices). Something similar would be useful in that context.
Comment 6 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2007-02-27 21:47:54 PST
Created attachment 256762 [details] [diff] [review]
Implement /y, v1

I looked, and it seemed like /y was pretty easy to implement quickly, so here's a first go at it.

Some of the other extensions (namely /x) are going to require a bunch more work.
Comment 7 Brendan Eich [:brendan] 2007-02-28 03:03:32 PST
Comment on attachment 256762 [details] [diff] [review]
Implement /y, v1

Looks ok at a glance, I'm kind of jetlagged still. But why do you mix all metaphors (NOSEARCH, anchored) except the new, mnemonic one (stickY)?

Brian should like this too, or in place of my r+ (I'll look again later today).

/be
Comment 8 Brian Crowder 2007-02-28 08:36:45 PST
Comment on attachment 256762 [details] [diff] [review]
Implement /y, v1

This looks good to me.
Comment 9 Brian Crowder 2007-02-28 08:51:25 PST
I should add that I agree with brendan's feedback on the use of "nosearch" and "anchored".  StickY is better.
Comment 10 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2007-02-28 08:59:27 PST
(In reply to comment #7)
> (From update of attachment 256762 [details] [diff] [review])
> Looks ok at a glance, I'm kind of jetlagged still. But why do you mix all
> metaphors (NOSEARCH, anchored) except the new, mnemonic one (stickY)?

At first, I wasn't thinking and used "anchored", then I saw on the page you linked to that the property that described this feature was called "noSearch". I now understand that the developer.mozilla.org page is out-of-date. New patch in a jiffy.
Comment 11 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2007-02-28 09:12:10 PST
Created attachment 256802 [details] [diff] [review]
Implement /y, v2

This patch doesn't mix its metaphors quite so much.
Comment 12 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2007-02-28 09:22:23 PST
Comment on attachment 256802 [details] [diff] [review]
Implement /y, v2

>-MSG_DEF(JSMSG_NO_INPUT,                59, 3, JSEXN_SYNTAXERR, "no input for /{0}/{1}{2}")
>+MSG_DEF(JSMSG_NO_INPUT,                59, 4, JSEXN_SYNTAXERR, "no input for /{0}/{1}{2}{3}")

Note that 4 parameters aren't enough. I had to change the 4 to a 5 and add a {4} to the end of the error string, but I don't think that's worth a new patch. Currently, we don't append the multiline flag to this error message (which is what confused me).
Comment 13 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2007-03-11 12:39:22 PDT
I checked this into the trunk (with crowder's review). I'll address any comments that brendan has ex-post facto.
Comment 14 Brendan Eich [:brendan] 2007-03-19 17:14:35 PDT
Comment on attachment 256802 [details] [diff] [review]
Implement /y, v2

Looks ok to me. Tests in the suite yet?

/be
Comment 15 Bob Clary [:bc:] 2007-03-19 18:14:02 PDT
(In reply to comment #14)

not yet.
Comment 16 Bob Clary [:bc:] 2007-03-20 01:18:41 PDT
/cvsroot/mozilla/js/tests/js1_7/regexp/yflag.js,v  <--  yflag.js
initial revision: 1.1

/cvsroot/mozilla/js/tests/js1_7/regexp/shell.js,v  <--  shell.js
initial revision: 1.1

/cvsroot/mozilla/js/tests/js1_7/regexp/browser.js,v  <--  browser.js
initial revision: 1.1
Comment 17 Bob Clary [:bc:] 2007-03-21 15:33:51 PDT
verified fixed 1.9.0 20070320 win/mac*/linux
Comment 19 Jeff Walden [:Waldo] (remove +bmo to email) 2008-01-17 20:23:14 PST
(In reply to comment #18)
http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Global_Objects:RegExp

I added an example of how to use /y and expanded the flag's documentation, because I couldn't understand how it worked just from reading it, even after partially following its implementation/functionality here.  It'd be nice if people could double-check that my example is a reasonably idiomatic use case.  :-)


http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Guide:Regular_Expressions

Maybe I'm blind, but I only see g/i/m on this page at a glance, and a search for 'flags' on the page doesn't mention it either.  I don't really have time to write the necessary prose here, or I'd fix it myself.

Note You need to log in before you can comment on or make changes to this bug.