Fix test generation placeholders inside and adjacent to RegExp literals#3817
Fix test generation placeholders inside and adjacent to RegExp literals#3817
Conversation
This fixes cases such as `/foo//*{ subst }*/` where previously the `//`
would be interpreted as a single-line comment. We now keep track in
find_comments() of when we are inside a RegExp literal.
However, this breaks cases that used to work (but not tested; maybe this
worked unintentionally?) of `/regexp /*{ subst }*//`.
See: tc39#3808
Co-authored-by: Guillaume Emont <guijemont@igalia.com>
For places where /* or */ would be misparsed (e.g. in RegExp literals) we add an escape hatch in the form of placeholder-prefix and placeholder-suffix keys that can be placed in the front matter of a test template file. These strings delimit placeholder comments, regardless of where they appear in the template file. Unlike /* */ placeholder comments, a placeholder comment delimited by these strings _must_ contain an identifier wrapped in curly braces. If it contains anything else, the file is invalid. This is to prevent accidentally using some valid JS syntax as the delimiters.
ptomato
force-pushed
the
3808-fix-test-generation-regexp
branch
from
bd4c562 to
a38d3cd
Compare
jugglinmike
left a comment
jugglinmike
left a comment
There was a problem hiding this comment.
The first commit looks like it unblocks gh-3807, so I'm all for it!
I'm less enthusiastic about the second commit, though. Addressing needs through configuration certainly makes the tool more powerful, but it also makes things more daunting for new contributors--even for those who have no need for it (presumably the majority of potential users). This implementation's distinct edge case (the restriction on interpolated values), if documented, will steepen the learning curve a little further still.
I'd be more inclined to collaborate on the design if there was a motivating use case. Without that, its hard for me to justify the complexity and to envision the scenarios where test authors would need this ability.
|
@jugglinmike I see your point, in fact I shared your skepticism about the configurable delimiters feature when Richard suggested it. This comment explains why I changed my mind. But like I said in the description, I'm not attached to the design of this feature, so I'd be happy to go with something completely different. But more immediately: the first commit actually doesn't unblock #3807 unless I add some more processing for comments inside regexp literals. (The first commit fixes |
2 participants
Several improvements to the test generation tool. The first commit is based on @guijemont's work and fixes the case where a placeholder comment is immediately adjacent to the end of a RegExp literal.
The second commit adds a feature to test template files which can be used as an escape hatch in case the
/*and*/delimiters of placeholder comments would be misparsed. (This is one way that it could work, I'm not attached to it.)Closes: #3808