From 43a4758865cbacbbde5bbb57ab3a36493c4b77ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Elsd=C3=B6rfer?= Date: Sun, 18 Nov 2012 21:53:22 +0100 Subject: [PATCH] Implement JST/Handlebars as concat() filters. This is a much better approach, no longer requires abusing multiple filter functions to work in concert, fixes the caching issue of #188, and in general should save us some headache. As part of this, FilterTool.apply_func was changed to raise an exception if no filters run. This allows us to differentiate between no filter running and a filter not returning anything. We didn't end up needing this here in the end, but it is useful nonetheless. --- docs/upgrading.rst | 8 +++++ src/webassets/bundle.py | 34 +++++++++++--------- src/webassets/filter/__init__.py | 2 +- src/webassets/filter/handlebars.py | 37 +++++++++++++--------- src/webassets/filter/jst.py | 50 ++++++++++++++++-------------- src/webassets/merge.py | 8 +++-- tests/test_bundle_build.py | 2 +- tests/test_bundle_various.py | 2 +- 8 files changed, 87 insertions(+), 56 deletions(-) diff --git a/docs/upgrading.rst b/docs/upgrading.rst index 96811247..c246331f 100644 --- a/docs/upgrading.rst +++ b/docs/upgrading.rst @@ -135,6 +135,14 @@ In Development version output as raw strings. Before, ``False`` behaved like ``None`` and used the builtin compiler. +- The API of the ``concat()`` filter method has changed. Instead of a + list of hunks, it is now given a list of 2-tuples of + ``(hunk, info_dict)``. + +- The internal ``JSTTemplateFilter`` base class has changed API. + - concat filter + - jst handlebar filters have changed, use concat, base class has changed + In 0.7 ~~~~~~ diff --git a/src/webassets/bundle.py b/src/webassets/bundle.py index 66861726..9cdd9154 100644 --- a/src/webassets/bundle.py +++ b/src/webassets/bundle.py @@ -4,7 +4,7 @@ from filter import get_filter from merge import (FileHunk, UrlHunk, FilterTool, merge, merge_filters, - select_filters, MoreThanOneFilterError) + select_filters, MoreThanOneFilterError, NoFilters) from updater import SKIP_CACHE from exceptions import BundleError, BuildError from utils import cmp_debug_levels @@ -343,7 +343,7 @@ def _merge_and_apply(self, env, output, force, parent_debug=None, # We construct two lists of filters. The ones we want to use in this # iteration, and the ones we want to pass down to child bundles. - # Why? Say we are in merge mode. Assume an "input()" filter which does + # Why? Say we are in merge mode. Assume an "input()" filter which does # not run in merge mode, and a child bundle that switches to # debug=False. The child bundle then DOES want to run those input # filters, so we do need to pass them. @@ -378,11 +378,13 @@ def _merge_and_apply(self, env, output, force, parent_debug=None, hunks = [] for item, cnt in resolved_contents: if isinstance(cnt, Bundle): + # Recursively process nested bundles. hunk = cnt._merge_and_apply( env, output, force, current_debug_level, filters_to_pass_down, disable_cache=disable_cache) if hunk is not None: - hunks.append(hunk) + hunks.append((hunk, {})) + else: # Give a filter the chance to open his file. try: @@ -403,19 +405,23 @@ def _merge_and_apply(self, env, output, force, parent_debug=None, cache_key=[FileHunk(cnt)] if not is_url(cnt) else []) except MoreThanOneFilterError, e: raise BuildError(e) - - if not hunk: + except NoFilters: + # Open the file ourselves. if is_url(cnt): hunk = UrlHunk(cnt, env=env) else: hunk = FileHunk(cnt) - hunks.append(filtertool.apply( - hunk, filters_to_run, 'input', - # Pass along both the original relative path, as - # specified by the user, and the one that has been - # resolved to a filesystem location. - kwargs={'source': item, 'source_path': cnt})) + # With the hunk, remember both the original relative + # path, as specified by the user, and the one that has + # been resolved to a filesystem location. We'll pass + # them along to various filter steps. + item_data = {'source': item, 'source_path': cnt} + + # Run input filters, unless open() told us not to. + hunk = filtertool.apply(hunk, filters_to_run, 'input', + kwargs=item_data) + hunks.append((hunk, item_data)) # If this bundle is empty (if it has nested bundles, they did # not yield any hunks either), return None to indicate so. @@ -426,10 +432,10 @@ def _merge_and_apply(self, env, output, force, parent_debug=None, # a filter here, by implementing a concat() method. try: final = filtertool.apply_func(filters_to_run, 'concat', [hunks]) - if final is None: - final = merge(hunks) - except (IOError, MoreThanOneFilterError), e: + except MoreThanOneFilterError, e: raise BuildError(e) + except NoFilters: + final = merge([h for h, _ in hunks]) # Apply output filters. # TODO: So far, all the situations where bundle dependencies are diff --git a/src/webassets/filter/__init__.py b/src/webassets/filter/__init__.py index 7aedb046..84ea87b8 100644 --- a/src/webassets/filter/__init__.py +++ b/src/webassets/filter/__init__.py @@ -285,7 +285,7 @@ def concat(self, out, hunks, **kw): Will be called once between the input() and output() steps, and should concat all the source files (given as hunks) - together, and return a string. + together, writing the result to the ``out`` stream. Only one such filter is allowed. """ diff --git a/src/webassets/filter/handlebars.py b/src/webassets/filter/handlebars.py index 03e6c503..675f9ac2 100644 --- a/src/webassets/filter/handlebars.py +++ b/src/webassets/filter/handlebars.py @@ -3,7 +3,6 @@ from webassets.exceptions import FilterError from webassets.filter.jst import JSTemplateFilter -from webassets.merge import FileHunk __all__ = ('Handlebars',) @@ -15,11 +14,27 @@ class Handlebars(JSTemplateFilter): This filter assumes that the ``handlebars`` executable is in the path. Otherwise, you may define a ``HANDLEBARS_BIN`` setting. - Note: Use this filter if you want to precompile Handlebars templates. - If compiling them in the browser is acceptable, you may use the JST - filter, which needs no external dependency. + .. note:: + Use this filter if you want to precompile Handlebars templates. + If compiling them in the browser is acceptable, you may use the + JST filter, which needs no external dependency. + + .. warning:: + Currently, this filter is not compatible with input filters. Any + filters that would run during the input-stage will simply be + ignored. Input filters tend to be other compiler-style filters, + so this is unlikely to be an issue. """ + # TODO: We should fix the warning above. Either, me make this filter + # support input-processing (we'd have to work with the hunks given to + # us, rather than the original source files), or make webassets raise + # an error if the handlebars filter is combined with an input filter. + # I'm unsure about the best API design. We could support open() + # returning ``True`` to indicate "no input filters allowed" ( + # surprisingly hard to implement) Or, use an attribute to declare + # as much. + name = 'handlebars' options = { 'binary': 'HANDLEBARS_BIN', @@ -28,28 +43,22 @@ class Handlebars(JSTemplateFilter): } max_debug_level = None - # XXX Due to the way this filter works, any other filters applied - # WILL BE IGNORED. Maybe this method should be allowed to return True - # to indicate that the input() processor is not supported. - def open(self, out, source_path, **kw): - self.templates.append(source_path) - # Write back or the cache would not detect changes - out.write(FileHunk(source_path).data()) + def process_templates(self, out, hunks, **kw): + templates = [info['source_path'] for _, info in hunks] - def output(self, _in, out, **kw): if self.root is True: root = self.get_config('directory') elif self.root: root = path.join(self.get_config('directory'), self.root) else: - root = self._find_base_path(self.templates) + root = self._find_base_path(templates) args = [self.binary or 'handlebars'] if root: args.extend(['-r', root]) if self.extra_args: args.extend(self.extra_args) - args.extend(self.templates) + args.extend(templates) proc = subprocess.Popen( args, stdin=subprocess.PIPE, diff --git a/src/webassets/filter/jst.py b/src/webassets/filter/jst.py index bccf7c49..e7e5d1cb 100644 --- a/src/webassets/filter/jst.py +++ b/src/webassets/filter/jst.py @@ -11,10 +11,28 @@ class JSTemplateFilter(Filter): possibly other Javascript templating systems in the future. """ - def setup(self): - super(JSTemplateFilter, self).setup() - # Reset template collection (same instance may run multiple times) - self.templates = [] + def concat(self, out, hunks, **kwargs): + self.process_templates(out, hunks, **kwargs) + + def process_templates(self, out, hunks, **kw): + raise NotImplementedError() + + def iter_templates_with_base(self, hunks): + """Helper that for list of ``hunks``, as given to + ``concat()``, yields 2-tuples of (name, hunk), with name + being the name of the source file relative to the common + prefix of all source files. + + In other words, each template gets the shortest possible + name to identify it. + """ + base_path = self._find_base_path( + [info['source_path'] for _, info in hunks]) + os.path.sep + for hunk, info in hunks: + name = info['source_path'] + name = name[len(base_path):] + name = os.path.splitext(name)[0] + yield name, hunk def _find_base_path(self, paths): """Hmmm. There should aways be some common base path.""" @@ -118,31 +136,22 @@ def setup(self): self.include_jst_script = (self.template_function == 'template') \ or not self.template_function - def input(self, _in, out, source_path, output_path, **kw): - data = _in.read() - self.templates.append((source_path, data)) - - # Write back or the cache would not detect changes - out.write(data) - - def output(self, _in, out, **kwargs): - base_path = self._find_base_path() + os.path.sep + def process_templates(self, out, hunks, **kwargs): namespace = self.namespace or 'window.JST' if self.bare is False: - out.write("(function(){\n ") + out.write("(function(){\n") out.write("%s = %s || {};\n" % (namespace, namespace)) if self.include_jst_script: out.write("%s\n" % _jst_script) - for path, contents in self.templates: + for name, hunk in self.iter_templates_with_base(hunks): # Make it a valid Javascript string. Is this smart enough? - contents = contents.replace('\n', '\\n').replace("'", r"\'") + contents = hunk.data().replace('\n', '\\n').replace("'", r"\'") - out.write("%s['%s'] = " % (namespace, - os.path.splitext(path[len(base_path):])[0])) + out.write("%s['%s'] = " % (namespace, name)) if self.template_function is False: out.write("'%s';\n" % (contents)) else: @@ -152,11 +161,6 @@ def output(self, _in, out, **kwargs): if self.bare is False: out.write("})();") - def _find_base_path(self): - """Hmmm. There should aways be some common base path.""" - paths = [path for path, content in self.templates] - return JSTemplateFilter._find_base_path(self, paths) - _jst_script = 'var template = function(str){var fn = new Function(\'obj\', \'var \ __p=[],print=function(){__p.push.apply(__p,arguments);};\ diff --git a/src/webassets/merge.py b/src/webassets/merge.py index f10b6e73..75271aa7 100644 --- a/src/webassets/merge.py +++ b/src/webassets/merge.py @@ -14,7 +14,7 @@ __all__ = ('FileHunk', 'MemoryHunk', 'merge', 'FilterTool', - 'MoreThanOneFilterError') + 'MoreThanOneFilterError', 'NoFilters') # Log which is used to output low-level information about what the build does. @@ -181,6 +181,10 @@ def __init__(self, message, filters): self.filters = filters +class NoFilters(Exception): + pass + + class FilterTool(object): """Can apply filters to hunk objects, while using the cache. @@ -283,7 +287,7 @@ def apply_func(self, filters, type, args, kwargs=None, cache_key=None): filters = [f for f in filters if getattr(f, type, None)] if not filters: # Short-circuit log.debug('No filters have a "%s" method' % type) - return None + raise NoFilters() if len(filters) > 1: raise MoreThanOneFilterError( diff --git a/tests/test_bundle_build.py b/tests/test_bundle_build.py index eb8f6d1c..f0555870 100644 --- a/tests/test_bundle_build.py +++ b/tests/test_bundle_build.py @@ -411,7 +411,7 @@ def test_concat(self): """ class ConcatFilter(Filter): def concat(self, out, hunks, **kw): - out.write('%%%'.join([h.data() for h in hunks])) + out.write('%%%'.join([h.data() for h, info in hunks])) self.create_files({'a': '1', 'b': '2'}) self.mkbundle('a', 'b', filters=ConcatFilter, output='out').build() assert self.get('out') == '1%%%2' diff --git a/tests/test_bundle_various.py b/tests/test_bundle_various.py index 51ebcd47..feec2f9e 100644 --- a/tests/test_bundle_various.py +++ b/tests/test_bundle_various.py @@ -555,7 +555,7 @@ def test_valid_url(self): def test_invalid_url(self): """If a bundle contains an invalid url, building will raise an error. """ - assert_raises(BuildError, + assert_raises(IOError, self.mkbundle('http://bar', output='out').build) def test_autorebuild_updaters(self):