Kevin Dangoor <[EMAIL PROTECTED]> writes:
> Consider this case:
> class TextArea(FormField, SimpleWidget):
> template = """
> <textarea xmlns:py="http://purl.org/kid/ns#";
> name="${name}"
> class="${field_class}"
> id="${field_id}"
> rows="${rows}"
> cols="${cols}"
> py:attrs="attrs"
> py:content="value"
> />
> """
>
> template_vars = ["rows", "cols"]
>
> def __init__(self, rows=7, cols=50, **params):
> super(TextArea, self).__init__(**params)
> self.rows = rows
> self.cols = cols
>
> Before I get to my main point, I'd like to mention something I just
> noticed: my preference is to have "name" be the first parameter. Like
> this:
>
> def __init__(self, name=None, rows=7, cols=50, **params):
> super(TextArea, self).__init__(name, **params)
>
> That way, widget users can always do this:
>
> TableForm(widgets = [TextAreaField("foo")])
>
> Is that a bad idea? It's easy to test for.
I like it.
> Now to my main point: the widget above is the way it looks after a
> change I made. Previously, it didn't have the template_vars and had an
> update_data that looked something like this:
>
> attrs = d["attrs"]
> d["rows"] = d[attrs].get(rows, self.rows)
> d["cols"] = d[attrs].get(cols, self.cols)
This makes it possible to work around IndexErrors if there's no default... If
every option is going to have a default value, then there's no problem with
the change, but if this isn't the case, some_dict.get('key') returns None if
there's no 'key' in the dictionary. Of course, you know that, but it is
something worth noting.
The above may helps a novice to provide defaults and be a bit more safe than
in your code, but your code is cleaner than this, making it easier to read.
> My preference, as you can see above, is for attributes that are
> specifically supported in the constructor to be explicitly contained
> in the template (as they) and not rely on attrs. With the style that's
I believe attrs should be used just for excetions and not for every
attribute. What is an exception or not is open to debate, but anything
required for making the widget work is essential, plus CSS class, everything
else should be an exception.
So, I'd make it default: id, name and class for all widgets.
> committed now, a user can do this:
>
> tf = TextField(rows=5, cols=10)
> tf.display(rows=8, cols=39)
> or even
> tf.display(attrs=dict(rows=10, cols=992))
If both styles work, I believe we're good with flexibility!
> So, this style is a bit less code (it leverages template_vars), it's more
> explicit in the template and it still makes it easy (and obvious!) for a
> user to override.
I believe that we have to make it easier for the user -- developers using TG
-- not TG developers ;-) As said, one will write a widget once and use it
many times. If we make this "once" a bit (not too much!) harder to make the
"many times" easier, that will be "a good thing"(TM)
--
Jorge Godoy <[EMAIL PROTECTED]>
