On Thu, 2007-07-12 at 01:58 -0500, Tom Tobin wrote: > Over at the Lawrence Journal-World, we have a bunch of custom template > tags included in Ellington, our commercial CMS based on Django. One > day, I finally got fed up with writing the same damned boilerplate > code for custom tags over and over again (for parsing arguments, > resolving context variables, etc.); the customtags library was the > result. My wonderfully cool boss
Suck up! :-) > agreed to allow me to contribute > this library back to Django. > > customtags is a higher-level API for template tags that borrows much > of its syntax style from the Django model API; it makes writing new > template tags a breeze (if I may say so ^_^). [...] > > I'd appreciate any and all feedback, both positive and critical. :-) I like the idea of having some helper functions. Should make things easier for people. It might take a while for this particular API to grow on me: all these inner classes usually make me wonder if there's another way. That always takes me a couple of days to think about, so not a lot of coherent comments there yet. A few comments (mostly questions) from an initial reading: (1) When it is ever going to be practical to pass strip_quotes=True to split_contents()? For example, you never use that added functionality in the new code you have here. Since you have quietly added the change that allows strings to be delimited by both single- and double-quotes (something I thought we'd "wontfixed" previously, but I don't care about it too much), it means that if you want to pass the string "foo" as an arg, you can use '"foo"' or "\"foo\"". I'm wondering if that argument could be removed without harm. (2) Some stylistic note, about customtags.py: I always feel that phrasing like "a higher-level API" is awkward in core code. Higher level than what? I'd be tempted to just call it "an API for..." or "a helper API for...". The subsection called "Advantages" is really just "Features", for my money. I guess I'm a little against the idea of outright advocacy in comments, but it's a very minor point, so feel free to ignore this entirely. As I mentioned, I haven't fully grokked the example code yet. (3) What type of arguments do validators take? It looks like oldforms-style, strings only and the user has to convert. Since you already know if something is a string or number earlier (when processing in convert_arguments(), at least), perhaps you could give them the type that was determined at parse time? If so, numbers that are not integers should be Decimal types (not floats), so as not to lose precision. For consistency, I'd probably prefer validators that worked like newforms (and like Adrian has suggested for model validation): methods called "validate_<Argname>", rather than the inner class idea. I realise one advantage of inner classes is avoiding name clashes, but the chance of a name collision here is essentially zero (you would need args called foo and validate_foo and somehow have strong technical objections to calling the latter one validate_foo_ or validate_foo1). (4) The name of customtags.py feels wrong somehow. Maybe taghelpers.py? (5) Need converting to handle Unicode. Tag and filter arguments need not be ASCII. Mostly this is easy. A few of the reg-exps should switch to using \w and being compiled with the re.UNICODE flag. (6) The rewrites in defaulttags.py provide interesting reading, as an example of how to use this. They don't strike me as being particularly shorter, though. Again, I want to let this sink in over a couple more reads and I'm not even sure it's a problem. Mostly noting that it's not an immediately clear win in code complexity, although I realise there are advantages in consistency of argument parsing. Mostly food for thought on the layout of TemplateTag subclasses. (7) Is the "resolve" argument on TagArg really necessary? We tend to always try to resolve at the moment and it works reasonably well. I guess it might be necessary for the "url" and "for" tags? (8) Reading the docs sort of clarifies one of my concerns with all the inner classes: there aren't really any non-inner things on the TemplateTag subclass. (9) If you're going to rewrite a lot of the core tags using this infrastucture (and even if not), it's worth importing the errors and most of the external API into the django.template namespace, as is currently done. I noticed that some of the tests changed, for example, to needing to catching template.customtags.TemplateTagValidationError. Given that custom tags live outside django.template, I would be inclined to make it easier for people on the import front. No real name clashes in the django.template namespace that I can see would prevent this. Regards, Malcolm -- On the other hand, you have different fingers. http://www.pointy-stick.com/blog/ --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
