On 7/12/07, Malcolm Tredinnick <[EMAIL PROTECTED]> wrote: > > 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! :-)
::grin:: > (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. Oops; the strip_quotes is a leftover from an earlier form of the code that made use of it, so I'll be yanking it. I have no attachment to what quoting form gets used (single vs. double); I was going by what split_contents() did. I'm fine with standardizing on double-quoting only. > (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...". "Helper API" sounds good to me. > 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. No, it's a valid point; "Advantages" made it through from when I originally wrote it for internal use here. ;-) > (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. I believe I may have already moved towards passing in the converted type without updating the documentation to match; I'll go over the code soon and check, and set things out that way if they're not already. Good point regarding float vs. Decimal; I'll change that. > 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). I'd like to have the library mirror the syntax of other parts of Django as much as possible, so I'll likely go ahead and do this. > (4) The name of customtags.py feels wrong somehow. Maybe taghelpers.py? I'm definitely not stuck on the name. :-) > (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. Yeah, that should be straightforward to fix. > (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. Unfortunately, the tags in defaulttags.py are probably the worst examples in terms of brevity gains. :-) Most of our converted internal tags are much shorter, since they tend to be fairly simple and all of the boilerplate argument handling code (e.g., " if bit[2] == 'foo' ") gets hidden away. > (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? Now that I've moved toward separating strings and "keywords" out as separate types, you may be right; perhaps the library should always attempt to resolve on a keyword argument. The only issue here becomes what to do if resolution on a keyword fails -- do we set it to None (as ifequal / ifnotequal currently expects), or set it to the string value of the keyword? > (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. Yeah -- anything not in an inner class is assumed to be an argument. > (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. Okay. > (10) On my radar to commit fairly soon is one of Brian Harring's > speed-ups for variable resolution -- #3453. It looks like this won't > have any real effect on your design or implementation (some changes > required, but they're routine). However, if you have 15 minutes to read > through the patch on that ticket and think about if there are any > collisions of concepts, I'd appreciate a second set of eyes. Okay, I'll take a look. > Regards, > Malcolm Thanks for going through the patch; this is exactly the sort of thing I wanted. :-) --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
