After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 685885 - javascript: shouldn't find a regexp in [thing / thing, thing / thing]
javascript: shouldn't find a regexp in [thing / thing, thing / thing]
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: Syntax files
3.6.x
Other Linux
: Normal normal
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2012-10-10 13:18 UTC by Arnaud B.
Modified: 2015-04-15 08:28 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
require special character after regex (1.01 KB, patch)
2015-04-05 12:07 UTC, Andreas Fackler 		accepted-commit_now Details | Review
an example file for smoke testing (906 bytes, patch)
2015-04-13 15:35 UTC, Andreas Fackler 		accepted-commit_now Details | Review

Description Arnaud B. 2012-10-10 13:18:53 UTC 
real case: 
  cr.translate(area_width / 2.0, area_height / 2.0)
test case:
  a = [thing / thing, thing / thing]

In both cases, the text between the two slashs (including them) is in pink in Gedit, for what I understand as if it was a regexp.
Comment 1 Ignacio Casal Quinteiro (nacho) 2012-10-10 13:25:47 UTC 
Patches welcomed against the javascript.lang http://git.gnome.org/browse/gtksourceview/tree/data/language-specs/javascript.lang
Comment 2 Andreas Fackler 2015-04-05 12:07:16 UTC 
Created attachment 300985 [details] [review]
require special character after regex

If I read my javascript.vim correctly, vim requires one of the characters ";.,)]}" after a regex, fixing this issue, so maybe GtkSourceView should do the same.
I also added ":" (for ternary operator) and "/" (for comments) in this patch.

Does that make sense? I'm not 100% sure that a regex can never be followed by other characters.

Also, there's still other false positives, like in this example from the ECMA specification:

a = b
/hi/g.exec(c).map(d);
Comment 3 Sébastien Wilmet 2015-04-13 13:52:37 UTC 
Comment on attachment 300985 [details] [review]
require special character after regex

>           \%{regex-opts}
>-          ([),;.]|\s|$)
>+          \s*
>+          ([),;.\/\]:}]|$)

What fixes the bug is the \s*, not the added characters. Are you sure the added characters are needed?

Can you add a JavaScript example file in the tests/syntax-highlighting/ directory? So you can provide an example for each possible character following a regex.
Comment 4 Andreas Fackler 2015-04-13 15:35:58 UTC 
Created attachment 301467 [details] [review]
an example file for smoke testing

Examples for these characters.

I didn't only add the \s* but I also removed the \s from the bracket expression following the \s*. Before that change, any whitespace following a slash was sufficient to qualify it as closing a regex. Now that specific special characters like ),;. are actually required, as was probably originally intended, I wanted to add the remaining ones, to avoid introducing new false negatives.
Comment 5 Sébastien Wilmet 2015-04-14 08:05:16 UTC 
Review of attachment 301467 [details] [review]:

Thanks, I'm not an expert in JS, but it looks good.
Comment 6 Sébastien Wilmet 2015-04-14 08:07:07 UTC 
Review of attachment 300985 [details] [review]:

> I didn't only add the \s* but I also removed the \s from the bracket expression
> following the \s*.

Oh, right. So I guess the commit is fine.