_attr method for common argument parsing

Nearly all methods adhere to the same peculiar shorthand of allowing
several optional arguments, but always expecting a trailing hashref to
be attribute options.  For various reasons, this is better abstracted.

The only significant behavioural change is that the first arguments to
input() can now be overridden with option values if undef (which seems
only logical).

diff --git a/lib/HTML/Form/Simple.pm b/lib/HTML/Form/Simple.pm
index 57b2a918b6f2a1351abdadc515de642598f9fb3b..2c8ce8de0d2938855c6fed97f31fb4c5ee819b8b 100644 (file)
--- a/lib/HTML/Form/Simple.pm
+++ b/lib/HTML/Form/Simple.pm
@@ -12,6 +12,15 @@ sub new {
        bless {}, $_[0];
 }
 
+sub _attr {
+       my $self = shift;
+       my $expect = shift;
+       my $attr = ref $_[-1] eq 'HASH' ? pop : {};
+       push @_, undef for @_+1 .. $expect;
+       push @_, $attr;
+       return @_;
+}
+
 sub quote {
        my $self = shift;
        return XML::Quote::xml_quote_min($_[0]);
@@ -49,48 +58,31 @@ sub stop {
 
 
 sub submit {
-       my ($self, $value, $attr) = @_;
-
-       if (ref $value eq 'HASH') {
-               $attr = $value;
-       }
-       else {
-               $attr ||= {};
-               $attr->{value} = $value;
-       }
+       my $self = shift;
+       my ($value, $attr) = $self->_attr(1, @_);
 
+       $attr->{value} = $value if defined $value;
        $attr->{type} = 'submit' unless defined $attr->{type};
 
        return $self->tag(input => $attr);
 }
 
 sub hidden {
-       my ($self, $name, $value) = @_;
+       my $self = shift;
+       my ($name, $value, $attr) = $self->_attr(2, @_);
 
+       $attr = {type => 'hidden', name => $name, value => $value};
        #TODO: $attr
 
-       return $self->tag(input => {type => 'hidden', name => $name, value => $value});
+       return $self->tag(input => $attr);
 }
 
 sub input {
-       my ($self, $name, $value, $attr) = @_;
-
-       if (ref $name eq 'HASH') {
-               # only attributes provided (first argument)
-               $attr = $name;
-       }
-       elsif (ref $value eq 'HASH') {
-               # name shorthand (attributes in value parameter)
-               $attr = $value;
-               $attr->{name} = $name;
-       }
-       else {
-               # name and value shorthands (all vars keep their assigned values)
-               $attr ||= {};
-               $attr->{name} = $name;
-               $attr->{value} = $value;
-       }
+       my $self = shift;
+       my ($name, $value, $attr) = $self->_attr(2, @_);
 
+       $attr->{name } = $name  if defined $name;
+       $attr->{value} = $value if defined $value;
        $attr->{id}   = $attr->{name} unless defined $attr->{id};
        $attr->{type} = 'text' unless defined $attr->{type} or defined $attr->{rows};
        $value = delete $attr->{value} if defined $attr->{rows};
@@ -103,16 +95,12 @@ sub input {
 }
 
 sub select {
-       my ($self, $name, $rows, $value, $attr) = @_;
+       my $self = shift;
+       my ($name, $rows, $default, $attr) = $self->_attr(3, @_);
+
+       $default = $attr->{value} unless defined $default;
+       delete $attr->{value};  # never a parent attribute
 
-       if (ref $value eq 'HASH') {
-               $attr = $value;
-               $value = delete $attr->{value};
-       }
-       else {
-               $attr ||= {};
-               delete $attr->{value};
-       }
        $attr->{name} = $name;
        $attr->{id}   = $attr->{name} unless defined $attr->{id};
        $attr->{type} = 'select' unless defined $attr->{type};
@@ -121,9 +109,9 @@ sub select {
 
        if ($attr->{type} eq 'select') {
                delete $attr->{type};
-               if (defined $value) {
+               if (defined $default) {
                        for (@options) {
-                               $_->{selected} = 1 if defined $_->{value} and $_->{value} eq $value;
+                               $_->{selected} = 1 if defined $_->{value} and $_->{value} eq $default;
                        }
                }
                my @return = (
@@ -144,9 +132,9 @@ sub select {
                                        for @options;
                        delete $attr->{label};
                }
-               if (defined $value) {
+               if (defined $default) {
                        for (@options) {
-                               $_->{checked} = 1 if defined $_->{value} and $_->{value} eq $value;
+                               $_->{checked} = 1 if defined $_->{value} and $_->{value} eq $default;
                        }
                }
                $_ = {%$attr, %$_} for @options;
@@ -161,36 +149,19 @@ sub select {
 }
 
 sub radio {
-       my ($self, $name, $value, $attr) = @_;
-
-       if (ref $value eq 'HASH') {
-               $attr = $value;
-       }
-       else {
-               $attr ||= {};
-               $attr->{value} = $value;
-       }
+       my $self = shift;
+       my ($name, $value, $attr) = $self->_attr(2, @_);
 
-       $self->select($name, [$attr], {type => 'radio'});
+       $self->select($name, [$value], {%$attr, type => 'radio'});
 }
 
 sub check {
-       my ($self, $name, $label, $checked, $attr) = @_;
+       my $self = shift;
+       my ($name, $label, $checked, $attr) = $self->_attr(3, @_);
 
-       if (ref $label eq 'HASH') {
-               $attr = $label;
-               undef $label;
-       }
-       elsif (ref $checked eq 'HASH') {
-               $attr = $checked;
-               $attr->{label} = $label;
-       }
-       else {
-               $attr ||= {};
-               $attr->{checked} = $checked;
-               $attr->{label} = $label;
-       }
-       $attr->{value} = '1' unless exists $attr->{value};
+       $attr->{label  } = $label   if defined $label;
+       $attr->{checked} = $checked if defined $checked;
+       $attr->{value  } = '1' unless exists $attr->{value};
 
        $self->select($name, [$attr], {type => 'checkbox'});
 }

diff --git a/t/html.t b/t/html.t
index d12d6ef8127ee73a16e2ba4c837dcab0b3616ccc..60e95074768d395f2567cac743440c6903576eb6 100644 (file)
--- a/t/html.t
+++ b/t/html.t
@@ -119,15 +119,22 @@ is(
 );
 
 is(
-       $form->input('', '', {disabled => 0, something => undef, class => undef, style => ''}),
+       $form->input('', '', {
+               disabled => 0,
+               something => undef,
+               class => undef,
+               style => '',
+               name => 'ignore',
+               value => 'overrides',
+       }),
        '<input name="" type="text" value="">',
        'input with empty attributes'
 );
 
 is(
-       $form->input(undef, undef, {name => 'override', value => 'override'}),
-       '<input type="text">',
-       'ignore input overrides'
+       $form->input(undef, undef, {name => '0', value => '0'}),
+       '<input id="0" name="0" type="text" value="0">',
+       'input overrides'
 );
 
 is(