[Webkit-unassigned] [Bug 51021] Add JSRegExp.h to YARR

Thu Dec 23 17:19:38 PST 2010

https://bugs.webkit.org/show_bug.cgi?id=51021

--- Comment #2 from Gavin Barraclough <barraclough at apple.com>  2010-12-23 17:19:38 PST ---
Hi Peter,

I can see a role for a new header here, and we may want to move some of the existing code, but I'm not sure this change is quite right.

(1) Moving the compile method irks me, because I think we should just delete this – it's only ever, and must always be, called on a new pattern – it should just be a part of pattern object construction! (and the pattern object only exists in order to compile the regexp).  The call to compile (i.e. populate the RegexPattern object) should just be a part of RegexPattern construction.

I hope you don't mind, but I think I'm going to fix this, which will conflict with this patch.

(2) The new file name seems a little inconsistent with convention (not that we're completely consistent unfortunately :-/ ).  We most commonly apply the JS prefix to garbage collected JS object types, for example JSObject, JSString, JSArray.  Yarr should ideally be usable as a standalone library – as pcre is, for uses other than within javascript (i.e., possibly used form WebCore, replacing current PCRE use?), and with no particular connection to JS other than the fact the engine implements the JS regex syntax.  As such, I think it may be best to avoid JS in the file names or interface.  Since the rest of the files in the directory currently use the name Regex, I'd suggest a new header containing common types could just be RegexTypes.h, or maybe just Regex.h.

However it's possible that it was a mistake to use the term Regex in all these class & filenames (since JS generally uses the term RegExp).  It might be less confusing to rename the existing files & classes, replacing "Regex" with "RegExp", or with "Yarr".  If we did so, then I imagine a new header would want to be "Yarr.h", "YarrTypes.h", or "RegexTypes.h"

cheers,
G.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.