########################################################### ### not gonna complain about strict, for whatever reason use MP3::Info; die "Must use parameters and seperator\n (SEPERATOR ARTIST TITLE YEAR +ALBUM GENRE COMMENT)" unless @ARGV; $sep=shift; @tokens=@ARGV; while(<*.mp3>) # while is wrong as you move files around, # should be for (grep /\.mp3$/i, <*>) # to insure getting files like "bla.MP3" ### indent your code! { my $tag=get_mp3tag($_) or (push(@errors,"No tag for $_\n") and next); $nn=''; ### now you're using as a separator ### I wanted only. for $i (@tokens){$nn.=$tag->{$i}." $sep ";} my $z=rindex($nn,$sep); my $len=length $sep; substr($nn,$z,$len+1,''); chop $nn; $nn.='.mp3'; ### the last 6 lines should've been something like: ### $nn = join($sep, map $tag->{$_}, @tokens) . ".mp3"; ### and this is not my personal preferrence, but a more ### readable, straight to the point, honest to god statement print "Renamed $_ to $nn \n" if rename $_,$nn; ### where is my error check? } print "\n\n"; print @errors;