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/