Fieldset and Multifield should not accept non basestring objects #56

Closed
gcbirzan opened this Issue Jun 2, 2012 · 5 comments

Comments

Projects
None yet
2 participants
@gcbirzan

gcbirzan commented Jun 2, 2012

While it may sound against Python's duck typing philosophy, this can lead to annoying bugs. If you pass it a layout object as the first item, it's converted to unicode and put in the template. Since it is done immediately after instantiation, there is no advantage over requiring it to be done outside.

@maraujop

This comment has been minimized.

Show comment Hide comment
@maraujop

maraujop Jun 2, 2012

Collaborator

I'm not quite sure I fully understand, could you please provide a code example. I think code will be easier to follow to me.

Thanks, cheers
Miguel

Collaborator

maraujop commented Jun 2, 2012

I'm not quite sure I fully understand, could you please provide a code example. I think code will be easier to follow to me.

Thanks, cheers
Miguel

@gcbirzan

This comment has been minimized.

Show comment Hide comment
@gcbirzan

gcbirzan Jun 2, 2012

I'm on my phone, so can't really type code easily but. Fieldset takes a
named parameter called label iirc (or maybe that was multifield), which is
the first parameter. If you, by mistake, omit that (or delete it, duh), and
the first element is a Div, it will be used as alabel. Something like:
Layout("type stuff here", Div('field'), Div('field2') ) works
Layout(Div('field'), Div('field2') ) doesn't.

The div gets converted to unicode in the init and since its rear looks
like a tag...

I will explain better tonight when I get home, even provide a patch if you
want, though it is trivial to fix.

On Jun 2, 2012 5:50 PM, "Miguel Araujo" <
reply@reply.github.com>
wrote:

I'm not quite sure I fully understand, could you please provide a code
example. I think code will be easier to follow to me.

Thanks, cheers
Miguel


Reply to this email directly or view it on GitHub:

maraujop#56 (comment)

gcbirzan commented Jun 2, 2012

I'm on my phone, so can't really type code easily but. Fieldset takes a
named parameter called label iirc (or maybe that was multifield), which is
the first parameter. If you, by mistake, omit that (or delete it, duh), and
the first element is a Div, it will be used as alabel. Something like:
Layout("type stuff here", Div('field'), Div('field2') ) works
Layout(Div('field'), Div('field2') ) doesn't.

The div gets converted to unicode in the init and since its rear looks
like a tag...

I will explain better tonight when I get home, even provide a patch if you
want, though it is trivial to fix.

On Jun 2, 2012 5:50 PM, "Miguel Araujo" <
reply@reply.github.com>
wrote:

I'm not quite sure I fully understand, could you please provide a code
example. I think code will be easier to follow to me.

Thanks, cheers
Miguel


Reply to this email directly or view it on GitHub:

maraujop#56 (comment)

@gcbirzan

This comment has been minimized.

Show comment Hide comment
@gcbirzan

gcbirzan Jun 2, 2012

Ah. Meant Fieldset instead of layout in the examlle
On Jun 2, 2012 6:02 PM, "George-Cristian Bîrzan" gcbirzan@gmail.com
wrote:

I'm on my phone, so can't really type code easily but. Fieldset takes a
named parameter called label iirc (or maybe that was multifield), which is
the first parameter. If you, by mistake, omit that (or delete it, duh), and
the first element is a Div, it will be used as alabel. Something like:
Layout("type stuff here", Div('field'), Div('field2') ) works
Layout(Div('field'), Div('field2') ) doesn't.

The div gets converted to unicode in the init and since its rear looks
like a tag...

I will explain better tonight when I get home, even provide a patch if you
want, though it is trivial to fix.

On Jun 2, 2012 5:50 PM, "Miguel Araujo" <
reply@reply.github.com>
wrote:

I'm not quite sure I fully understand, could you please provide a code
example. I think code will be easier to follow to me.

Thanks, cheers
Miguel


Reply to this email directly or view it on GitHub:

maraujop#56 (comment)

gcbirzan commented Jun 2, 2012

Ah. Meant Fieldset instead of layout in the examlle
On Jun 2, 2012 6:02 PM, "George-Cristian Bîrzan" gcbirzan@gmail.com
wrote:

I'm on my phone, so can't really type code easily but. Fieldset takes a
named parameter called label iirc (or maybe that was multifield), which is
the first parameter. If you, by mistake, omit that (or delete it, duh), and
the first element is a Div, it will be used as alabel. Something like:
Layout("type stuff here", Div('field'), Div('field2') ) works
Layout(Div('field'), Div('field2') ) doesn't.

The div gets converted to unicode in the init and since its rear looks
like a tag...

I will explain better tonight when I get home, even provide a patch if you
want, though it is trivial to fix.

On Jun 2, 2012 5:50 PM, "Miguel Araujo" <
reply@reply.github.com>
wrote:

I'm not quite sure I fully understand, could you please provide a code
example. I think code will be easier to follow to me.

Thanks, cheers
Miguel


Reply to this email directly or view it on GitHub:

maraujop#56 (comment)

@maraujop

This comment has been minimized.

Show comment Hide comment
@maraujop

maraujop Jun 2, 2012

Collaborator

Ok, now I see I had understood you correctly.

I was planning on doing this and some other type checkings. I will probably be able to work on this tomorrow evening.

Thanks, cheers
Miguel

Collaborator

maraujop commented Jun 2, 2012

Ok, now I see I had understood you correctly.

I was planning on doing this and some other type checkings. I will probably be able to work on this tomorrow evening.

Thanks, cheers
Miguel

@maraujop

This comment has been minimized.

Show comment Hide comment
@maraujop

maraujop Jul 29, 2012

Collaborator

I've been thinking on this and I'm afraid this might not be as doable as I thought in the beginning. The problem resides in the fact that not only a basestring subclass can be passed as a Fieldset legend, you can also pass i18n objects, like when you use from django.utils.translation import ugettext_lazy and there are others.

So most likely this will be hard to do, as other people will not be able to pass other objects that behave like a duck, see what I mean?

Cheers,
Miguel

Collaborator

maraujop commented Jul 29, 2012

I've been thinking on this and I'm afraid this might not be as doable as I thought in the beginning. The problem resides in the fact that not only a basestring subclass can be passed as a Fieldset legend, you can also pass i18n objects, like when you use from django.utils.translation import ugettext_lazy and there are others.

So most likely this will be hard to do, as other people will not be able to pass other objects that behave like a duck, see what I mean?

Cheers,
Miguel

@maraujop maraujop closed this Sep 24, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment