On Sunday 04 November 2007 18:06, Mike Martin wrote:
>
> Hi

Hello,

> I have the following script
>
> #!/usr/bin/perl -w
> use CGI qw/:standard/;
> use CGI::Carp qw(fatalsToBrowser warningsToBrowser);
> use strict;
> use File::Basename;
>
> our $list;
> sub run_cmd { return print

print() returns either true or false.  Why are you returning this value 
from your sub?

> span( { -class => 'place_cmd' }, submit( -name => 'action',
> -value => shift ) ), p };
>
> my %opts=(entry=>\&entry,
> print1=>\&print1,
> print2=>\&print2
> );
>
> my $cmd=param('action');
> if ($opts{$cmd}){

You should probably use exists() there:

if ( exists $opts{ $cmd } ) {


> $opts{$cmd}->();
> }
>
> run_cmd('entry');

The run_cmd() sub does not take any arguments.  Perhaps you meant this 
instead:

$opts{ entry }->();


> &entry;
> sub none {print header};
> #}
> sub entry {
> print header();
> print start_html('Man Pages');
> print start_multipart_form;
> print "man Page ",textfield('man');
> run_cmd('print1');

The run_cmd() sub does not take any arguments.  Perhaps you meant this 
instead:

$opts{ print1 }->();


> print end_form;
> print end_html;
> }
>
>
> sub print1 {
>
> my %list;
> print header();
> print start_html('Man Pages');
> my $page=param('man');
> my @array=`find /usr/share/man -name $page*`;

Do you need to search through /usr/share/man each time this sub is 
called?  Do your man pages change that often?  Perhaps you could just 
populate the array outside the sub.

The backticks produce a list of lines with each line ending in a 
newline so you have to chomp() them first:

chomp( my @array = `find /usr/share/man -name $page*` );

Why just /usr/share/man?  What about the other paths in $ENV{ MANPATH }?


> my @list;
> foreach my $man (@array){
> push (@list,$man);
>
> }

Why are you copying @array to @list?  Why not just assign the list to 
@list in the first place?


> foreach my $list (@list){
>
> my $name=fileparse($list);
> my @man=split /\./, $name;
> my $end = (scalar @man) -3;
> my [EMAIL PROTECTED];
> my $section=$man[-2];

Or more simply:

my ( $name1, $section ) = ( split /\./, $name )[ -3, -2 ];

But of course that won't work correctly if you have a man page name 
with a '.' character in it.


> my $title="$name1 Section  $section";
> $list{$list}=$title;
> }
> my @list1;
> foreach my $keys (sort keys %list){
> push (@list1,$list{$keys});
> }

Or more simply:

my @list1 = @list{ sort leys %list };


> print start_multipart_form;
> print "select man page", popup_menu('sel',[EMAIL PROTECTED]);
> run_cmd('print2');

The run_cmd() sub does not take any arguments.  Perhaps you meant this 
instead:

$opts{ print2 }->();


> print end_form;
>
> $list=\%list;
> foreach my $keys (keys %{$list}){
> print $keys,br
> }
> print end_html;
> return $list;
> }

I would have written that sub as:

use File::Find;

sub print1 {
    my $page  = param( 'man' );

    my %list;
    find sub {
        return unless /\A(\Q$page\E.*)\.(\d)\.gz\z/;
        $list{ $File::Find::name } = "$1 Section  $2";
        }, '/usr/share/man';

    print header(),
        start_html( 'Man Pages' ),
        start_multipart_form(),
        'select man page',
        popup_menu( 'sel', [ @list{ sort keys %list } ] ),
        span( { -class => 'place_cmd' }, submit( -name => 'action', 
-value => shift ) ),
        p(),
        end_form(),
        map( ( $_ => br() ), keys %list ),
        end_html();

    return \%list;
    }




John
-- 
use Perl;
program
fulfillment

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/


Reply via email to