libwww-perl / WWW-Mechanize

Handy web browsing in a Perl object
https://metacpan.org/pod/WWW::Mechanize
Other
68 stars 52 forks source link

Inconsistent handling of forms which cannot be found #286

Closed oalders closed 4 years ago

oalders commented 4 years ago

In submit_form():

    if ( $args{with_fields} ) {
        $fields || die q{must submit some 'fields' with with_fields};
        my @got = $self->all_forms_with_fields(keys %{$fields});
        die "There is no form with the requested fields" if not @got;
        push @filtered_sets, \@got;
    }
    if ( my $form_number = $args{form_number} ) {
        my $got = $self->form_number( $form_number );
        die "There is no form numbered $form_number" if not $got;
        push @filtered_sets, [ $got ];
    }
    if ( my $form_name = $args{form_name} ) {
        my @got = $self->all_forms_with( name => $form_name );
        die qq{There is no form named "$form_name"} if not @got;
        push @filtered_sets, \@got;
    }
    if ( my $form_id = $args{form_id} ) {
        my @got = $self->all_forms_with( id => $form_id );
        $self->warn(qq{ There is no form with ID "$form_id"}) if not @got;
        push @filtered_sets, \@got;
    }

First off, why do we warn on a missing form_id, but die in the other cases?

Secondly, why do we die rather than $self->die?

We should either clean this up or document the logic. @petdance any thoughts on this?

cc @oschwald

petdance commented 4 years ago

I don't have any backstory to add here. Sorry.

Also, this error message has a leading space:

        $self->warn(qq{ There is no form with ID "$form_id"}) if not @got;
oalders commented 4 years ago

Thanks @petdance. If you can't think of a compelling reason for this behaviour, that's already helpful. We should strip that leading space as well. :)