A text editor driven file renamer in Ruby

Question

I wrote the following Ruby script several years ago and have been using it often ever since.

It opens a text editor with the list of files in current directory. You can then edit the file names as text. Once you save and exit the files are renamed.

The renamer allowed me to maintain spreadsheet-like file names thanks to the powerful column editing capabilities of vi (emacs also has those).

Here is how one such directory looks:

I would like to use it as an example for a Ruby workshop. Could you please suggest best-practice and design pattern improvements?

#!/usr/bin/ruby

RM = '/bin/rm'
MV = '/bin/mv'

from = Dir.entries('.').sort; from.delete('.'); from.delete('..')
from.sort!

from.delete_if {|i| i =~ /^\./} # Hidden files

tmp = "/tmp/renamer.#{Time.now.to_i}.#{(rand * 1000).to_i}"

File.open(tmp, 'w') do |f|
  from.each {|i| f.puts i}
end

ENV['EDITOR'] = 'vi' if ENV['EDITOR'].nil?
system("#{ENV['EDITOR']} #{tmp}")

to = File.open(tmp) {|f| f.readlines.collect{|l| l.chomp}}
`#{RM} #{tmp}`

if to.size != from.size
  STDERR.puts "renamer: ERROR: number of lines changed"
  exit(1)
end

from.each_with_index do |f, i|
  puts `#{MV} -v --interactive "#{f}" "#{to[i]}"` unless f == to[i]
end

Answer 1

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

Answer 2

Just a couple of things:

#!/usr/bin/ruby

It's possible that on some systems this isn't where ruby lives, it's better to do:

#!/usr/bin/env ruby

---

from = Dir.entries('.').sort; from.delete('.'); from.delete('..')

Can be written as

from = Dir.entries('.').sort - ['.', '..']

Which is more succinct and eliminates having three statements on one line (which you shouldn't do).

Or you could eliminate hidden files and . / .. in one go with:

from = Dir.entries(.).select do |filename|
    filename[0].chr != '.'
end

Edit:

from = Dir.glob("*").sort

Is definitely the best solution.