DjangoBot: #django-dev

http://dpaste.com/55961/ <-- code review me for max_num in newforms-admin formsets if anyone wants to

0 for max_forms means no limit I assume?

right

Looks good by me, except on line 103 isn't the 0 before the colon unnescesary

i am pretty sure if there is no 0 it evaluates

i remember being bit by that. not 100% sure though ;)

Oh, I have never tested that, time to go look at the QS code :P

I don't think it does, http://code.djangoproject.com/browser/django/br...

Nope, it doesn't

yeah. hang on let me open my work source and look at my comment

That may be a pre-qsrf relic

and not 100% sure

probably was

like it really matters in this case anyways ;)

ah, i see

yeah i was confused

[0:1] != [0] ;)

:D

[0] will grab the whole result set and give you [0] on the _result_cache while [0:1] would do the limit. thats what i had commented in my code.

Is that still true?

yes

I'm guessing its no longer an issue because the iterator is efficient, it will only read the first few objects into memory

but in my case the query the database got was making it really slow

Ah, fun

it may be efficient in django, but the database chugs away at the select * from table;

;)

Hehe, right

ok cool i remove that 0 in my patch. make it look better ;)

i'll get this committed tonight

Awesome, what other big things are left, after this?

there is one last compatibility ticket, but it is waiting on a newforms change i believe

and the rest should be non-blockers. and some docs

I think nfa should be using django-ajax-validation :P

heh, no reason for you not to use it with it if you like

Actually, it would be really easy, since the JS is non intrusive, I'd have to make a custom callback I think... I'm going to play with this tommorow I think

:)

What was the newforms issue you said was blocking nfa?

look at#5731. that is the nfa ticket for the missing feature

#5731

maybe it can be avoided. i need to look at the patch in depth again

newforms-admin chaps: need to sanity check a suggestion for making view subclassing a bit more useful

I want to be able to add extra stuff to the context used in changelist_view and friends

at the moment it doesn't provide a hook for adding to the context, and it returns a pre-rendered template so you can't intercept it

There are two possibilities for fixes, both based around passing an optional argument to changelist_view

def changelist_view(self, request, extra_context=None)

then towards the end of the method:

c.update(extra_context or {})

Option 2: def changelist_view(self, request, return_context=False)

if return_context is True, the method returns just the context rather than rendering it to a template and returning an HttpResponse

Option 1 matches what the change_view function does already

but Option 2 is slightly more powerful - since you can modify existing things in the context and even load and render a different template

Option 2 is a bit weird though, because it means that you're changing the return type of the function based on an argument you pass to it

i would tend to lean towards the former. to render the templates that they do now the latter would make it harder since it relies on some local variables in those functions

Option 2 looks more interesting for agregation too

that's always the same problem (at least for me) of current vs. logic+responders

ultimately i would prefer to see a class based approach where those bits of functionality can be overridden and can be made consistent across views if need be

hang on, pasting better example to dpaste.com

brosner, I do agree :)

that shows what it looks like with each option

having written them out like that, I'm leaning towards option 1

the only advantage of option 2 is it gives you more flexibility in selecting the template

but there are other ways we can do that which would have a nicer overall API

option 2 would force the template change

For templates, I want to be able to say "class MyAdmin(ModelAdmin): changelist_template = 'path/to/template.html' "

I figure if you want a dynamic template you can still have one, you just override changelist_view and have it modify self.changelist_template before calling super

SimonW: how do you feel towards a class-based approached to customizing the views?

brosner: I love it, but if it gets added to newforms-admin and breaks backwards compatibility I will probably be brutally murdered by my colleagues

who have spent several months coding against existing newforms-admin under the expectation that there weren't going to be any major API changes before it got merged

minor backwards compatibility breaks aren't a problem, but major ones will probably lead to my death

how large are the proposed changes for class-based view customisation?

are you already doing view overriding?

in a few places yes

I can grep our source and find out how much

from what i have seen of jkocherhans' stuff is that those instances would be broken if thats what he ends up doing

well actually probably not if you are duplicating functionality

hmm... turns out we're not doing it that much - only in three or four places

got a link to his stuff that illustrates the API?

SimonW: let me take a quick look at my notes and translate them to the pastebin

SimonW, #6735

and there is a thread about ModelView on the devel list

looks pretty awesome to me

especially if it means newforms-admin stuff can be used as regular generic views separate from the rest of newforsm-admin

SimonW: http://dpaste.com/56054/

how does formfield_for_dbfield fit in that?

that looks really nice - would make it a cinch to create reusable custom admin view classes

SimonW, http://groups.google.com/group/django-developer...

SimonW: not sure yet. I want it to die, but I'm still not sure how to get the admin widgets to work without it.

Did you see my choices_for_* etc proposal?

I got the code for that written the other day, haven't tried it out on anything yet though

could really do with a massive newforms-admin testing application that exercises ALL of the admin features in one app

yes, but I don't remember much about it right now. my head has been elsewhere

definitel

y

I've gotta head for my train unfortunately