Hi David,
In article <[EMAIL PROTECTED]>, David wrote:
> Kevin Pfeiffer wrote:
>
>> Hi all,
>>
>> I just took another look at an exercise I wrote a couple months ago and
>> fleshed it out a bit. It is a commandline "trashcan" program which I
>> actually use. Instead of typing 'rm' I almost always use 'rmm' now.
>>
>> It's meant to be used in a "friendly" environment, nonetheless, if anyone
>> sees any potentials for error (or suggestions for better work) please say
>> so...
>>
>
> i didn't read your whole script, only the first 20 lines or so and i have
> a few suggestions:
>
> 1. you have:
>
> if ($ARGV[0] =~ /^-(.)$/) {
>
> can your program handle files starting with a '-'? it's legal in *nix for
> file names to begin with a '-':
[...]
> i understand that you can still supply files to rmm starting with '-'
> like:
>
> [panda]$ rmm ./-remove-this-file
> [panda]$ rmm /home/dzhuo/-remove-this-file
>
> but i think it's still nice (maybe) to be able to just:
>
> [panda]$ rmm -remove-this-file
You would have to escape the dash: 'rmm --remove-this-file'.
I don't see how even GetOpts can cover all possibilities. How do you
distinguish between 'rmm -l' (list contents of trash) and 'rmm -l' (delete
a file named "-l")? (Other than by having to escape these possibilities.)
> 2. you have:
>
> ($trash_dir) or die "Error: No trash directory defined!\n";
>
> which means you will not allow user to name his/her trash directory '0'?
> for a general utility program like your rmm, you should relax this
> restriction. btw, $trash_dir is hard coded in your script so how can it
> ever be false?
It's not meant to be changed, though as you mention, one could modify the
script. It's meant more as a protection to ensure that a trash dir has been
set, so that one doesn't accidentally issue a delete all command in
$ENV{HOME}/.
>
> 3. you have:
>
> $path =~ /.*\..+$/ or die "Name of trash directory must begin with a dot
> followed by one or more letters!\n";
>
> your regex does not match your die message.
Oops!
> 4. you have:
>
> unless (-e $path) {
> print "Creating trash directory: $path\n";
> mkdir $path or die "Couldn't create $path: $!\n";
> }
>
> don't do this for a general utility program where your program might be
> involved very often from different scripts. you have a race condition. do
> something similar to the following instead:
>
> [panda]$ perl -MErrno -e 'die $! unless(mkdir('foo') || $!{EEXIST});'
This says "die unless you can mkdir or you get an error message that it
already exists"? I understand your point, but in this situation (creating a
trash directory) I don't see the danger. If I try mkdir on an already
existing directory I get the error message that it already exists. So if I
check for the existence, then try to mkdir and in between another process
has created the same directory, won't I simply get an error message that
the dir already exists? I've made the change, but am trying to follow the
reasoning in this particular case.
[...]
> 6. see if Fild::Find can help you simplify some of your dir scanning code.
I also just noticed that my glob did not handle dot-files (.my_file). I've
fixed this.
> 7. it will be nice to set a limit on how big your trash dir can be.
> otherwise, it's easy for people to give putting stuff into it and forget
> about how big it has grown to.
Sounds good; perhaps also a possible auto-empty function. I also want to add
commandline completion for restoring files (moving them out of the trash) -
or else a down/up arrow function that scrolls through the list of files.
Thanks for your help and advice!
-K
--
Kevin Pfeiffer
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]