Chromium Code Reviews
Help | Chromium Project | Sign in
(47)

Issue 12474002: Support for parsing all CSS and producing one CSS file (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by terry
Modified:
2 years, 1 month ago
CC:
reviews_dartlang.org, web-ui-dev+reviews_dartlang.org
Base URL:
https://github.com/dart-lang/web-ui.git@master
Visibility:
Public.

Description

Support for parsing all CSS and producing one CSS file BUG= Committed: https://github.com/dart-lang/web-ui/commit/92b680a

Patch Set 1 #

Patch Set 2 : Use Uri parser #

Patch Set 3 : Append .css to all CSS for the app. #

Patch Set 4 : More code cleanup #

Patch Set 5 : Fixed cleanup bug #

Patch Set 6 : Merged #

Patch Set 7 : Merge fixes with latest SDK #

Patch Set 8 : merged #

Total comments: 43

Patch Set 9 : Integrated CL comments #

Patch Set 10 : Integrated CL comments #

Patch Set 11 : added new line #

Patch Set 12 : merged #

Total comments: 22

Patch Set 13 : Integrated more CL comments #

Patch Set 14 : minor nit cleanup #

Patch Set 15 : merged #

Patch Set 16 : merged #

Total comments: 2

Patch Set 17 : Reworked URI computation and added test for URI fixup. #

Total comments: 5

Patch Set 18 : Integrate CL suggestions #

Patch Set 19 : Merged #

Patch Set 20 : merged #

Patch Set 21 : merged #

Patch Set 22 : merged #

Patch Set 23 : merged #

Patch Set 24 : merged #

Patch Set 25 : merged #

Patch Set 26 : merged #

Patch Set 27 : merged #

Patch Set 28 : merged #

Patch Set 29 : merged #

Patch Set 30 : bumped version #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+274 lines, -81 lines) Patch
M example/todomvc/base.css View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -1 line 0 comments Download
M lib/src/analyzer.dart View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +13 lines, -14 lines 0 comments Download
M lib/src/compiler.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 7 chunks +145 lines, -24 lines 0 comments Download
M lib/src/emitters.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +20 lines, -9 lines 0 comments Download
M lib/src/files.dart View 1 2 3 4 5 6 7 8 1 chunk +12 lines, -2 lines 0 comments Download
M lib/src/html_css_fixup.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +29 lines, -4 lines 0 comments Download
M lib/src/info.dart View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +4 lines, -4 lines 0 comments Download
M lib/src/options.dart View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M pubspec.yaml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 1 chunk +1 line, -1 line 1 comment Download
M test/data/expected/css_main_test.html.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 1 chunk +15 lines, -14 lines 0 comments Download
M test/data/expected/todomvc_listorder_shadowdom_test.html.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 3 chunks +0 lines, -3 lines 0 comments Download
M test/data/input/css_main_test.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 3 chunks +15 lines, -1 line 0 comments Download
A test/data/input/main.png View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 0 chunks +-1 lines, --1 lines 0 comments Download
A test/data/input/resources/base.css View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +11 lines, -0 lines 0 comments Download
A test/data/input/resources/glyph.png View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 0 chunks +-1 lines, --1 lines 0 comments Download
A test/data/input/root.css View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +5 lines, -0 lines 0 comments Download
M test/emitter_test.dart View 1 2 3 4 5 6 7 8 9 10 5 chunks +5 lines, -5 lines 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 10 (0 generated)
terry
Handle parsing all local stylesheets and emitting one style sheet per app.
2 years, 1 month ago (2013-03-07 17:16:39 UTC) #1
Siggi Cherem (dart-lang)
https://chromiumcodereview.appspot.com/12474002/diff/17001/lib/src/analyzer.dart File lib/src/analyzer.dart (right): https://chromiumcodereview.appspot.com/12474002/diff/17001/lib/src/analyzer.dart#newcode730 lib/src/analyzer.dart:730: if (rel != 'components' && rel != 'stylesheet') return; ...
2 years, 1 month ago (2013-03-07 22:14:20 UTC) #2
terry
Thanks Siggi, PTAL https://chromiumcodereview.appspot.com/12474002/diff/17001/lib/src/analyzer.dart File lib/src/analyzer.dart (right): https://chromiumcodereview.appspot.com/12474002/diff/17001/lib/src/analyzer.dart#newcode730 lib/src/analyzer.dart:730: if (rel != 'components' && rel ...
2 years, 1 month ago (2013-03-08 20:11:24 UTC) #3
Siggi Cherem (dart-lang)
Thanks Terry, just a few more comments... https://chromiumcodereview.appspot.com/12474002/diff/33001/lib/src/compiler.dart File lib/src/compiler.dart (right): https://chromiumcodereview.appspot.com/12474002/diff/33001/lib/src/compiler.dart#newcode254 lib/src/compiler.dart:254: ..code = ...
2 years, 1 month ago (2013-03-08 21:57:52 UTC) #4
terry
PTAL https://chromiumcodereview.appspot.com/12474002/diff/33001/lib/src/compiler.dart File lib/src/compiler.dart (right): https://chromiumcodereview.appspot.com/12474002/diff/33001/lib/src/compiler.dart#newcode254 lib/src/compiler.dart:254: ..code = code.trim()) I like it _parseCss only. ...
2 years, 1 month ago (2013-03-08 22:42:23 UTC) #5
Siggi Cherem (dart-lang)
I forgot to mention it in the last round - I like that we reverted ...
2 years, 1 month ago (2013-03-09 01:32:09 UTC) #6
terry
https://chromiumcodereview.appspot.com/12474002/diff/31002/lib/src/html_css_fixup.dart File lib/src/html_css_fixup.dart (right): https://chromiumcodereview.appspot.com/12474002/diff/31002/lib/src/html_css_fixup.dart#newcode188 lib/src/html_css_fixup.dart:188: node.text = '${_cssRelativePath.toString()}/${node.text}'; On 2013/03/09 01:32:10, Siggi Cherem (dart-lang) ...
2 years, 1 month ago (2013-03-11 21:04:25 UTC) #7
Siggi Cherem (dart-lang)
Thanks Terry! Looks great. I just have some minor nits and fixing something small in ...
2 years, 1 month ago (2013-03-11 21:13:11 UTC) #8
terry
thx https://chromiumcodereview.appspot.com/12474002/diff/52003/lib/src/html_css_fixup.dart File lib/src/html_css_fixup.dart (right): https://chromiumcodereview.appspot.com/12474002/diff/52003/lib/src/html_css_fixup.dart#newcode186 lib/src/html_css_fixup.dart:186: ).directoryPath; On 2013/03/11 21:13:11, Siggi Cherem (dart-lang) wrote: ...
2 years, 1 month ago (2013-03-11 22:09:13 UTC) #9
Siggi Cherem (dart-lang)
2 years, 1 month ago (2013-03-11 22:21:54 UTC) #10
Message was sent while issue was closed.
lgtm

https://chromiumcodereview.appspot.com/12474002/diff/74001/pubspec.yaml
File pubspec.yaml (right):

https://chromiumcodereview.appspot.com/12474002/diff/74001/pubspec.yaml#newcode5
pubspec.yaml:5: version: 0.4.1+7
I'm inclined to keep the old version here. I don't think we need to publish a
new version right away (unless we have a bug fix to make). For now, I rather
keep this unreleased so we have a couple days to test it internally and release
it together with the next cut we make to fix breaking APIs.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld e0e3771