[Webkit-unassigned] [Bug 128422] Web Inspector: update check-webkit-style to flag single quotes in WebInspectorUI projects

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Diego Pino <dpino at igalia.com>  2014-02-19 05:33:22 PST ---
(In reply to comment #12)
> (From update of attachment 224482 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=224482&action=review
> 
> > Tools/Scripts/webkitpy/style/checkers/js.py:30
> > +This checker is only used to check WebInspector Javascript files.
> 
> Typo: "Javascript" => "JavaScript".
> 

Ok

> > Tools/Scripts/webkitpy/style/checkers/js.py:63
> > +            # FIXME: Despite stripping comments, there are two single-line comments
> 
> Apparently this comment is wrong. There are more then two single-line comments that are not stripped.
> 

Ok

> > Tools/Scripts/webkitpy/style/checkers/js.py:100
> > +        pattern = re.compile(r'//.*?$|/\*.*?\*/|\'(?:\\.|[^\\\'])*\'|"(?:\\.|[^\\"])*"', re.DOTALL | re.MULTILINE)
> 
> I wonder if the string regexes are too greedy. Given that they are multiline.
> 
> The string regex components are:
> 
>     \'(?:\\.|[^\\\'])*\'
> 
> and:
> 
>     "(?:\\.|[^\\"])*"
> 
> Both of these are greedy. I believe the pattern /A(.)*A/ should be /A(.)*?A/. This may explain why some things are not getting stripped properly.
>

I tried non-greedy and got the same results.

> Also, the regex attempts to avoid escape sequences. If you're in a single quoted string it attempts to avoid the sequence |\'| in the regex with [^\\\']. However, that is wrong.
>

My fault, the snippet of code was intented for removing comments in C. In addition, to have it completely ready for JavaScript it should match regular expressions too (in case a // or /* happens within a regex) :/

> Maybe we should rethink this.

I spent some time yesterday but I still got nothing. Before sending this patch I used a tool called jsstrip (https://code.google.com/p/jsstrip/wiki/UsagePython) to remove the comments. It worked well but it seemed to me a little bit overkill to include an external tool just for that (and there might be problems with licenses, etc). I've been taking a look at jsmin too (http://code.google.com/p/v8/source/browse/branches/bleeding_edge/tools/jsmin.py?r=3565) to see what it does for handling comments.

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