Re: Suppressed template errors in admin
On 08/24/2011 05:44 PM, Ole Laursen wrote: Hi! I have a project where setting TEMPLATE_STRING_IF_INVALID has been invaluable in finding problems (for various reasons). The caveat mentioned in the docs don't kick in here because no templates are inherited from elsewhere. Except the admin which unfortunately breaks down in some places. I realize there's been a big discussion before about the merits of error suppression. No need to repeat that. But is it really not possible to fix admin in the few places it's a problem? Those I've seen so far have one-line fixes where you add an empty variable to the context. Seems harmless to me. I realize cluttering the template itself with a {% if var %} is ugly. No need to go there. I don't mind writing a patch for the cases that bother me, but won't do it unless somebody is willing to apply it? Ole +1 This little ugly hack is incredibly useful during development: # Hack class RaiseMissingVariable(object): def __mod__(self, val): raise AttributeError("Missing %r" % val) def __contains__(self, what): return True TEMPLATE_STRING_IF_INVALID = RaiseMissingVariable() -- 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 django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Suppressed template errors in admin
On Wed, Aug 24, 2011 at 11:44 PM, Ole Laursen wrote: > Hi! > > I have a project where setting TEMPLATE_STRING_IF_INVALID has been > invaluable in finding problems (for various reasons). The caveat > mentioned in the docs don't kick in here because no templates are > inherited from elsewhere. Except the admin which unfortunately breaks > down in some places. > > I realize there's been a big discussion before about the merits of > error suppression. No need to repeat that. > > But is it really not possible to fix admin in the few places it's a > problem? Those I've seen so far have one-line fixes where you add an > empty variable to the context. Seems harmless to me. I realize > cluttering the template itself with a {% if var %} is ugly. No need to > go there. > > I don't mind writing a patch for the cases that bother me, but won't > do it unless somebody is willing to apply it? On principle, I have no objection to the idea of making the admin templates more robust in the presence of TEMPLATE_STRING_IF_INVALID; adding dummy values in the context sounds like a reasonable approach -- *if* doing this doesn't undermine broader error handling in the templates. Also -- dummy values may not be the only option. There's also |default and |default_if_none filters. However, in order to evaluate the details, I'd need to see them -- which unfortunately means a patch (or, at least, an indicative start of a patch). I fully appreciate your desire not to waste your time, though -- there's no point making a patch if it will be ignored. I would suggest the best approach here is to attack this in stages. Provide a patch that fixes a small number of uses in a limited number of key templates, and poke around on django-dev or on IRC to ask for a review. Once that initial patch is approved and/or applied, expand the patch until it covers all problematic uses. 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-developers@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: Suppressed template errors in admin
I'm not sure suppressing templates errors for the admin is such a great idea. I work on two projects[1] where I would lose a useful debugging information and it would be the same lost to anyone working on the admin or extending the admin. Of course we could use a settings to toggle on/off the admin error suppression, but that would mean another setting.. which is to be avoided. Maybe a better approach would be to simply mark the fallback string as safe. Then it would be possible to suppress them like so: TEMPLATE_STRING_IF_INVALID = "" This would have the benefit to be more flexible: TEMPLATE_STRING_IF_INVALID = "!! Invalid var !!" # settings_dev.py TEMPLATE_STRING_IF_INVALID = "" # settings_prod.py But this would have side effects on the non-admin templates too.. so it's not ideal. Maybe something like this: TEMPLATE_STRING_IF_INVALID = {'default: '!! Invalid var !!', 'django.contrib.admin': ''} I think it would pretty much solve the problem, preserve backward compatibility/behavior and even allow to suppress errors for any other apps. [1] https://github.com/sehmaschine/django-grappelli http://jqmobile-sandbox.motion-m.ca/ -- Maxime Haineault On Aug 24, 7:39 pm, Russell Keith-Magee wrote: > On Wed, Aug 24, 2011 at 11:44 PM, Ole Laursen wrote: > > Hi! > > > I have a project where setting TEMPLATE_STRING_IF_INVALID has been > > invaluable in finding problems (for various reasons). The caveat > > mentioned in the docs don't kick in here because no templates are > > inherited from elsewhere. Except the admin which unfortunately breaks > > down in some places. > > > I realize there's been a big discussion before about the merits of > > error suppression. No need to repeat that. > > > But is it really not possible to fix admin in the few places it's a > > problem? Those I've seen so far have one-line fixes where you add an > > empty variable to the context. Seems harmless to me. I realize > > cluttering the template itself with a {% if var %} is ugly. No need to > > go there. > > > I don't mind writing a patch for the cases that bother me, but won't > > do it unless somebody is willing to apply it? > > On principle, I have no objection to the idea of making the admin > templates more robust in the presence of TEMPLATE_STRING_IF_INVALID; > adding dummy values in the context sounds like a reasonable approach > -- *if* doing this doesn't undermine broader error handling in the > templates. > > Also -- dummy values may not be the only option. There's also |default > and |default_if_none filters. > > However, in order to evaluate the details, I'd need to see them -- > which unfortunately means a patch (or, at least, an indicative start > of a patch). > > I fully appreciate your desire not to waste your time, though -- > there's no point making a patch if it will be ignored. I would suggest > the best approach here is to attack this in stages. Provide a patch > that fixes a small number of uses in a limited number of key > templates, and poke around on django-dev or on IRC to ask for a > review. Once that initial patch is approved and/or applied, expand the > patch until it covers all problematic uses. > > 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-developers@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: Suppressed template errors in admin
On 25 Aug., 01:39, Russell Keith-Magee wrote: > On principle, I have no objection to the idea of making the admin > templates more robust in the presence of TEMPLATE_STRING_IF_INVALID; > adding dummy values in the context sounds like a reasonable approach > -- *if* doing this doesn't undermine broader error handling in the > templates. Yeah, sure. I don't think this is a big problem, a couple of places tops. Thanks for considering this! I'm checking out the SVN release right now, but actually the one that I've been bitten by so far has a patch here (patch against current trunk below): https://code.djangoproject.com/ticket/12136 Note that this was merged into a monster bug that had worse patches (as far as I can tell), adding if-tags in the templates, and depressingly never went anywhere. Ole Index: django/contrib/admin/templatetags/admin_list.py === --- django/contrib/admin/templatetags/admin_list.py (revision 16696) +++ django/contrib/admin/templatetags/admin_list.py (working copy) @@ -102,7 +102,10 @@ admin_order_field = getattr(attr, "admin_order_field", None) if not admin_order_field: # Not sortable -yield {"text": text} +yield { +"text": text, +"class_attrib": "" +} continue # OK, it is sortable if we got this far -- 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 django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Suppressed template errors in admin
On 25 Aug., 06:19, h3 wrote: > I'm not sure suppressing templates errors for the admin is such a > great idea. The suggestion on the table is to fix the couple of places where admin is sloppy and doesn't include all the variables it uses in the context. Normally you don't see this because this kind of error is suppressed by Django. However, if you turn on TEMPLATE_STRING_IF_INVALID, they show up and break the markup, e.g. the column headers in the overview table. Ole -- 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 django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Suppressed template errors in admin
On 25/08/11 00:39, Russell Keith-Magee wrote: > On principle, I have no objection to the idea of making the admin > templates more robust in the presence of TEMPLATE_STRING_IF_INVALID; > adding dummy values in the context sounds like a reasonable approach > -- *if* doing this doesn't undermine broader error handling in the > templates. If we call this a bug, and agree to fix it, does it mean that from now on for any changes to the admin template we have to be careful to test with TEMPLATE_STRING_IF_INVALID != '' ? That sounds kind of tedious, and I would be against making a promise never to break this again in the future. If we can fix it now with relatively little incovenience, fine, but I don't want that to turn into a promise. Regards, Luke -- "Doubt: In the battle between you and the world, bet on the world." (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-developers@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: Suppressed template errors in admin
Hi Luke, On Sat, Sep 3, 2011 at 9:13 PM, Luke Plant wrote: > On 25/08/11 00:39, Russell Keith-Magee wrote: > >> On principle, I have no objection to the idea of making the admin >> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID; >> adding dummy values in the context sounds like a reasonable approach >> -- *if* doing this doesn't undermine broader error handling in the >> templates. > > If we call this a bug, and agree to fix it, does it mean that from now > on for any changes to the admin template we have to be careful to test > with TEMPLATE_STRING_IF_INVALID != '' ? We could switch admin test suite to always run with proposed TEMPLATE_STRING_IF_INVALID = RaiseMissingVariable() and check if there are more mistakes, fix mistakes (if any), and make final patch! Any missing {{ variable }} could be easily replaced with {% firstof variable "" %} -- it works, and I remember |default and |default_if_none didn't few years ago. Not sure if it's still the same. At least there are https://code.djangoproject.com/browser/django/trunk/tests/regressiontests/admin_views/tests.py and https://code.djangoproject.com/browser/django/trunk/tests/regressiontests/admin_inlines/tests.py which render admin templates. > That sounds kind of tedious, and I would be against making a promise > never to break this again in the future. If we can fix it now with > relatively little incovenience, fine, but I don't want that to turn into > a promise. Then we could generously ask to show us a test which breaks admin output when TEMPLATE_STRING_IF_INVALID = RaiseMissingVariable() I don't believe there will be much work, because current patch proposes to fix exactly same wart that were there 4 years ago, just with different, more elegant, piece of code -- check my https://code.djangoproject.com/ticket/3579 ( some part has been fixed later at https://code.djangoproject.com/ticket/4497 ). -- Best regards, Yuri V. Baburov, Skype: yuri.baburov, MSN: bu...@live.com -- 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 django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Suppressed template errors in admin
Am 03.09.2011 16:13, schrieb Luke Plant: > On 25/08/11 00:39, Russell Keith-Magee wrote: > >> On principle, I have no objection to the idea of making the admin >> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID; >> adding dummy values in the context sounds like a reasonable approach >> -- *if* doing this doesn't undermine broader error handling in the >> templates. > > If we call this a bug, and agree to fix it, does it mean that from now > on for any changes to the admin template we have to be careful to test > with TEMPLATE_STRING_IF_INVALID != '' ? > > That sounds kind of tedious, and I would be against making a promise > never to break this again in the future. If we can fix it now with > relatively little incovenience, fine, but I don't want that to turn into > a promise. If the django core makes a promise that TEMPLATE_STRING_IF_INVALID=RaiseMissingVariable() is supported in admin (and other contrib apps), I will help to get it done. Up to now, that's why I don't use the template language in my apps. Zen of Python: Errors should never pass silently. Ole, can you please set up a a branch at github or bitbucket? Thomas -- Thomas Guettler, http://www.thomas-guettler.de/ E-Mail: guettli (*) thomas-guettler + de -- 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 django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Suppressed template errors in admin
On Tue, Sep 6, 2011 at 3:17 PM, Thomas Guettler wrote: > > > Am 03.09.2011 16:13, schrieb Luke Plant: >> On 25/08/11 00:39, Russell Keith-Magee wrote: >> >>> On principle, I have no objection to the idea of making the admin >>> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID; >>> adding dummy values in the context sounds like a reasonable approach >>> -- *if* doing this doesn't undermine broader error handling in the >>> templates. >> >> If we call this a bug, and agree to fix it, does it mean that from now >> on for any changes to the admin template we have to be careful to test >> with TEMPLATE_STRING_IF_INVALID != '' ? >> >> That sounds kind of tedious, and I would be against making a promise >> never to break this again in the future. If we can fix it now with >> relatively little incovenience, fine, but I don't want that to turn into >> a promise. > > If the django core makes a promise that > TEMPLATE_STRING_IF_INVALID=RaiseMissingVariable() is > supported in admin (and other contrib apps), I will help to get it done. > > Up to now, that's why I don't use the template language in my apps. > > Zen of Python: Errors should never pass silently. This particular argument fails because we're not talking about Python, here. We're talking about a template language that renders to a public facing output. Errors on user-facing code isn't cool. 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-developers@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: Suppressed template errors in admin
On 6 Sep., 09:17, Thomas Guettler wrote: > Ole, can you please set up a a branch at github or bitbucket? Eh, is that not overkill? I don't think discussing this is worth anyone's time. No need to promise anything or elevate this to a Big Decision as long as it's about trivial non-intrusive fixes. I'm still hoping someone (Russ?) will just take the patch and apply it, and I can go back to being the happy camper. :) Ole -- 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 django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Suppressed template errors in admin
Am 25.08.2011 06:19, schrieb h3: But this would have side effects on the non-admin templates too.. so it's not ideal. Maybe something like this: TEMPLATE_STRING_IF_INVALID = {'default: '!! Invalid var !!', 'django.contrib.admin': ''} I do a hack in my admin.py: - # some django admin stuff is broken if TEMPLATE_STRING_IF_INVALID != "" # http://code.djangoproject.com/ticket/3579 if settings.TEMPLATE_STRING_IF_INVALID != "": # Patch the render_to_response function ;) from django.contrib.auth import admin as auth_admin from django.contrib.admin import options def patched_render_to_response(*args, **kwargs): old = settings.TEMPLATE_STRING_IF_INVALID settings.TEMPLATE_STRING_IF_INVALID = "" result = render_to_response(*args, **kwargs) settings.TEMPLATE_STRING_IF_INVALID = old return result options.render_to_response = patched_render_to_response auth_admin.render_to_response = patched_render_to_response - So you can use TEMPLATE_STRING_IF_INVALID, but it's excluded from django admin ;) -- Mfg. Jens Diemer http://www.jensdiemer.de -- 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 django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.