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]