New CSS parser written in Dart to replace pyparser

utils/css/generate.dart

+// Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+class Generate {

[nweiz, 2012/01/04 19:05:41]

A class full of static methods seems like it wants to just be a bunch of
top-level methods instead.

+
+  // Build up list of all known class selectors in all CSS files.
+  static List<String> computeClassSelectors(RuleSet ruleset, classes) {

[nweiz, 2012/01/04 19:05:41]

Shouldn't this be a Set rather than a List? That would make this O(n) rather
than O(n^2).

Even if it's a list, you don't need to return it, since you're destructively
modifying it.

+    for (var selector in ruleset.selectorGroup.selectors) {
+      var selSeqs = selector.simpleSelectorSequences;
+      for (selSeq in selSeqs) {
+        var simpleSelector = selSeq.simpleSelector;
+        if (simpleSelector is ClassSelector) {
+          String className = simpleSelector.name;
+          if (classes.indexOf(className) == -1) {   // Class name already known?
+            // No, expose it.
+            classes.add(className);
+          }
+        }
+      }
+    }
+
+    return classes;
+  }
+
+  static dartClass(FileSystem files, String outPath, Stylesheet stylesheet,

[nweiz, 2012/01/04 19:05:41]

This should either list the return type or not return a value.

+      String filename) {
+
+    List<String> knownClasses = [];

[nweiz, 2012/01/04 19:05:41]

Set?

+
+    StringBuffer buff = new StringBuffer(
+        '// File generated by Dart CSS from source file ${filename}\n' +

[nweiz, 2012/01/04 19:05:41]

Style nit: $filename

+        '// Do not edit.\n\n');
+
+    // Emit class for any @stylet directive.

[nweiz, 2012/01/04 19:05:41]

Shouldn't this comment be attached to the StyletDirective code in particular?

+    for (var production in stylesheet._topLevels) {
+      if (production is Directive) {

[nweiz, 2012/01/04 19:05:41]

Looks like you can just check for StyletDirective and IncludeDirective directly,
without the additional nesting of the Directive check.

+        if (production is StyletDirective) {
+          // TODO(terry): Need safer mechanism for stylets in different files
+          //              and stylets with colliding names.
+          buff.add('class ${production.dartClassName} {\n');
+          buff.add('  // selector, properties<propertyName, value>\n');
+          buff.add('  static final selectors = const {\n');
+
+          for (var ruleset in production.rulesets) {
+            for (var selector in ruleset.selectorGroup.selectors) {

[nweiz, 2012/01/04 19:05:41]

I'm having trouble understanding this loop. Why are you ignoring simple selector
sequences with length > 1? If there's more than one with length == 1, won't you
end up with more than one "{"?

If you only expect there to be one simple selector in the ruleset, make that
explicit by using #first and asserting the length.

+              var selSeq = selector.simpleSelectorSquences;

[nweiz, 2012/01/04 19:05:41]

s/Squences/Sequences/

+              if (selSeq.length == 1) {
+                buff.add('    \'${selSeq.toString()}\' : const {\n');

[nweiz, 2012/01/04 19:05:41]

$selSeq; toString will be called automatically.

+              }
+            }
+
+            for (var decl in ruleset.declarationGroup.declarations) {
+              buff.add('      \'${decl.property}\' : ' +
+                  '\'${decl.expression.toString()}\',\n');

[nweiz, 2012/01/04 19:05:41]

${decl.expression}

+            }
+            buff.add('    },\n');   // End of declarations for stylet class.
+          }
+          buff.add('  };\n');       // End of static selectors constant.
+          buff.add('}\n\n');        // End of stylet class
+        } else if (production is IncludeDirective) {
+          for (var topLevel in production.styleSheet._topLevels) {
+            if (topLevel is RuleSet) {
+              knownClasses = computeClassSelectors(topLevel, knownClasses);
+            }
+          }
+        }
+      } else if (production is RuleSet) {
+        knownClasses = computeClassSelectors(production, knownClasses);
+      }
+    }
+
+    // Generate all known classes encountered in all processed CSS files.
+    StringBuffer classSelectors = new StringBuffer(

[nweiz, 2012/01/04 19:05:41]

Why is this a separate StringBuffer? Why not use the existing buffer?

+      'class CSS {\n' +
+      '  // CSS class selectors:\n');
+    for (var className in knownClasses) {
+      String classAsDart = className.replaceAll('-', '_').toUpperCase();
+      classSelectors.add('  static final String ${classAsDart} = ' +
+          '\'${className}\';\n');
+    }
+    classSelectors.add('}\n');            // End of class selectors.
+    buff.add(classSelectors.toString());
+
+    // Write Dart file.
+    String outFile = '${outPath}CSS.dart';
+    files.writeString(outFile, buff.toString());
+
+    return outFile;
+  }
+}
+