Merge lp:~sidnei/lazr-js/assignment-for-better-compression into lp:lazr-js

Proposed by Sidnei da Silva
Status: Merged
Merged at revision: 207
Proposed branch: lp:~sidnei/lazr-js/assignment-for-better-compression
Merge into: lp:lazr-js
Diff against target: 270 lines (+93/-83)
2 files modified
src-py/lazr/js/meta.py (+59/-24)
src-py/lazr/js/tests/test_meta.py (+34/-59)
To merge this branch: bzr merge lp:~sidnei/lazr-js/assignment-for-better-compression
Reviewer Review Type Date Requested Status
Free Ekanayaka (community) Approve
Review via email: mp+53735@code.launchpad.net

Description of the change

Uses assignment for building up the modules structure, in hope of achieving better minification of the generated metadata file.

To post a comment you must log in.
210. By Sidnei da Silva

- Also replace prefix.

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Thanks for the explanations on IRC, Sidnei. The changes look good to me, +1!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src-py/lazr/js/meta.py'
2--- src-py/lazr/js/meta.py 2010-03-16 19:07:19 +0000
3+++ src-py/lazr/js/meta.py 2011-03-17 03:46:25 +0000
4@@ -2,7 +2,6 @@
5 import re
6 import sys
7 import glob
8-import itertools
9 import optparse
10
11 import simplejson
12@@ -108,7 +107,10 @@
13 metadata.extend(meta)
14
15 modules = {}
16- all_literals = []
17+
18+ # Include config keys in literals.
19+ all_literals = ["use", "requires", "after", "type",
20+ "ext", "path", "css", "js", "skinnable"]
21 for meta in metadata:
22 name = meta.pop("name")
23 modules[name] = meta
24@@ -118,13 +120,6 @@
25 all_literals.extend(meta.get("after", ()))
26 all_literals.append(meta["type"])
27
28- # Only optimize string literals if they are used more than
29- # once, since otherwise the optimization is pointless. This
30- # loop here is basically filtering out those that only occur
31- # once.
32- literals = [k for k, g in itertools.groupby(sorted(all_literals))
33- if len(list(g)) > 1]
34-
35 # For each string literal we are interested in, generate a
36 # variable declaration for the string literal, to improve
37 # minification.
38@@ -136,10 +131,7 @@
39 # We'll save a mapping of literal -> variable name here for
40 # reuse below on the re.sub() helper function.
41 literals_map = dict([(literal, NAME_RE.sub("_", literal).upper())
42- for literal in literals])
43- variables_decl = "var %s" % ",\n ".join(
44- ["%s = \"%s\"" % (variable, literal)
45- for literal, variable in sorted(literals_map.iteritems())])
46+ for literal in set(all_literals)])
47
48 # This re.sub() helper function operates on the JSON-ified
49 # representation of the modules, by looking for string
50@@ -156,13 +148,63 @@
51 return match.group(1) + literals_map[literal] + match.group(3)
52 return match.group(0)
53
54- modules_decl = LITERAL_RE.sub(literal_sub, simplejson.dumps(modules))
55+ variables_decl = ",\n ".join(
56+ ["var SKIN_SAM_PREFIX = 'skin-sam-'",
57+ "PREFIX = '%s'" % self.prefix] +
58+ ["%s = %s" % (
59+ variable, ('"%s"' % literal).replace(
60+ '"skin-sam-', 'SKIN_SAM_PREFIX + "').replace(
61+ '"%s.' % self.prefix, 'PREFIX + ".'))
62+ for literal, variable in sorted(literals_map.iteritems())])
63+
64+ # Default 'after' modules from YUI Loader. Might have to
65+ # be changed in the future, if YUI itself changes.
66+ core_css = ["cssreset", "cssfonts",
67+ "cssgrids", "cssreset-context",
68+ "cssfonts-context",
69+ "cssgrids-context"]
70+
71+ # Append a few more helper variables for our own use.
72+ variables_decl += ",\n ".join(
73+ ["",
74+ "modules = {}",
75+ "TRUE = true",
76+ "FALSE = false",
77+ "CORE_CSS = %s" % LITERAL_RE.sub(literal_sub,
78+ simplejson.dumps(core_css)),
79+ "module_info"])
80+
81+ modules_decl = []
82+ for module_name, module_info in sorted(modules.iteritems()):
83+ module_decl = [
84+ "modules[%s] = module_info = {}" %
85+ NAME_RE.sub("_", module_name).upper()]
86+ for key, value in sorted(module_info.iteritems()):
87+ if value is True or value is False:
88+ value = str(value).upper()
89+ elif value in ("css", "js"):
90+ value = value.upper()
91+ else:
92+ value = LITERAL_RE.sub(literal_sub, simplejson.dumps(value))
93+ if key == "after" and module_info["type"] == "css":
94+ value = "CORE_CSS + %s" % value
95+ if key == "path":
96+ value = value.replace(
97+ '"%s/' % self.prefix, 'PREFIX + "/')
98+ module_decl.append("module_info[%s] = %s" %
99+ (key.upper(), value))
100+ modules_decl.append(";\n ".join(module_decl))
101+
102+ modules_decl = ";\n\n ".join(modules_decl)
103
104 module_config = open(out, "w")
105 try:
106 module_config.write("""var %s = (function(){
107 %s;
108- return %s;
109+
110+ %s;
111+
112+ return modules;
113 })();""" % (var_name, variables_decl, modules_decl))
114 finally:
115 module_config.close()
116@@ -181,19 +223,12 @@
117 if self.prefix and not prefix.endswith("/"):
118 prefix = self.prefix + "/"
119
120- # Default 'after' modules from YUI Loader. Might have to
121- # be changed in the future, if YUI itself changes.
122- after = ["cssreset", "cssfonts",
123- "cssgrids", "cssreset-context",
124- "cssfonts-context",
125- "cssgrids-context"]
126-
127 if entry.get("requires"):
128 # If the base module requires other modules, extend
129 # the after entry with the (expected) skins for those
130 # modules to force our skin to be loaded after those.
131- after.extend(["skin-sam-%s" % s
132- for s in entry["requires"]])
133+ after = ["skin-sam-%s" % s
134+ for s in entry["requires"]]
135
136 assets = os.path.join(
137 os.path.dirname(entry["path"][len(prefix):]), "assets")
138
139=== modified file 'src-py/lazr/js/tests/test_meta.py'
140--- src-py/lazr/js/tests/test_meta.py 2010-03-16 19:07:19 +0000
141+++ src-py/lazr/js/tests/test_meta.py 2011-03-17 03:46:25 +0000
142@@ -2,7 +2,6 @@
143 from unittest import defaultTestLoader, TestCase
144
145 import mocker
146-import simplejson
147
148 from lazr.js.meta import Builder, extract_metadata
149
150@@ -220,13 +219,13 @@
151
152 got = open(output, "r").read()
153 prefix = got[:18]
154- modules = got.splitlines()[-2].strip()[7:-1]
155+ modules = "\n\n".join(got.split("\n\n")[1:-1])
156 self.assertEquals(prefix, "var LAZR_CONFIG = ")
157- self.assertTrue('"path": "anim/anim-min.js"' in modules)
158- self.assertFalse('"skinnable": false' in modules)
159- self.assertTrue('"type": "js"' in modules)
160- self.assertTrue('"use": ["dom"]' in modules)
161- self.assertTrue('"ext": true' in modules)
162+ self.assertTrue('[PATH] = "anim/anim-min.js"' in modules)
163+ self.assertFalse('[SKINNABLE] = FALSE' in modules)
164+ self.assertTrue('[TYPE] = JS' in modules)
165+ self.assertTrue('[USE] = [DOM]' in modules)
166+ self.assertTrue('[EXT] = TRUE' in modules)
167
168 def test_extract_skinnable(self):
169 """
170@@ -256,22 +255,21 @@
171
172 got = open(output, "r").read()
173 prefix = got[:18]
174- modules = got.splitlines()[-2].strip()[7:-1]
175+ modules = "\n\n".join(got.split("\n\n")[1:-1])
176 self.assertEquals(prefix, "var LAZR_CONFIG = ")
177 self.assertTrue(
178- '"path": "lazr/anim/assets/skins/sam/anim-skin.css"' in modules)
179- self.assertTrue(
180- '"path": "lazr/anim/anim-min.js"' in modules)
181- self.assertTrue('"skinnable": true' in modules)
182- self.assertTrue('"type": "js"' in modules)
183- self.assertTrue('"type": "css"' in modules)
184- self.assertTrue('"use": ["dom"]' in modules)
185- self.assertTrue('"requires": ["widget"]' in modules)
186- self.assertTrue(
187- ('"after": ["cssreset", "cssfonts", "cssgrids", '
188- '"cssreset-context", "cssfonts-context", "cssgrids-context", '
189- '"skin-sam-widget"]') in modules)
190- self.assertTrue('"ext": true' in modules)
191+ '[PATH] = PREFIX + '
192+ '"/anim/assets/skins/sam/anim-skin.css"' in modules)
193+ self.assertTrue(
194+ '[PATH] = PREFIX + "/anim/anim-min.js"' in modules)
195+ self.assertTrue('[SKINNABLE] = TRUE' in modules)
196+ self.assertTrue('[TYPE] = JS' in modules)
197+ self.assertTrue('[TYPE] = CSS' in modules)
198+ self.assertTrue('[USE] = [DOM]' in modules)
199+ self.assertTrue('[REQUIRES] = [WIDGET]' in modules)
200+ self.assertTrue(
201+ ('[AFTER] = CORE_CSS + [SKIN_SAM_WIDGET]') in modules)
202+ self.assertTrue('[EXT] = TRUE' in modules)
203
204 def test_extract_skinnable_with_lazr_conventions(self):
205 """
206@@ -308,49 +306,26 @@
207
208 got = open(output, "r").read()
209 prefix = got[:18]
210- lines = got.splitlines()
211- modules = lines[-2].strip()[7:-1]
212- variables = {}
213- for line in lines[1:-2]:
214- variable, literal = line.split("=")
215- variable = variable.strip(" ").split(" ")[-1]
216- variables[variable] = literal.strip(" ").split("\"")[1]
217+ blocks = got.split("\n\n")
218+ modules = "\n\n".join(blocks[1:-1])
219 self.assertEquals(prefix, "var LAZR_CONFIG = ")
220
221- # Variables are only defined for optimizing minification if
222- # they are used more than once, see how "dom" and "js" are not
223- # included in this list, even though they are used by the
224- # module. All the other names are used more than once due to
225- # the two CSS files that are part of the skin for this module.
226- self.assertEquals(variables, {
227- "CSS": "css",
228- "CSSFONTS": "cssfonts",
229- "CSSFONTS_CONTEXT": "cssfonts-context",
230- "CSSGRIDS": "cssgrids",
231- "CSSGRIDS_CONTEXT": "cssgrids-context",
232- "CSSRESET": "cssreset",
233- "CSSRESET_CONTEXT": "cssreset-context",
234- "SKIN_SAM_LAZR_ANIM_CORE": "skin-sam-lazr.anim+core",
235- "SKIN_SAM_WIDGET": "skin-sam-widget"})
236-
237- self.assertTrue(
238- '"path": "lazr/anim/assets/purty-anim-core.css"' in modules)
239- self.assertTrue(
240- '"path": "lazr/anim/assets/skins/sam/purty-anim-skin.css"'
241+ self.assertTrue(
242+ '[PATH] = PREFIX + "/anim/assets/purty-anim-core.css"' in modules)
243+ self.assertTrue(
244+ '[PATH] = PREFIX + "/anim/assets/skins/sam/purty-anim-skin.css"'
245 in modules)
246 self.assertTrue(
247- '"path": "lazr/anim/anim-min.js"' in modules)
248- self.assertTrue('"skinnable": true' in modules)
249- self.assertTrue('"type": "js"' in modules)
250- self.assertTrue('"type": CSS' in modules)
251- self.assertTrue('"use": ["dom"]' in modules)
252- self.assertTrue('"requires": ["widget"]' in modules)
253- self.assertTrue('"requires": [SKIN_SAM_LAZR_ANIM_CORE]' in modules)
254+ '[PATH] = PREFIX + "/anim/anim-min.js"' in modules)
255+ self.assertTrue('[SKINNABLE] = TRUE' in modules)
256+ self.assertTrue('[TYPE] = JS' in modules)
257+ self.assertTrue('[TYPE] = CSS' in modules)
258+ self.assertTrue('[USE] = [DOM]' in modules)
259+ self.assertTrue('[REQUIRES] = [WIDGET]' in modules)
260+ self.assertTrue('[REQUIRES] = [SKIN_SAM_LAZR_ANIM_CORE]' in modules)
261 self.assertTrue(
262- ('"after": [CSSRESET, CSSFONTS, CSSGRIDS, '
263- 'CSSRESET_CONTEXT, CSSFONTS_CONTEXT, CSSGRIDS_CONTEXT, '
264- 'SKIN_SAM_WIDGET, SKIN_SAM_LAZR_ANIM_CORE]') in modules)
265- self.assertTrue('"ext": true' in modules)
266+ ('[AFTER] = CORE_CSS + [SKIN_SAM_WIDGET, SKIN_SAM_LAZR_ANIM_CORE]') in modules)
267+ self.assertTrue('[EXT] = TRUE' in modules)
268
269
270 def test_suite():

Subscribers

People subscribed via source and target branches