RM = '/bin/rm'
MV = '/bin/mv'
In general it's preferable to use the FileUtils
class rather than relying on shell utilities. Though in this particular case, you might want to stick at least with mv
since the FileUtils.mv
method does not have an :interactive
option.
tmp = "/tmp/renamer.#{Time.now.to_i}.#{(rand * 1000).to_i}"
Ruby has a Tempfile
class which can generate a unique temporary file more reliably than this. You should use it.
from.each {|i| f.puts i}
Calling puts
on an array will puts
each line individually, so the above can just be shortened to f.puts from
.
ENV['EDITOR'] = 'vi' if ENV['EDITOR'].nil?
Can be shortened to ENV['EDITOR'] ||= 'vi'
. Though what you have isn't particularly verbose either, so it doesn't really matter much which one you choose.
system("#{ENV['EDITOR']} #{tmp}")
Use system(ENV[EDITOR], tmp)
instead. This way you get rid of the string interpolation and the code still works if either ENV['EDITOR']
or tmp
should contain a space or other shell meta-character (not that they're particularly likely to, but it's a good idea to use the multiple-argument-form of system
where ever possible).
to = File.open(tmp) {|f| f.readlines.collect{|l| l.chomp}}
Usually this could be replaced with to = File.readlines(tmp).collect {|l| l.chomp}
. However if you follow my suggestion of using Tempfile
, that won't be an option any more.
`#{RM} #{tmp}`
If you use fileutils, this will just be FileUtils.rm(tmp)
(or rm(tmp)
if you include FileUtils
). If you don't want to use FileUtils
, you should at least use system(RM, tmp)
for the same reasons as above.
However if you use Tempfile
, which you should, this becomes redundant anyway.
from.each_with_index do |f, i|
puts `#{MV} -v --interactive "#{f}" "#{to[i]}"` unless f == to[i]
end
To iterate over two arrays in parallel, use zip
:
from.zip(to) do |f, t|
system(MV, "-v", "--interactive", f, t) unless f == t
end
Note that here using system
instead of backticks is especially important since one of the files in from
or to
containing spaces is actually somewhat likely.
So with all my suggestions, your code should now look like this:
#!/usr/bin/env ruby
require 'tempfile'
MV = '/bin/mv'
from = Dir.glob('*').sort
ENV['EDITOR'] ||= 'vi'
to = nil
Tempfile.open("renamer") do |f|
f.puts from
f.close
system(ENV['EDITOR'], f.path)
f.open
to = f.readlines.collect {|l| l.chomp}
end
if to.size != from.size
STDERR.puts "renamer: ERROR: number of lines changed"
exit(1)
end
from.zip(to) do |f, t|
system(MV, "-v", "--interactive", f, t) unless f == t
end