Implemented yielding Dir.each_child (#4811)

Sija · ysbaddaden · commit 8de1d8ed14b6 · 2017-08-14T00:32:52.000+02:00
* Renamed Dir.foreach to Dir.each_entry
* Renamed Dir#each to Dir#each_entry
* Added Dir#entries and Dir#each_child/children instance methods
* Added Dir#each_child iterator
diff --git a/spec/std/dir_spec.cr b/spec/std/dir_spec.cr
@@ -234,7 +234,7 @@ describe "Dir" do
     filenames = [] of String
 
     dir = Dir.new(__DIR__)
-    dir.each do |filename|
+    dir.each_entry do |filename|
       filenames << filename
     end.should be_nil
     dir.close
@@ -246,7 +246,7 @@ describe "Dir" do
     filenames = [] of String
 
     Dir.open(__DIR__) do |dir|
-      dir.each do |filename|
+      dir.each_entry do |filename|
         filenames << filename
       end.should be_nil
     end
@@ -272,11 +272,26 @@ describe "Dir" do
   it "gets dir iterator" do
     filenames = [] of String
 
-    iter = Dir.new(__DIR__).each
+    iter = Dir.new(__DIR__).each_entry
     iter.each do |filename|
       filenames << filename
     end
 
+    filenames.includes?(".").should be_true
+    filenames.includes?("..").should be_true
+    filenames.includes?("dir_spec.cr").should be_true
+  end
+
+  it "gets child iterator" do
+    filenames = [] of String
+
+    iter = Dir.new(__DIR__).each_child
+    iter.each do |filename|
+      filenames << filename
+    end
+
+    filenames.includes?(".").should be_false
+    filenames.includes?("..").should be_false
     filenames.includes?("dir_spec.cr").should be_true
   end
 
diff --git a/src/compiler/crystal/codegen/cache_dir.cr b/src/compiler/crystal/codegen/cache_dir.cr
@@ -114,9 +114,7 @@ module Crystal
     end
 
     private def gather_cache_entries(dir)
-      Dir.entries(dir)
-         .reject { |name| name == "." || name == ".." }
-         .map! { |name| File.join(dir, name) }
+      Dir.children(dir).map! { |name| File.join(dir, name) }
     end
   end
 end
diff --git a/src/compiler/crystal/crystal_path.cr b/src/compiler/crystal/crystal_path.cr
@@ -127,11 +127,11 @@ module Crystal
       files = [] of String
       dirs = [] of String
 
-      Dir.foreach(dir) do |filename|
+      Dir.each_child(dir) do |filename|
         full_name = "#{dir}/#{filename}"
 
         if File.directory?(full_name)
-          if filename != "." && filename != ".." && recursive
+          if recursive
             dirs << filename
           end
         else
diff --git a/src/dir.cr b/src/dir.cr
@@ -48,7 +48,7 @@ class Dir
   # File.write("testdir/config.h", "")
   #
   # d = Dir.new("testdir")
-  # d.each { |x| puts "Got #{x}" }
+  # d.each_entry { |x| puts "Got #{x}" }
   # ```
   #
   # produces:
@@ -58,16 +58,62 @@ class Dir
   # Got ..
   # Got config.h
   # ```
-  def each : Nil
+  def each_entry : Nil
     while entry = read
       yield entry
     end
   end
 
-  def each
+  def each_entry
     EntryIterator.new(self)
   end
 
+  # Returns an array containing all of the filenames in the given directory.
+  def entries : Array(String)
+    entries = [] of String
+    each_entry do |filename|
+      entries << filename
+    end
+    entries
+  end
+
+  # Calls the block once for each entry except for `.` and `..` in this directory,
+  # passing the filename of each entry as a parameter to the block.
+  #
+  # ```
+  # Dir.mkdir("testdir")
+  # File.write("testdir/config.h", "")
+  #
+  # d = Dir.new("testdir")
+  # d.each_entry { |x| puts "Got #{x}" }
+  # ```
+  #
+  # produces:
+  #
+  # ```text
+  # Got config.h
+  # ```
+  def each_child : Nil
+    excluded = {".", ".."}
+    while entry = read
+      yield entry unless excluded.includes?(entry)
+    end
+  end
+
+  def each_child
+    ChildIterator.new(self)
+  end
+
+  # Returns an array containing all of the filenames except for `.` and `..`
+  # in the given directory.
+  def children : Array(String)
+    entries = [] of String
+    each_child do |filename|
+      entries << filename
+    end
+    entries
+  end
+
   # Reads the next entry from dir and returns it as a string. Returns `nil` at the end of the stream.
   #
   # ```
@@ -135,34 +181,36 @@ class Dir
     end
   end
 
-  # Calls the block once for each entry in the named directory,
-  # passing the filename of each entry as a parameter to the block.
-  def self.foreach(dirname)
+  # See `#each_entry`.
+  def self.each_entry(dirname)
     Dir.open(dirname) do |dir|
-      dir.each do |filename|
+      dir.each_entry do |filename|
         yield filename
       end
     end
   end
 
-  # Returns an array containing all of the filenames in the given directory.
+  # See `#entries`.
   def self.entries(dirname) : Array(String)
-    entries = [] of String
-    foreach(dirname) do |filename|
-      entries << filename
+    Dir.open(dirname) do |dir|
+      return dir.entries
     end
-    entries
   end
 
-  # Returns an array containing all of the filenames except for `.` and `..`
-  # in the given directory.
+  # See `#each_child`.
+  def self.each_child(dirname)
+    Dir.open(dirname) do |dir|
+      dir.each_child do |filename|
+        yield filename
+      end
+    end
+  end
+
+  # See `#children`.
   def self.children(dirname) : Array(String)
-    excluded = {".", ".."}
-    entries = [] of String
-    foreach(dirname) do |filename|
-      entries << filename unless excluded.includes?(filename)
+    Dir.open(dirname) do |dir|
+      return dir.children
     end
-    entries
   end
 
   # Returns `true` if the given path exists and is a directory
@@ -189,8 +237,8 @@ class Dir
   def self.empty?(path) : Bool
     raise Errno.new("Error determining size of '#{path}'") unless exists?(path)
 
-    foreach(path) do |f|
-      return false unless {".", ".."}.includes?(f)
+    each_child(path) do |f|
+      return false
     end
     true
   end
@@ -261,6 +309,26 @@ class Dir
       self
     end
   end
+
+  private struct ChildIterator
+    include Iterator(String)
+
+    def initialize(@dir : Dir)
+    end
+
+    def next
+      excluded = {".", ".."}
+      while entry = @dir.read
+        return entry unless excluded.includes?(entry)
+      end
+      stop
+    end
+
+    def rewind
+      @dir.rewind
+      self
+    end
+  end
 end
 
 require "./dir/*"
diff --git a/src/file_utils.cr b/src/file_utils.cr
@@ -136,14 +136,10 @@ module FileUtils
   def cp_r(src_path : String, dest_path : String)
     if Dir.exists?(src_path)
       Dir.mkdir(dest_path)
-      Dir.open(src_path) do |dir|
-        dir.each do |entry|
-          if entry != "." && entry != ".."
-            src = File.join(src_path, entry)
-            dest = File.join(dest_path, entry)
-            cp_r(src, dest)
-          end
-        end
+      Dir.each_child(src_path) do |entry|
+        src = File.join(src_path, entry)
+        dest = File.join(dest_path, entry)
+        cp_r(src, dest)
       end
     else
       cp(src_path, dest_path)
@@ -268,13 +264,9 @@ module FileUtils
   # ```
   def rm_r(path : String) : Nil
     if Dir.exists?(path) && !File.symlink?(path)
-      Dir.open(path) do |dir|
-        dir.each do |entry|
-          if entry != "." && entry != ".."
-            src = File.join(path, entry)
-            rm_r(src)
-          end
-        end
+      Dir.each_child(path) do |entry|
+        src = File.join(path, entry)
+        rm_r(src)
       end
       Dir.rmdir(path)
     else
diff --git a/src/http/server/handlers/static_file_handler.html b/src/http/server/handlers/static_file_handler.html
@@ -7,8 +7,7 @@
     <h2>Directory listing for <%= HTML.escape request_path %></h2>
     <hr/>
     <ul>
-      <% Dir.foreach(path) do |entry| %>
-        <% next if entry == "." || entry == ".." %>
+      <% Dir.each_child(path) do |entry| %>
         <li>
           <a href="<%= request_path == "/" ? "" : escaped_request_path %>/<%= URI.escape entry %>"><%= HTML.escape entry %></a>
         </li>
