Bug 49586 – Chrome DevTools: concatenate CSS files, do not link missing JS files in release mode.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

Drive-by move of py script files upstream.

[Pavel Feldman]

Created attachment 73976 [details]
[PATCH] Proposed change.

[WebKit Review Bot]

Attachment 73976 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/Build/concatenate_css_files.py', u'WebKit/chromium/Build/concatenate_js_files.py', u'WebKit/chromium/Build/generate_devtools_html.py', u'WebKit/chromium/ChangeLog', u'WebKit/chromium/WebKit.gyp']" exit_code: 1
Last 3072 characters of output:
 four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:74:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:75:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:76:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:77:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:81:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:86:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:9:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/generate_devtools_html.py:10:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:11:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:13:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:15:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:16:  at least two spaces before inline comment  [pep8/E261] [5]
WebKit/chromium/Build/generate_devtools_html.py:21:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:29:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:30:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:31:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:32:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:33:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:35:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:37:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:39:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:41:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:43:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:47:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:52:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:53:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:57:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:62:  indentation is not a multiple of four  [pep8/E111] [5]
Total errors found: 79 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mikhail Naganov]

Comment on attachment 73976 [details]
[PATCH] Proposed change.

View in context: https://bugs.webkit.org/attachment.cgi?id=73976&action=review

> WebKit/chromium/WebKit.gyp:733
>                      '<@(devtools_files)',

If you are adding 'DevTools.js' and 'Tests.js' manually in the script, why do you need devtools_files here?

> WebKit/chromium/WebKit.gyp:734
> +                    '<(SHARED_INTERMEDIATE_DIR)/webcore/InspectorBackendStub.js'

BTW, Python is OK with array elements list ended with a comma.

> WebKit/chromium/WebKit.gyp:750
> +                '../../WebCore/WebCore.gyp/WebCore.gyp:inspector_protocol_sources'

Why CSS depends on inspector_protocol_sources?

> WebKit/chromium/WebKit.gyp:761
> +                    '<@(devtools_files)'

If you are adding 'devTools.css' manually in the script, why do you need devtools_files here?

[Pavel Feldman]

Created attachment 73984 [details]
[PATCH] Same with style fixes

[WebKit Review Bot]

Attachment 73984 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/Build/concatenate_css_files.py', u'WebKit/chromium/Build/concatenate_js_files.py', u'WebKit/chromium/Build/generate_devtools_html.py', u'WebKit/chromium/ChangeLog', u'WebKit/chromium/WebKit.gyp']" exit_code: 1
Last 3072 characters of output:
2] [5]
WebKit/chromium/Build/concatenate_js_files.py:31:  multiple statements on one line (semicolon)  [pep8/E702] [5]
WebKit/chromium/Build/concatenate_js_files.py:46:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_js_files.py:73:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_js_files.py:78:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:14:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_css_files.py:21:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:29:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_css_files.py:31:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:32:  multiple statements on one line (semicolon)  [pep8/E702] [5]
WebKit/chromium/Build/concatenate_css_files.py:32:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:34:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:35:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:36:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:37:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:38:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:39:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:40:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:41:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:43:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:44:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:45:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:47:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_css_files.py:53:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:58:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:61:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:73:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:78:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:84:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/generate_devtools_html.py:9:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/generate_devtools_html.py:16:  at least two spaces before inline comment  [pep8/E261] [5]
Total errors found: 32 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Pavel Feldman]

Created attachment 73985 [details]
[PATCH] Same with style fixes for real.

[Pavel Feldman]

> > WebKit/chromium/WebKit.gyp:750
> > +                '../../WebCore/WebCore.gyp/WebCore.gyp:inspector_protocol_sources'
> 

Fixed.

Rest seems to be correct as we discussed offline.

[Pavel Feldman]

Created attachment 73986 [details]
[PATCH] Addressed Mikhail's comments.

[Mikhail Naganov]

Comment on attachment 73986 [details]
[PATCH] Addressed Mikhail's comments.

Looks good to me, I would put r+

[Yury Semikhatsky]

Comment on attachment 73986 [details]
[PATCH] Addressed Mikhail's comments.

View in context: https://bugs.webkit.org/attachment.cgi?id=73986&action=review

Please address the comments before landing.

> WebKit/chromium/Build/concatenate_css_files.py:2
> +# Copyright (c) 2009 The Chromium Authors. All rights reserved.

it's 2010 already:) and btw should we use a different license header like in the other webkit files?

> WebKit/chromium/Build/concatenate_css_files.py:7
> +# using <script> tags in a given 'order.html' file.

did you mean using <link> tags?

> WebKit/chromium/Build/concatenate_css_files.py:60
> +    extractor.ordered_css_files.append('devTools.css')

should we have a comment explaining this stuff?

> WebKit/chromium/Build/concatenate_css_files.py:67
> +        if (full_path is None):

nit: if not full_path:

> WebKit/chromium/Build/concatenate_js_files.py:2
> +# Copyright (c) 2009 The Chromium Authors. All rights reserved.

2009->2010

> WebKit/chromium/Build/concatenate_js_files.py:42
> +                    raise Exception('Ambiguous file %s: found in %s and %s' %

I understand that these are build scripts but can we extract common functionality into its own file?

> WebKit/chromium/Build/generate_devtools_html.py:2
> +# Copyright (c) 2009 The Chromium Authors. All rights reserved.

2009->2010

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        A       WebKit/chromium/Build/concatenate_css_files.py
        A       WebKit/chromium/Build/concatenate_js_files.py
        A       WebKit/chromium/Build/generate_devtools_html.py
        M       WebKit/chromium/ChangeLog
        M       WebKit/chromium/WebKit.gyp
Committed r72103