launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02991
[Merge] lp:~sidnei/lazr-js/assignment-for-better-compression into lp:lazr-js
Sidnei da Silva has proposed merging lp:~sidnei/lazr-js/assignment-for-better-compression into lp:lazr-js.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #736553 in LAZR Javascript Library: "make metadata more compressable by using assignment"
https://bugs.launchpad.net/lazr-js/+bug/736553
For more details, see:
https://code.launchpad.net/~sidnei/lazr-js/assignment-for-better-compression/+merge/53735
Uses assignment for building up the modules structure, in hope of achieving better minification of the generated metadata file.
--
https://code.launchpad.net/~sidnei/lazr-js/assignment-for-better-compression/+merge/53735
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sidnei/lazr-js/assignment-for-better-compression into lp:lazr-js.
=== modified file 'src-py/lazr/js/meta.py'
--- src-py/lazr/js/meta.py 2010-03-16 19:07:19 +0000
+++ src-py/lazr/js/meta.py 2011-03-17 03:14:23 +0000
@@ -2,7 +2,6 @@
import re
import sys
import glob
-import itertools
import optparse
import simplejson
@@ -108,7 +107,10 @@
metadata.extend(meta)
modules = {}
- all_literals = []
+
+ # Include config keys in literals.
+ all_literals = ["use", "requires", "after", "type",
+ "ext", "path", "css", "js", "skinnable"]
for meta in metadata:
name = meta.pop("name")
modules[name] = meta
@@ -118,13 +120,6 @@
all_literals.extend(meta.get("after", ()))
all_literals.append(meta["type"])
- # Only optimize string literals if they are used more than
- # once, since otherwise the optimization is pointless. This
- # loop here is basically filtering out those that only occur
- # once.
- literals = [k for k, g in itertools.groupby(sorted(all_literals))
- if len(list(g)) > 1]
-
# For each string literal we are interested in, generate a
# variable declaration for the string literal, to improve
# minification.
@@ -136,10 +131,7 @@
# We'll save a mapping of literal -> variable name here for
# reuse below on the re.sub() helper function.
literals_map = dict([(literal, NAME_RE.sub("_", literal).upper())
- for literal in literals])
- variables_decl = "var %s" % ",\n ".join(
- ["%s = \"%s\"" % (variable, literal)
- for literal, variable in sorted(literals_map.iteritems())])
+ for literal in set(all_literals)])
# This re.sub() helper function operates on the JSON-ified
# representation of the modules, by looking for string
@@ -156,13 +148,60 @@
return match.group(1) + literals_map[literal] + match.group(3)
return match.group(0)
- modules_decl = LITERAL_RE.sub(literal_sub, simplejson.dumps(modules))
+ variables_decl = ",\n ".join(
+ ["var SKIN_SAM_PREFIX = 'skin-sam-'",
+ "LAZR = 'lazr'"] +
+ ["%s = %s" % (
+ variable, ('"%s"' % literal).replace(
+ '"skin-sam-', 'SKIN_SAM_PREFIX + "').replace(
+ '"lazr', 'LAZR + "'))
+ for literal, variable in sorted(literals_map.iteritems())])
+
+ # Default 'after' modules from YUI Loader. Might have to
+ # be changed in the future, if YUI itself changes.
+ core_css = ["cssreset", "cssfonts",
+ "cssgrids", "cssreset-context",
+ "cssfonts-context",
+ "cssgrids-context"]
+
+ # Append a few more helper variables for our own use.
+ variables_decl += ",\n ".join(
+ ["",
+ "modules = {}",
+ "TRUE = true",
+ "FALSE = false",
+ "CORE_CSS = %s" % LITERAL_RE.sub(literal_sub,
+ simplejson.dumps(core_css)),
+ "module_info"])
+
+ modules_decl = []
+ for module_name, module_info in sorted(modules.iteritems()):
+ module_decl = [
+ "modules[%s] = module_info = {}" %
+ NAME_RE.sub("_", module_name).upper()]
+ for key, value in sorted(module_info.iteritems()):
+ if value is True or value is False:
+ value = str(value).upper()
+ elif value in ("css", "js"):
+ value = value.upper()
+ else:
+ value = LITERAL_RE.sub(literal_sub, simplejson.dumps(value))
+ if key == "after" and module_info["type"] == "css":
+ value = "CORE_CSS + %s" % value
+ module_decl.append("module_info[%s] = %s" %
+ (key.upper(), value))
+ modules_decl.append(";\n ".join(module_decl))
+
+ modules_decl = ";\n\n ".join(modules_decl)
module_config = open(out, "w")
try:
module_config.write("""var %s = (function(){
%s;
- return %s;
+
+ %s;
+
+ return modules;
})();""" % (var_name, variables_decl, modules_decl))
finally:
module_config.close()
@@ -181,19 +220,12 @@
if self.prefix and not prefix.endswith("/"):
prefix = self.prefix + "/"
- # Default 'after' modules from YUI Loader. Might have to
- # be changed in the future, if YUI itself changes.
- after = ["cssreset", "cssfonts",
- "cssgrids", "cssreset-context",
- "cssfonts-context",
- "cssgrids-context"]
-
if entry.get("requires"):
# If the base module requires other modules, extend
# the after entry with the (expected) skins for those
# modules to force our skin to be loaded after those.
- after.extend(["skin-sam-%s" % s
- for s in entry["requires"]])
+ after = ["skin-sam-%s" % s
+ for s in entry["requires"]]
assets = os.path.join(
os.path.dirname(entry["path"][len(prefix):]), "assets")
=== modified file 'src-py/lazr/js/tests/test_meta.py'
--- src-py/lazr/js/tests/test_meta.py 2010-03-16 19:07:19 +0000
+++ src-py/lazr/js/tests/test_meta.py 2011-03-17 03:14:23 +0000
@@ -2,7 +2,6 @@
from unittest import defaultTestLoader, TestCase
import mocker
-import simplejson
from lazr.js.meta import Builder, extract_metadata
@@ -220,13 +219,13 @@
got = open(output, "r").read()
prefix = got[:18]
- modules = got.splitlines()[-2].strip()[7:-1]
+ modules = "\n\n".join(got.split("\n\n")[1:-1])
self.assertEquals(prefix, "var LAZR_CONFIG = ")
- self.assertTrue('"path": "anim/anim-min.js"' in modules)
- self.assertFalse('"skinnable": false' in modules)
- self.assertTrue('"type": "js"' in modules)
- self.assertTrue('"use": ["dom"]' in modules)
- self.assertTrue('"ext": true' in modules)
+ self.assertTrue('[PATH] = "anim/anim-min.js"' in modules)
+ self.assertFalse('[SKINNABLE] = FALSE' in modules)
+ self.assertTrue('[TYPE] = JS' in modules)
+ self.assertTrue('[USE] = [DOM]' in modules)
+ self.assertTrue('[EXT] = TRUE' in modules)
def test_extract_skinnable(self):
"""
@@ -256,22 +255,20 @@
got = open(output, "r").read()
prefix = got[:18]
- modules = got.splitlines()[-2].strip()[7:-1]
+ modules = "\n\n".join(got.split("\n\n")[1:-1])
self.assertEquals(prefix, "var LAZR_CONFIG = ")
self.assertTrue(
- '"path": "lazr/anim/assets/skins/sam/anim-skin.css"' in modules)
- self.assertTrue(
- '"path": "lazr/anim/anim-min.js"' in modules)
- self.assertTrue('"skinnable": true' in modules)
- self.assertTrue('"type": "js"' in modules)
- self.assertTrue('"type": "css"' in modules)
- self.assertTrue('"use": ["dom"]' in modules)
- self.assertTrue('"requires": ["widget"]' in modules)
- self.assertTrue(
- ('"after": ["cssreset", "cssfonts", "cssgrids", '
- '"cssreset-context", "cssfonts-context", "cssgrids-context", '
- '"skin-sam-widget"]') in modules)
- self.assertTrue('"ext": true' in modules)
+ '[PATH] = "lazr/anim/assets/skins/sam/anim-skin.css"' in modules)
+ self.assertTrue(
+ '[PATH] = "lazr/anim/anim-min.js"' in modules)
+ self.assertTrue('[SKINNABLE] = TRUE' in modules)
+ self.assertTrue('[TYPE] = JS' in modules)
+ self.assertTrue('[TYPE] = CSS' in modules)
+ self.assertTrue('[USE] = [DOM]' in modules)
+ self.assertTrue('[REQUIRES] = [WIDGET]' in modules)
+ self.assertTrue(
+ ('[AFTER] = CORE_CSS + [SKIN_SAM_WIDGET]') in modules)
+ self.assertTrue('[EXT] = TRUE' in modules)
def test_extract_skinnable_with_lazr_conventions(self):
"""
@@ -308,49 +305,26 @@
got = open(output, "r").read()
prefix = got[:18]
- lines = got.splitlines()
- modules = lines[-2].strip()[7:-1]
- variables = {}
- for line in lines[1:-2]:
- variable, literal = line.split("=")
- variable = variable.strip(" ").split(" ")[-1]
- variables[variable] = literal.strip(" ").split("\"")[1]
+ blocks = got.split("\n\n")
+ modules = "\n\n".join(blocks[1:-1])
self.assertEquals(prefix, "var LAZR_CONFIG = ")
- # Variables are only defined for optimizing minification if
- # they are used more than once, see how "dom" and "js" are not
- # included in this list, even though they are used by the
- # module. All the other names are used more than once due to
- # the two CSS files that are part of the skin for this module.
- self.assertEquals(variables, {
- "CSS": "css",
- "CSSFONTS": "cssfonts",
- "CSSFONTS_CONTEXT": "cssfonts-context",
- "CSSGRIDS": "cssgrids",
- "CSSGRIDS_CONTEXT": "cssgrids-context",
- "CSSRESET": "cssreset",
- "CSSRESET_CONTEXT": "cssreset-context",
- "SKIN_SAM_LAZR_ANIM_CORE": "skin-sam-lazr.anim+core",
- "SKIN_SAM_WIDGET": "skin-sam-widget"})
-
- self.assertTrue(
- '"path": "lazr/anim/assets/purty-anim-core.css"' in modules)
- self.assertTrue(
- '"path": "lazr/anim/assets/skins/sam/purty-anim-skin.css"'
+ self.assertTrue(
+ '[PATH] = "lazr/anim/assets/purty-anim-core.css"' in modules)
+ self.assertTrue(
+ '[PATH] = "lazr/anim/assets/skins/sam/purty-anim-skin.css"'
in modules)
self.assertTrue(
- '"path": "lazr/anim/anim-min.js"' in modules)
- self.assertTrue('"skinnable": true' in modules)
- self.assertTrue('"type": "js"' in modules)
- self.assertTrue('"type": CSS' in modules)
- self.assertTrue('"use": ["dom"]' in modules)
- self.assertTrue('"requires": ["widget"]' in modules)
- self.assertTrue('"requires": [SKIN_SAM_LAZR_ANIM_CORE]' in modules)
+ '[PATH] = "lazr/anim/anim-min.js"' in modules)
+ self.assertTrue('[SKINNABLE] = TRUE' in modules)
+ self.assertTrue('[TYPE] = JS' in modules)
+ self.assertTrue('[TYPE] = CSS' in modules)
+ self.assertTrue('[USE] = [DOM]' in modules)
+ self.assertTrue('[REQUIRES] = [WIDGET]' in modules)
+ self.assertTrue('[REQUIRES] = [SKIN_SAM_LAZR_ANIM_CORE]' in modules)
self.assertTrue(
- ('"after": [CSSRESET, CSSFONTS, CSSGRIDS, '
- 'CSSRESET_CONTEXT, CSSFONTS_CONTEXT, CSSGRIDS_CONTEXT, '
- 'SKIN_SAM_WIDGET, SKIN_SAM_LAZR_ANIM_CORE]') in modules)
- self.assertTrue('"ext": true' in modules)
+ ('[AFTER] = CORE_CSS + [SKIN_SAM_WIDGET, SKIN_SAM_LAZR_ANIM_CORE]') in modules)
+ self.assertTrue('[EXT] = TRUE' in modules)
def test_suite():
