Simplify regexp literal

[bkruczyk]

No description provided.

[josteink]

There's always weird edge-cases when parsing is involved.

That said, the code in question here has been unchanged since the initial commit, with none of the current maintainers altering it.

For me at least, that means I don’t know any specific reasons to keep it as is, in its current form.

I’d be happy to accept this PR if you can:

1. Prove it fixes a bug and
2. Don’t break anything else.

And the best way to do that would be adding some test-cases to the test-suite for regexp-parsing.

Take a look at the existing tests and let us know if you need any help.

[bkruczyk]

@josteink thanks for feedback, maybe we can discuss test cases when I will update the PR, need a day or two

[josteink]

Sure thing. Take your time.

[bkruczyk]

If anyone have anything to add on added tests, you're welcome to leave a comment :)

[josteink]

Thanks for the update!

When it comes to unit-tests I usually check one thing before considering myself "happy": I ensure that the test prove the error-condition by failing before the appropriate fix is applied.

In this case your test are equally good before and after your fix is applied, meaning they are not sufficient to reproduce the error you say you want fixed.

If you could you address that (that is, make the tests fail without your fix in place), I'll consider this good and will be happy to merge!

[bkruczyk]

That's a great tip on testing "fixes", I'll incorporate it in my daily work, thanks!
I've updated the PR.

[josteink]

Now that's more like it! Now the tests behave as tests should :)

And given my promise just one comment away, sorry about being a bit thick-headed here today (got a cold and all), but had the impression that this PR was meant to fix either #34 or #44.

As far as I can see from my interactive testing that's not the case though. Or is there something else I have misunderstood? Was this PR mostly about simplifying existing code?

If I try the testcase from #34:

test.replace(/\/g,"/");
this_identified_is_still_a_string

Second line is still highlighted as a string somehow. Is there something wrong with my test-setup perhaps?

Again: Sorry for being thick-headed about all this when you're obviously putting in a good effort just trying to full-fill my requests.

Edit: If nothing else, we can at least be happy Github too has the fontification of this expression wrong :)

[bkruczyk]

Yup I intended for this PR to be a fix to #34.

I think the highlighting is correct, please check this comment :)

[josteink]

Ah gotcha. My bad.

Thanks for your contribution and consider this merged!