I have a couple of minor comments and questions. Should be good to go. > === modified file 'z3c/recipe/filetemplate/README.txt' > + >>> cat(sample_buildout, 'bin', 'dosomething.sh') # doctest: +ELLIPSIS > + #!/bin/sh > + Z3C_RECIPE_FILETEMPLATE_BASE=`\ > + readlink -f "$0" 2>/dev/null || \ > + realpath "$0" 2>/dev/null || \ > + type -P "$0" 2>/dev/null` > + Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ${Z3C_RECIPE_FILETEMPLATE_BASE}` > + Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ${Z3C_RECIPE_FILETEMPLATE_BASE}` > + cat "$Z3C_RECIPE_FILETEMPLATE_BASE"/data/info.csv > + The multiple mutation of Z3C_RECIPE_FILETEMPLATE_BASE are confusing. Especially, the two identical Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ...` I assume that's the way to get to the parent directory of the script. Would be clearer to use multiple variables, at the detriment of more namespace polluting. At the very least a comment would help the reader. Your call. > === modified file 'z3c/recipe/filetemplate/__init__.py' > + def _get(self, section, option, start): > + if section is None: > + section = self.recipe.name # This sets up error messages properly. > + if section == self.recipe.name: > + factory = self.recipe.dynamic_options.get(option) > + if factory is not None: > + try: > + return factory(self, start, option) > + except: > + # Argh. Would like to raise wrapped exception. > + colno, lineno = self.get_colno_lineno(start) > + msg = ('Dynamic option %r in line %d, col %d of %s ' > + 'crashed.') % (option, lineno, colno, self.source) > + self.recipe.logger.error(msg, exc_info=True) > + raise > + # else... > + options = self.recipe.options > Shouldn't you catch and re-raise SystemExit and KeyboardInterrupt? Or was this fixed in Python 2.5? > - def substitute(self, recipe, seen): > + def substitute(self): > def convert(mo): > + start = mo.start() > # Check the most common path first. > - option = mo.group('braced_single') > + option = mo.group('option') > if option is not None: > - val = self._get(recipe.options, recipe.name, option, seen, > - mo.start('braced_single')) > + section = mo.group('section') > + val = self._get(section, option, start) > + path_extension = mo.group('path_extension') > + filters = mo.group('filters') > + if path_extension is not None: > + val = os.path.join(val, *path_extension.split('/')[1:]) > + if filters is not None: > + for filter in filters.split('|')[1:]: > + filter = filter.strip() > + if filter not in self.recipe.filters: > + colno, lineno = self.get_colno_lineno(start) > + raise ValueError( > + 'Unknown filter %r ' > + 'in line %d, col %d of %s' % > + (filter, lineno, colno, self.source)) > + try: > + val = self.recipe.filters[filter]( > + val, self, start, filter) > + except: > + # Argh. Would like to raise wrapped exception. > + colno, lineno = self.get_colno_lineno(start) > + msg = ('Filter %r in line %d, col %d of %s ' > + 'crashed processing value %r') % ( > + filter, lineno, colno, self.source, val) > + self.recipe.logger.error(msg, exc_info=True) > + raise Same question about the bare except. > +def _relative_depth(common, path): > + # Helper ripped from zc.buildout.easy_install. Is the copy and paste really necessary? easy_install isn't something that can be imported?