Hi Shlomi,
Thanks for the feedback.
My intention with my $files, according to the documentation for
Archive::Tar is that you're supposed to be able to provide a list of
files. It is my intention for the user to provide a single file or a
directory, but it doesn't seem to be working properly.
I've tried running it like so:
$ ./eztar -c git.tar.gz *
And only one file in the working directory seems to be getting added to
the tarball so I think I may be doing something incorrect. Likewise with
listing contents of a tarball, only the top most files inside the
tarball is getting listed when using the -l switch so I think I might be
doing something wrong there, as well.
On 05/10/2014 04:25 AM, Shlomi Fish wrote:
Hi Mike,
you have not replied to my earlier E-mail about trying to explain
the code in MetaCPAN::API. Please do so, or else you'll follow this -
http://www.shlomifish.org/humour/fortunes/show.cgi?id=God_gave_us_two_eyes .
Also - please reply to all recipients - select the right option in GMail.com or
whatever your client is.
On Sat, 10 May 2014 04:01:06 -0500
Mike Dunaway <ekimduna...@gmail.com> wrote:
So I've just gotten back into Perl and I've written a tarring utility
for my first application. It seems to work okay, but I'm wondering how
it could better be written. Any ideas?
#!/usr/bin/perl -w
Do not use the -w flag - use "use warnings;" instead:
use strict;
use Archive::Tar;
use Getopt::Std;
Do not use Getopt::Std - use Getopt::Long instead.
use feature qw(say);
my %opt;
getopts('hcel', \%opt);
help() if $opt{h};
list() if $opt{l};
extract() if $opt{e};
compress() if $opt{c};
These options should be mutually exclusive - maybe do:
if ($opt{h})
{
help();
}
elsif ($opt{l})
{
list();
}
…
sub help {
say "Easily create and extract tarballs";
say " -h display this menu";
say " -c compress file/directory";
say " | eztar -c <file name you wish to make>
<file/directory you wish to compress>";
say " -e extract tarball";
say " | eztar -e <tarball>";
say " -l list contents of tarball";
say " | eztar -l <tarball>";
This should be a here document - https://en.wikipedia.org/wiki/Here_document .
}
sub list {
if (defined $ARGV[0]) {
my $tar = Archive::Tar->new();
for($tar->list_archive($ARGV[0])) {
Do not use position $ARGV[0] arguments, instead extract out of @ARGV:
http://perl-begin.org/tutorials/bad-elements/#subroutine-arguments
(Note that perl-begin.org is my domain).
Your indentation is wrong, as well.
Finally, do not use $_ for iteration - use a lexical variable instead:
http://perl-begin.org/tutorials/bad-elements/#overuse_dollar_underscore
say $_;
exit 0;
}
} else {
die 'No file provided.'
}
}
sub extract {
if (defined $ARGV[0]) {
my $tar = Archive::Tar->new();
$tar->read($ARGV[0]);
Indentation is off again.
$tar->extract();
} else {
die 'No file provided.';
}
}
sub compress {
my $file_name = $ARGV[0] or die 'No file name specified.';
if ($file_name !~ /.*\.tar\.gz$/) {
die 'File name must end with \'.tar.gz\'';
}
This regular expression match can be simplified into:
if ($file_name !~ /\.tar\.gz$/)
(without the .* which is unnecessary)
And please consider using \z for end-of-string instead of $.
my $files = $ARGV[1] or die 'No files specified.';
$files is in plural, but it is a scalar and a single string. What is your
intention? Also see:
http://perl-begin.org/tutorials/bad-elements/#calling-variables-file
my $tar = Archive::Tar->new();
$tar->write($file_name);
$tar->read($file_name);
$tar->add_files($files);
$tar->write($file_name, COMPRESS_GZIP);
So far so good.
Regards,
Shlomi Fish
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
