Re: Bug in HTML::Form <label> support

Re: Bug in HTML::Form <label> support

am 11.12.2004 16:08:24 von gisle

Dan Kubb writes:

> Hi Gisle,
>
> > Are there other form elements than that might take labels?
>
> Yes, all the normal form elements can take labels. I'm
> just not sure how you would use them without adding to
> or changing the interface in HTML::Form.
>
> For tags that are radio or checkboxes its easy..
> just set the value_name attribute with the label name
> and the existing interface will use it. I can do that
> for other elements, but some of them inherit a noop
> value_names() method -- I didn't want to change this
> method's behaviour because it says in the docs that
> the values it returns correspond 1 to 1 with the return
> values from possible_values().
>
> Still it would be nice to set the values of a text
> value like this:
>
> $form->value('First Name');
>
> Rather than:
>
> $form->value('contact.name.first');
>
> I wasn't going to propose any interface changes in my
> patch without checking with the you first.

Seems like it might be a good idea to introduce a 'label' attribute
for inputs, but perhaps that creates the wrong expectation for radio
and checkbox entries. Got to ponder that some more.


> > Indentation is not consistent with the rest of the code.
>
> What's your indenting style for patches? I'm a two-space
> intender myself. The patch you received had tabs inserted
> manually just as I was finishing up. I tried to find a
> pattern in HTML::Form, but the style wasn't consistent
> enough for me to pick one up -- I figured there must be
> a lot of different maintainers ;)

It seems consistent to me. Perhaps you have tweaked your tab-stop to
not be the standard 8.

> >> + 1 while $attr->{value_name} =~ s/\s\z//;
> >
> > why not '$attr->{value_name} =~ s/\s+\z//;'
>
> Just finished a project with some large file processing..
> the "1 while" version is faster (strangely enough), there
> were some benchmarks on Perlmonks I believe.

You learn something new every day. I guess the + is too much for RE
optimizer here then.

> course it makes no difference with such small strings,
> I put it in more out of habit than anything.
>
> >> + $attr->{value_name} =~ s/\s+/ /;
> >
> > There can't really be multispace anywhere since get_phrase will trim
> > the text. This would always be a noop.
>
> You're right. I eliminated the need for regexes in
> a new patch which I've attached to this email. I
> think I've got the formatting right this time.

The new patch has now been applied. Thanks.

Regards,
Gisle