Re: smart if tag
On Wednesday 09 December 2009 12:43:04 Russell Keith-Magee wrote: > I've done another review, and it's looking pretty good. I have > three relatively minor comments: > > * There is what appears to be vestigial documentation around line > 346 of ref/templates/builtins.txt; it says evaluation order is > left-to-right, rather than operator precedence as described later > on. Thanks, changed. > * Most of the lambdas you have defined in smartif.py are already > available in the builtin operator module - e.g., operator.or_, > operator.and_, operator.gt, etc. Oh yeah, forgot about those! > * The handling of nodelist_false in IfNode isn't wrong (as in, it > doesn't generate wrong results), but it was contrary to my > expectations. I would have expected a simple "else: > self.nodelist_false.render()". Is there a particular reason for > this? I made the change in source and it didn't break any tests. > > The only two reasons I could come up with to explain this approach > were that it would be marginally faster to return '' rather than > render an empty nodelist, and/or an old-school mandate that every > function should have a return statement at the end. I'm not > necessarily saying this should be changed; but at the very least, > it warrants an explanatory comment as to why the obvious approach > isn't being used. I don't know the reason either, it's part of Chris' original patch, and I've simplified it now. > This is a hard one. Keeping with our tradition of disagreeing with > each other, I'm actually inclined to go the other way :-) > > I have two reasons. Firstly, I'm of the opinion that templates > shouldn't ever throw exceptions during rendering - they should fail > silently. Differentiating between False and Exception may be > difficult or inconvenient for the developer, but it's even more > inconvenient for the end user to see a 500 IMHO. You're right, and I was misreading the current code in thinking that it currently raises exceptions during rendering. While Variable and others raise exceptions, they are caught higher up. (There are just occasional places where builtins propagate exceptions, like {% url %}). > Secondly, while I can see the analog with silent_variable_failure, > it isn't quite the same circumstances. Most of the interesting > functions that will be invoked in templates will be internal to > Django or user-code, so we have almost full control over the > silent_variable_failure attribute. We can set > silent_variable_failure on DoesNotExist because it is internal to > Django, and it will behave nicely when errors occur in templates. > > However, to my reckoning, this isn't true for operators. Many, if > not most of the interesting use cases for operators will be > comparisons with literals, sets, lists, or other Python builtins - > which throw exceptions which we can't control. We may well honor a > silent_variable_failure attribute, but if it's near impossible to > actually set that attribute in practice, there isn't much point in > honoring it in the first place. Very good point. You convinced me easily, contrary to tradition this time :-) Thanks for the review again. Luke -- If you can't answer a man's arguments, all is not lost; you can still call him vile names. (Elbert Hubbard) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Wed, Dec 9, 2009 at 9:17 AM, Luke Plant wrote: > Hi all, > > I've now addressed everything in Russell's last e-mail, I think, so I > think I'm pretty much good to go, apart from: > > 1) my last change rewrote a lot of IfParser, which was the heart of > the patch. That means it probably needs looking at again! On the other > hand, the new implementation is based on a much more general and more > tested parser methodology, so it should hopefully be pretty solid. I've done another review, and it's looking pretty good. I have three relatively minor comments: * There is what appears to be vestigial documentation around line 346 of ref/templates/builtins.txt; it says evaluation order is left-to-right, rather than operator precedence as described later on. * Most of the lambdas you have defined in smartif.py are already available in the builtin operator module - e.g., operator.or_, operator.and_, operator.gt, etc. * The handling of nodelist_false in IfNode isn't wrong (as in, it doesn't generate wrong results), but it was contrary to my expectations. I would have expected a simple "else: self.nodelist_false.render()". Is there a particular reason for this? I made the change in source and it didn't break any tests. The only two reasons I could come up with to explain this approach were that it would be marginally faster to return '' rather than render an empty nodelist, and/or an old-school mandate that every function should have a return statement at the end. I'm not necessarily saying this should be changed; but at the very least, it warrants an explanatory comment as to why the obvious approach isn't being used. Other than the issue you raise below, I'm happy. In particular, I'm much happier with the parsing code now - I found it much less confusing that the previous iteration, and it has the added benefit of being correct :-) > 2) what should be done about errors with operators? For the most > part, it isn't much of an issue, as 'object' defines many of the magic > methods involved. For 'in', however, you are likely to get TypeErrors > if the object doesn't support it. Currently missing variables are > handled gracefully, but {% if 'x' in 1 %} will get you a TypeError > that is not caught. And there is still the possibility that other > operators will cause errors. > > Our current policies don't quite handle this situation, AFAICS. For > the case of 'in', catching any Exception and returning False could be > defended - to use the above example, 1 is not an object that contains > 'x', therefore it should return False. This would protect template > authors against various exceptions. > > Or we could go with the policy for method calls, which is to check the > exception for 'silent_variable_failure' attribute. This has the > advantage that inappropriate use of 'in' by template authors, which is > almost always going to be a mistake, will throw a loud error. The > error message in this case isn't particularly obvious though: > "TypeError: argument of type 'int' is not iterable" > > I'm leaning towards the latter. If a view changes so that, in the > variables passed to the template, a container with a __contains__() > method is replaced by one without, or by one with a buggy > __contains__() method that throws exceptions, it's more useful and > arguably more correct to get an exception than for the 'if' tag > expression to return False. This is a hard one. Keeping with our tradition of disagreeing with each other, I'm actually inclined to go the other way :-) I have two reasons. Firstly, I'm of the opinion that templates shouldn't ever throw exceptions during rendering - they should fail silently. Differentiating between False and Exception may be difficult or inconvenient for the developer, but it's even more inconvenient for the end user to see a 500 IMHO. Secondly, while I can see the analog with silent_variable_failure, it isn't quite the same circumstances. Most of the interesting functions that will be invoked in templates will be internal to Django or user-code, so we have almost full control over the silent_variable_failure attribute. We can set silent_variable_failure on DoesNotExist because it is internal to Django, and it will behave nicely when errors occur in templates. However, to my reckoning, this isn't true for operators. Many, if not most of the interesting use cases for operators will be comparisons with literals, sets, lists, or other Python builtins - which throw exceptions which we can't control. We may well honor a silent_variable_failure attribute, but if it's near impossible to actually set that attribute in practice, there isn't much point in honoring it in the first place. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com.
Re: smart if tag
Hi all, I've now addressed everything in Russell's last e-mail, I think, so I think I'm pretty much good to go, apart from: 1) my last change rewrote a lot of IfParser, which was the heart of the patch. That means it probably needs looking at again! On the other hand, the new implementation is based on a much more general and more tested parser methodology, so it should hopefully be pretty solid. 2) what should be done about errors with operators? For the most part, it isn't much of an issue, as 'object' defines many of the magic methods involved. For 'in', however, you are likely to get TypeErrors if the object doesn't support it. Currently missing variables are handled gracefully, but {% if 'x' in 1 %} will get you a TypeError that is not caught. And there is still the possibility that other operators will cause errors. Our current policies don't quite handle this situation, AFAICS. For the case of 'in', catching any Exception and returning False could be defended - to use the above example, 1 is not an object that contains 'x', therefore it should return False. This would protect template authors against various exceptions. Or we could go with the policy for method calls, which is to check the exception for 'silent_variable_failure' attribute. This has the advantage that inappropriate use of 'in' by template authors, which is almost always going to be a mistake, will throw a loud error. The error message in this case isn't particularly obvious though: "TypeError: argument of type 'int' is not iterable" I'm leaning towards the latter. If a view changes so that, in the variables passed to the template, a container with a __contains__() method is replaced by one without, or by one with a buggy __contains__() method that throws exceptions, it's more useful and arguably more correct to get an exception than for the 'if' tag expression to return False. Regards, Luke -- "If something is hard, it's not worth doing." (Homer Simpson) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
The undcoumented scanner class in the regex module may interest you: http://code.activestate.com/recipes/457664/ Alex On Sun, Dec 6, 2009 at 3:32 AM, SmileyChris wrote: > Because that link intrigued me, I challenged myself to write my own > generic lexer & parser based on what I had read: > http://gist.github.com/250128 > > > > On Dec 6, 2:07 pm, Luke Plant wrote: >> On Saturday 05 December 2009 20:09:21 Luke Plant wrote: >> >> > I'm not likely to able to look at this before Tuesday. If anyone >> > wants to look at it, I think the right approach is something like >> > the following: >> >http://effbot.org/zone/simple-top-down-parsing.htm >> > (without the globals, obviously, they can be converted to instance >> > variables/methods). >> >> Cancel that - I unexpectedly had free time this evening, and I >> implemented this. It's a nice replacement I think (credit to Vaughan >> Pratt and Fredrik Lundh for the basic approach and Python >> implementation respectively). The new implementation is pretty much >> the same length as the old one, and hopefully easier to read, with a >> much smaller core parser. Precedence is specified directly, rather >> than implicitly, so it's much easier to check that it's the same as >> Python's. >> >> Latest patch attached to this e-mail. >> >> Regards, >> >> Luke >> >> -- >> "Idiocy: Never underestimate the power of stupid people in large >> groups." (despair.com) >> >> Luke Plant ||http://lukeplant.me.uk/ >> >> smartif_8.diff >> 27KViewDownload > > -- > > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-develop...@googlegroups.com. > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > > -- "I disapprove of what you say, but I will defend to the death your right to say it." -- Voltaire "The people's good is the highest law." -- Cicero "Code can always be simpler than you think, but never as simple as you want" -- Me -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
Because that link intrigued me, I challenged myself to write my own generic lexer & parser based on what I had read: http://gist.github.com/250128 On Dec 6, 2:07 pm, Luke Plant wrote: > On Saturday 05 December 2009 20:09:21 Luke Plant wrote: > > > I'm not likely to able to look at this before Tuesday. If anyone > > wants to look at it, I think the right approach is something like > > the following: > >http://effbot.org/zone/simple-top-down-parsing.htm > > (without the globals, obviously, they can be converted to instance > > variables/methods). > > Cancel that - I unexpectedly had free time this evening, and I > implemented this. It's a nice replacement I think (credit to Vaughan > Pratt and Fredrik Lundh for the basic approach and Python > implementation respectively). The new implementation is pretty much > the same length as the old one, and hopefully easier to read, with a > much smaller core parser. Precedence is specified directly, rather > than implicitly, so it's much easier to check that it's the same as > Python's. > > Latest patch attached to this e-mail. > > Regards, > > Luke > > -- > "Idiocy: Never underestimate the power of stupid people in large > groups." (despair.com) > > Luke Plant ||http://lukeplant.me.uk/ > > smartif_8.diff > 27KViewDownload -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
uses with the same tag, -because the order of logic would be ambigous. For example, this is -invalid:: +Comparison operators are also available, and the use of filters is also +allowed, for example: -{% if athlete_list and coach_list or cheerleader_list %} +{% if articles|length >= 5 %}...{% endif %} -If you need to combine ``and`` and ``or`` to do advanced logic, just use -nested if tags. For example:: +Arguments and operators _must_ have a space between them, so +``{% if 1>2 %}`` is not a valid if tag. -{% if athlete_list %} -{% if coach_list or cheerleader_list %} -We have athletes, and either coaches or cheerleaders! -{% endif %} -{% endif %} +All supported operators are: ``or``, ``and``, ``in``, ``=`` (or ``==``), +``!=``, ``>``, ``>=``, ``<`` and ``<=``. + +Operator precedence follows Python. """ -bits = token.contents.split() -del bits[0] -if not bits: -raise TemplateSyntaxError("'if' statement requires at least one argument") -# Bits now looks something like this: ['a', 'or', 'not', 'b', 'or', 'c.d'] -bitstr = ' '.join(bits) -boolpairs = bitstr.split(' and ') -boolvars = [] -if len(boolpairs) == 1: -link_type = IfNode.LinkTypes.or_ -boolpairs = bitstr.split(' or ') -else: -link_type = IfNode.LinkTypes.and_ -if ' or ' in bitstr: -raise TemplateSyntaxError, "'if' tags can't mix 'and' and 'or'" -for boolpair in boolpairs: -if ' ' in boolpair: -try: -not_, boolvar = boolpair.split() -except ValueError: -raise TemplateSyntaxError, "'if' statement improperly formatted" -if not_ != 'not': -raise TemplateSyntaxError, "Expected 'not' in if statement" -boolvars.append((True, parser.compile_filter(boolvar))) -else: -boolvars.append((False, parser.compile_filter(boolpair))) +bits = token.split_contents()[1:] +var = TemplateIfParser(parser, bits).parse() nodelist_true = parser.parse(('else', 'endif')) token = parser.next_token() if token.contents == 'else': @@ -853,7 +831,7 @@ parser.delete_first_token() else: nodelist_false = NodeList() - return IfNode(boolvars, nodelist_true, nodelist_false, link_type) +return IfNode(var, nodelist_true, nodelist_false) do_if = register.tag("if", do_if) #...@register.tag diff -r 70e75e8cd224 django/template/smartif.py --- /dev/null Thu Jan 01 00:00:00 1970 + +++ b/django/template/smartif.py Sun Dec 06 01:04:04 2009 + @@ -0,0 +1,181 @@ +""" +Parser and utilities for the smart 'if' tag +""" + +# Using a simple top down parser, as described here: +#http://effbot.org/zone/simple-top-down-parsing.htm. +# 'led' = left denotation +# 'nud' = null denotation +# 'bp' = binding power (left = lbp, right = rbp) + +class TokenBase(object): +""" +Base class for operators and literals, mainly for debugging and for throwing +syntax errors. +""" +id = None # node/token type name +value = None # used by literals +first = second = None # used by tree nodes + +def nud(self, parser): +# Null denotation - called in prefix context +raise parser.error_class( +"Not expecting '%s' in this position in if tag." % self.id +) + +def led(self, left, parser): +# Left denotation - called in infix context +raise parser.error_class( +"Not expecting '%s' as infix operator in if tag." % self.id +) + +def display(self): +""" +What to display in error messages for this node +""" +return self.id + +def __repr__(self): +if self.id == "literal": +return "(%s %r)" % (self.id, self.value) +out = [self.id, self.first, self.second] +out = map(str, filter(None, out)) +return "(" + " ".join(out) + ")" + + +def infix(bp, func): +""" +Create an infix operator, given a binding power and a function that +evaluates the node +""" +class Operator(TokenBase): +lbp = bp + +def led(self, left, parser): +self.first = left +self.second = parser.expression(bp) +return self + +def resolve(self, context): +return func(self.first.resol
Re: smart if tag
First, thanks for your review Russell, I appreciate that you've been doing tons of work on lots of things! On Saturday 05 December 2009 15:39:03 Russell Keith-Magee wrote: > Here's the review I promised. First the minor points: > > * Line 814 of templates/defaulttags.py has a wierd UTF-8 character > - an emdash, rather than an ASCII hyphen. I don't know what's > happening on your system, but this causes crashes for me > complaining about PEP-0263 problems. I did add a 'coding' line to fix that, but perhaps after you pulled my latest code. Anyway, it's not worth it so I removed it. > * The if-tag-not-02/05 tests that have been removed should be > retained, but be testing for the TemplateSyntaxError (just like the > if-tag-error tests do). If we're going to implement a new language > contract and introduce some reserved words, we should test that > that those words are actually reserved. There are actually other tests that check this, in the IfParser tests. I didn't see the usefulness of 5 different tests all checking that 'not' is a keyword, but renumbering the following tests would be worse, hence the note about why they were missing. Also, it makes more sense to put the syntax error tests together. > * I'm not wild about the docs for the if tag referring how > comparisons work "just like Python operators". We have historically > made a big deal of templates not being Python, and while we are > blurring the lines a little with this patch, I'd still like to > maintain the pretense. The preceding notes about logical operators > is a good example of how it should be done IMHO - it gives > examples of how and/or work without needing to resort to saying > "just like Python". Fixed now, hopefully, I've given examples for all the operators. > * The explanation of why {% if a > b > c %} doesn't work is a bit > vague. At the very least, it should have an example of the correct > implmentation - {% if a > b and b > c %}. Fixed now. > Now the major issues: There's only one that I found - the parsing > strategy for complex logical expressions. "A or B and C" is parsed > as "(A or B) and C", not the pythonic way "A or (B and C)" (giving > operator precedence to AND over OR). > > Personally, I found this very surprising. When I said in a previous > email that I didn't think it was worth hobbling the if statement to > prevent complex logical operations, I presumed it was going to be > replaced with a full parser. Historically, it hasn't been an issue > because we've only allowed one type of logical operator in an {% if > %}. I think I'd rather see this tradition continued rather than > allow mixing in a way that will be confusing to anyone with prior > programming experience. Good catch. I think it would be nicer to fix than just disallow, although it could be a substantial change to the way the IfParser class works. I don't think it is too much work, however, because the hard bit is tokenizing, which has already been done. I'm not likely to able to look at this before Tuesday. If anyone wants to look at it, I think the right approach is something like the following: http://effbot.org/zone/simple-top-down-parsing.htm (without the globals, obviously, they can be converted to instance variables/methods). I don't think trying to hack this on to the current IfParser would be a good idea. IfParser already has a slightly hacky way of handling the precedence of 'and' and 'or' relative to the other operators, and this is a good opportunity to replace that with something nicer. > Lastly, a controversial topic, but I think we need an answer on > this - whither {% ifequal %}? Should we be deprecating it? Given > the current level of usage of the ifequal/ifnotequal tags, this > seems excessive. Perhaps we should consider this for long-term > deprecation (i.e., flag it as a 2.0 deprecation)? Yes, I see no value in deprecating this until 2.0, given how much it is used. However, while I remember, there is this bug which we may need to revisit: http://code.djangoproject.com/ticket/10975 Essentially, you can't use {% block %} inside an {% if %} tag, but you can in an {% ifequal %} tag. Irrespective of which one is right, it seems unreasonable that {% if x == y %} behaves so differently from {% ifequal x y %} in this regard. (Actually, I didn't test that. Perhaps the behaviour of 'if' has changed with the smart if code). Regards, Luke -- "Idiocy: Never underestimate the power of stupid people in large groups." (despair.com) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Tue, Dec 1, 2009 at 9:55 AM, Russell Keith-Magee wrote: > On Tue, Dec 1, 2009 at 1:08 AM, Luke Plant wrote: >> Patch >> = >> >> Review would be welcome, especially as I'm ill at the moment. I'm only >> coding because the boredom of doing nothing is killing me... > > I'll try and take a look over the next couple of days. Here's the review I promised. First the minor points: * Line 814 of templates/defaulttags.py has a wierd UTF-8 character - an emdash, rather than an ASCII hyphen. I don't know what's happening on your system, but this causes crashes for me complaining about PEP-0263 problems. * The if-tag-not-02/05 tests that have been removed should be retained, but be testing for the TemplateSyntaxError (just like the if-tag-error tests do). If we're going to implement a new language contract and introduce some reserved words, we should test that that those words are actually reserved. * I'm not wild about the docs for the if tag referring how comparisons work "just like Python operators". We have historically made a big deal of templates not being Python, and while we are blurring the lines a little with this patch, I'd still like to maintain the pretense. The preceding notes about logical operators is a good example of how it should be done IMHO - it gives examples of how and/or work without needing to resort to saying "just like Python". * The explanation of why {% if a > b > c %} doesn't work is a bit vague. At the very least, it should have an example of the correct implmentation - {% if a > b and b > c %}. * The interaction between this patch and #6262 will be fun. (This is more a mental note to myself that I should commit #6262) Now the major issues: There's only one that I found - the parsing strategy for complex logical expressions. "A or B and C" is parsed as "(A or B) and C", not the pythonic way "A or (B and C)" (giving operator precedence to AND over OR). Personally, I found this very surprising. When I said in a previous email that I didn't think it was worth hobbling the if statement to prevent complex logical operations, I presumed it was going to be replaced with a full parser. Historically, it hasn't been an issue because we've only allowed one type of logical operator in an {% if %}. I think I'd rather see this tradition continued rather than allow mixing in a way that will be confusing to anyone with prior programming experience. Lastly, a controversial topic, but I think we need an answer on this - whither {% ifequal %}? Should we be deprecating it? Given the current level of usage of the ifequal/ifnotequal tags, this seems excessive. Perhaps we should consider this for long-term deprecation (i.e., flag it as a 2.0 deprecation)? Russ %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Tue, Dec 1, 2009 at 1:08 AM, Luke Plant wrote: > On Monday 30 November 2009 11:18:13 Russell Keith-Magee wrote: > >> > 1) Handling non-existent variables >> > == > So, I'm *now* suggesting that we convert everything missing to None. Sounds good to me. > Behaviour of 'not' - revisited > == > > I propose a backwards incompatibility here, it looks like this: > > The 'if' tag no longer accepts 'and', 'or' and 'not' as valid > variable names. Previously that was allowed in some cases even > though they were normally treated as keywords. Now, the keyword > status is always enforced, and code like {% if not %} or {% if and %} > will throw a TemplateSyntaxError. > > This would now be the only backwards incompatibility (discounting the > possibility that people were relying on TemplateSyntaxError for things > that are now legal). Also sounds good. > Patch > = > > Review would be welcome, especially as I'm ill at the moment. I'm only > coding because the boredom of doing nothing is killing me... I'll try and take a look over the next couple of days. Russ %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Monday 30 November 2009 20:27:32 SmileyChris wrote: > On Dec 1, 6:08 am, Luke Plant wrote: > > Given that the 'Null' stuff has now been removed, we could > > move back to your way to reduce the code a bit, but I'm not sure > > it is worth it. > > I'd say reduce the code again. I've removed Less, LessOrEqual and NotEqual. I kept 'Not' as I think it's slightly cleaner than putting 'negate' into BaseCalc. It also removes the asymmetry of using 'Or' at one point in the code, which I found slightly puzzling. > And I think you missed adding some files in your repo? Doh! Added now, along with some docs. Regards, Luke -- "Humiliation: The harder you try, the dumber you look." (despair.com) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Dec 1, 6:08 am, Luke Plant wrote: > Given that the 'Null' stuff has now been removed, we could > move back to your way to reduce the code a bit, but I'm not sure it is > worth it. I'd say reduce the code again. And I think you missed adding some files in your repo? -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Dec 1, 6:08 am, Luke Plant wrote: > > I was with you right up until the equality comparisons (Null == > > Null -> False etc). As noted by Alex, it conflicts with the answer > > given by {% ifequal %}; it also differs with Python's behavior. > > Yeah, I hadn't thought of that. I don't think it's a case of differing > with Python's behaviour, as we are making a deliberate choice to > substitute a missing variable with None (or Null or whatever). You > could argue that Python throws a NameError in this case. Personally, I think that "(not found)" should *not* equal "None", even if that differs from ifequal. But whatever, I'm not too worried either way. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Monday 30 November 2009 11:18:13 Russell Keith-Magee wrote: > > 1) Handling non-existent variables > > == ... > > To do this properly, I think the best approach would be something > > like "almost three valued logic", which would work as follows: > > > > 1) variables that don't exist are represented by Null > > 2) all operators return either True or False, never Null > > 3) for 'not', 'and' and 'or', Null effectively acts like False > > (so "not Null" == True, "Null or True" == True etc) > > 4) for all comparison operators, if either argument is Null then > > the result is False e.g. "Null == Null" is False > > "Null > 1" is False, "Null <= 1" is False, except > > "!=", which should return true if either value is Null. > > > > Any comments on this approach? I've started to implement this, > > and it works OK so far, just needs a bit more work. > > I was with you right up until the equality comparisons (Null == > Null -> False etc). As noted by Alex, it conflicts with the answer > given by {% ifequal %}; it also differs with Python's behavior. Yeah, I hadn't thought of that. I don't think it's a case of differing with Python's behaviour, as we are making a deliberate choice to substitute a missing variable with None (or Null or whatever). You could argue that Python throws a NameError in this case. I was trying to think about what is intuitive. e.g. what should happen here if foo has not been supplied: {% if foo < 1 %} yes {% endif %} I think it is fairly counter-intuitive for a template author that you get 'yes' here. However, on further reflection, the "almost 3 valued logic" can be equally counter-intuitive, say with the following case: {% if foo < 1 %} yes {% else %} no {% endif %} It would render 'no' if foo was missing, but it might be quite surprising that if you decided to re-arrange the template by inverting the logical condition: {% if foo >= 1 %} no {% else %} yes {% endif %} then you get different behaviour. (i.e. 'yes' if foo was missing). So, I'm *now* suggesting that we convert everything missing to None. In fact, I've found that doing anything apart from this would be hard. (Alex, you were right about what Malcolm had done with FilterExpression, which resolves the problem with TEMPLATE_STRING_IF_INVALID, but this gives me None instead of '', and not information about whether the variable actually exists, which is a slightly different question. I didn't find this out earlier because most of the tests are against the 'IfParser' which is a subclass of 'TemplateIfParser', and works slightly differently, and because I wasn't thinking properly). Behaviour of 'not' - revisited == I think this leaves just the behaviour of 'not'. It is a bit more complicated than Alex suggested — the tests include "if not" and "if not not", and with the current behaviour you can do things like "if foo and not not" etc. In fact, you can do it (to some extent) with *any* of the keywords e.g. "if and", "if x and or". But not always e.g. "if not or and x" - that, illogically, is a syntax error. I suspect that allowing everything that was possible before will be extremely difficult, because the current behaviour is very badly defined for these edge cases — it works just fine if you do sensible things like not using variables called 'not', 'and' or 'or', but working out the rules for the exceptional cases will be very hard. I propose a backwards incompatibility here, it looks like this: The 'if' tag no longer accepts 'and', 'or' and 'not' as valid variable names. Previously that was allowed in some cases even though they were normally treated as keywords. Now, the keyword status is always enforced, and code like {% if not %} or {% if and %} will throw a TemplateSyntaxError. This would now be the only backwards incompatibility (discounting the possibility that people were relying on TemplateSyntaxError for things that are now legal). Patch = It can be tracked in my 'lp-smartif' branch here: http://bitbucket.org/spookylukey/django-trunk-lukeplant/ Currently various tests about 'not' are failing, and docs are not written. Chris: in the course of my attempted 'Null' behaviour changes, I changed your implementation slightly — I added explicit classes for Less, LessOrEqual, NotEqual and Not, pulling out the 'negate' behaviour from the individual classes (it made implementing the Null logic simpler). Other than that, very little of the parsing has changed. Given that the 'Null' stuff has now been removed, we could move back to your way to reduce the code a bit, but I'm not sure it is worth it. Review would be welcome, especially as I'm ill at the moment. I'm only coding because the boredom of doing nothing is killing me... Luke -- "Humiliation: The harder you try, the dumber you look." (despair.com) Luke Plant || http://lukeplant.me.uk/ -- You received this me
Re: smart if tag
On Sun, Nov 29, 2009 at 1:40 AM, Luke Plant wrote: > Hi all, > > I started work replacing Django's if with the "smart-if" template tag > by Chris Beaven ( http://www.djangosnippets.org/snippets/1350/ ) > > Of course, it is not as simple as it looks... > > 4 issues: > > 1) Handling non-existent variables > == > > The current smart-if does not handle the case of non-existent > variables like Django does. (For example, something like > > {% if foo or bar %} > > when one of foo or bar is not defined in the context.) > > To do this properly, I think the best approach would be something like > "almost three valued logic", which would work as follows: > > 1) variables that don't exist are represented by Null > 2) all operators return either True or False, never Null > 3) for 'not', 'and' and 'or', Null effectively acts like False > (so "not Null" == True, "Null or True" == True etc) > 4) for all comparison operators, if either argument is Null then > the result is False e.g. "Null == Null" is False > "Null > 1" is False, "Null <= 1" is False, except > "!=", which should return true if either value is Null. > > Any comments on this approach? I've started to implement this, and it > works OK so far, just needs a bit more work. I was with you right up until the equality comparisons (Null == Null -> False etc). As noted by Alex, it conflicts with the answer given by {% ifequal %}; it also differs with Python's behavior. > 2) TEMPLATE_STRING_IF_INVALID breaks everything > === > > The smart 'if' tag allows for filters: > > {% if articles|length >= 5 %} > > To achieve this, it puts its arguments through FilterExpression. If > settings.TEMPLATE_STRING_IS_INVALID is not an empty string, this means > that: > > {% if foo %}blah{% endif %} > > will render 'blah' instead of '' if 'foo' is not in the context. This > causes test failures at the moment. We could change the tests, but > that is a backwards incompatibility, and it highlights a fundamental > change in the behaviour of {% if %}, which basically makes > TEMPLATE_STRING_IF_INVALID useless for even its stated purpose of > debugging, because the logic of a template will now be changed. > > Personally, I care nothing for TEMPLATE_STRING_IF_INVALID, as there > are other things that it breaks, and I never use it. But obviously I > can't just think of myself here. > > If there was an easy solution that didn't change the behaviour of the > if tag in this case that would be great, but I can't see one. The resolve call on FilterExpression takes an "ignore_failures" argument that exists for exactly this purpose - contrib.comments uses this setting to ensure that generic content type lookups don't end up with spurious data. I suspect the same argument should be usable here, too. > 3) Behaviour of 'x and b or y' > == > > Previously this was a syntax error. In Chris's code, it is fine and > works like Python. I would vote for keeping the smart-if as it is, as > I don't see the value of complicating the implementation to disallow > something that could be useful sometimes. I agree. While I still think having excessive logic in the template is a bad idea, I don't see too much harm in providing the capability on the off chance it is legitimately useful. This is double so given that preventing complex logic will complicate the implementation, and almost inevitably cause grief from those users that don't understand why the limitation is in place. The kink in this particular plan is that it will require clearly documenting the order of evaluation of logical operators - especially since parentheses aren't available for explicit clarification. > 4) Behaviour of 'not' > = > Current Django will interpret 'not' as a variable if there is no > ambiguity e.g. > > {% if not %}blah{% endif %} > > This is not documented, but the behaviour is specified in detail in > the tests, so changing it would be a backwards incompatibility. The > smart-if is different (the above is a syntax error), and I suspect > that making the smart-if behave like current Django could complicate > things a lot. > > IMO, current Django behaviour here is complete misfeature! I don't > know of any other language where keywords can be treated as variables > if the keyword doesn't make sense in that position... I completely agree that this is a nasty wart, bu
Re: smart if tag
On Nov 29, 6:40 am, Luke Plant wrote: > Hi all, > > I started work replacing Django's if with the "smart-if" template tag > by Chris Beaven (http://www.djangosnippets.org/snippets/1350/) Neat! I'm assuming you'll be posting this to your bitbucket soon? :) > 1) Handling non-existent variables > 2) TEMPLATE_STRING_IF_INVALID breaks everything Yep, this is a bit of a problem which I've been thinking about recently (encountered it for a project). My (non-implemented) solution to both of these is to use a NotFound object. I just coded one up: http://gist.github.com/244861 This could actually be a much better return value than a plain TEMPLATE_STRING_IF_INVALID string in general. > 3) Behaviour of 'x and b or y' Like I commented in the snippet, I don't think it's worth obfuscating code to limit this behaviour. > 4) Behaviour of 'not' Bleh, whatever. If we need it for backwards compatibility, it's not hard to implement. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: smart if tag
On Sat, Nov 28, 2009 at 11:40 AM, Luke Plant wrote: > Hi all, > > I started work replacing Django's if with the "smart-if" template tag > by Chris Beaven ( http://www.djangosnippets.org/snippets/1350/ ) > > Of course, it is not as simple as it looks... > > 4 issues: > > 1) Handling non-existent variables > == > > The current smart-if does not handle the case of non-existent > variables like Django does. (For example, something like > > {% if foo or bar %} > > when one of foo or bar is not defined in the context.) > > To do this properly, I think the best approach would be something like > "almost three valued logic", which would work as follows: > > 1) variables that don't exist are represented by Null > 2) all operators return either True or False, never Null > 3) for 'not', 'and' and 'or', Null effectively acts like False > (so "not Null" == True, "Null or True" == True etc) > 4) for all comparison operators, if either argument is Null then > the result is False e.g. "Null == Null" is False > "Null > 1" is False, "Null <= 1" is False, except > "!=", which should return true if either value is Null. > > Any comments on this approach? I've started to implement this, and it > works OK so far, just needs a bit more work. > Wouldn't that be a break from how {% ifequal %} works? Unless I'm mistaken if you have a nonexistant value there it becomes None, which is equal to None. > 2) TEMPLATE_STRING_IF_INVALID breaks everything > === > > The smart 'if' tag allows for filters: > > {% if articles|length >= 5 %} > > To achieve this, it puts its arguments through FilterExpression. If > settings.TEMPLATE_STRING_IS_INVALID is not an empty string, this means > that: > > {% if foo %}blah{% endif %} > > will render 'blah' instead of '' if 'foo' is not in the context. This > causes test failures at the moment. We could change the tests, but > that is a backwards incompatibility, and it highlights a fundamental > change in the behaviour of {% if %}, which basically makes > TEMPLATE_STRING_IF_INVALID useless for even its stated purpose of > debugging, because the logic of a template will now be changed. > I'm almost positive Malcolm did some refactoring in this area that lets you get something other than TEMPLATE_STRING_IF_INVALID for an invlaid FilterExpression. > Personally, I care nothing for TEMPLATE_STRING_IF_INVALID, as there > are other things that it breaks, and I never use it. But obviously I > can't just think of myself here. > > If there was an easy solution that didn't change the behaviour of the > if tag in this case that would be great, but I can't see one. > > 3) Behaviour of 'x and b or y' > == > > Previously this was a syntax error. In Chris's code, it is fine and > works like Python. I would vote for keeping the smart-if as it is, as > I don't see the value of complicating the implementation to disallow > something that could be useful sometimes. > +1 on allowing that. > 4) Behaviour of 'not' > = > Current Django will interpret 'not' as a variable if there is no > ambiguity e.g. > > {% if not %}blah{% endif %} > > This is not documented, but the behaviour is specified in detail in > the tests, so changing it would be a backwards incompatibility. The > smart-if is different (the above is a syntax error), and I suspect > that making the smart-if behave like current Django could complicate > things a lot. > > IMO, current Django behaviour here is complete misfeature! I don't > know of any other language where keywords can be treated as variables > if the keyword doesn't make sense in that position... > It's kind of wacky behavior, agreed. But if should be sort of trivial to implement right? Just if len(bits) == 1 and bits[0] == "not". Or is this expected to work in cases like "if a and not". If the latter is expected to work than it'll be a pain I think. > Regards, > > Luke > > -- > "He knows the way I take: when he has tried me, I shall come forth > as gold" (Job 23:10). > > Luke Plant || http://lukeplant.me.uk/ > > -- > > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-develop...@googlegroups.com. > To unsubscribe from this group, send email to > django
smart if tag
Hi all, I started work replacing Django's if with the "smart-if" template tag by Chris Beaven ( http://www.djangosnippets.org/snippets/1350/ ) Of course, it is not as simple as it looks... 4 issues: 1) Handling non-existent variables == The current smart-if does not handle the case of non-existent variables like Django does. (For example, something like {% if foo or bar %} when one of foo or bar is not defined in the context.) To do this properly, I think the best approach would be something like "almost three valued logic", which would work as follows: 1) variables that don't exist are represented by Null 2) all operators return either True or False, never Null 3) for 'not', 'and' and 'or', Null effectively acts like False (so "not Null" == True, "Null or True" == True etc) 4) for all comparison operators, if either argument is Null then the result is False e.g. "Null == Null" is False "Null > 1" is False, "Null <= 1" is False, except "!=", which should return true if either value is Null. Any comments on this approach? I've started to implement this, and it works OK so far, just needs a bit more work. 2) TEMPLATE_STRING_IF_INVALID breaks everything === The smart 'if' tag allows for filters: {% if articles|length >= 5 %} To achieve this, it puts its arguments through FilterExpression. If settings.TEMPLATE_STRING_IS_INVALID is not an empty string, this means that: {% if foo %}blah{% endif %} will render 'blah' instead of '' if 'foo' is not in the context. This causes test failures at the moment. We could change the tests, but that is a backwards incompatibility, and it highlights a fundamental change in the behaviour of {% if %}, which basically makes TEMPLATE_STRING_IF_INVALID useless for even its stated purpose of debugging, because the logic of a template will now be changed. Personally, I care nothing for TEMPLATE_STRING_IF_INVALID, as there are other things that it breaks, and I never use it. But obviously I can't just think of myself here. If there was an easy solution that didn't change the behaviour of the if tag in this case that would be great, but I can't see one. 3) Behaviour of 'x and b or y' == Previously this was a syntax error. In Chris's code, it is fine and works like Python. I would vote for keeping the smart-if as it is, as I don't see the value of complicating the implementation to disallow something that could be useful sometimes. 4) Behaviour of 'not' = Current Django will interpret 'not' as a variable if there is no ambiguity e.g. {% if not %}blah{% endif %} This is not documented, but the behaviour is specified in detail in the tests, so changing it would be a backwards incompatibility. The smart-if is different (the above is a syntax error), and I suspect that making the smart-if behave like current Django could complicate things a lot. IMO, current Django behaviour here is complete misfeature! I don't know of any other language where keywords can be treated as variables if the keyword doesn't make sense in that position... Regards, Luke -- "He knows the way I take: when he has tried me, I shall come forth as gold" (Job 23:10). Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.