Dan Kubb <[EMAIL PROTECTED]> writes:

> Hi Gisle,
> 
> > Are there other form elements than <input> 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 <input> 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 <input>
> 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

Reply via email to