Re: Suppressed template errors in admin

2011-08-24 Thread Jonas H.

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

2011-08-24 Thread Russell Keith-Magee
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

2011-08-24 Thread h3
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

2011-08-26 Thread Ole Laursen
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

2011-08-26 Thread Ole Laursen
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

2011-09-03 Thread 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.

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

2011-09-05 Thread Yuri Baburov
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

2011-09-06 Thread Thomas Guettler


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

2011-09-06 Thread Russell Keith-Magee
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

2011-09-06 Thread Ole Laursen
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

2011-09-06 Thread Jens Diemer

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.