From: Evan Phoenix <evan@...>
Date: 2006-03-28T03:03:13+09:00
Subject: Re: PATCH: A subclassable Pathname

------=_Part_3404_4421603.1143482523990
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Alright, heres a new patch with the glob and getwd fixed (with 2 tests
for them).

Also, this patch makes the behavior closer to what the user expects
anyway. Without the patch, any subclasses still emit Pathname's. At
least with the patch, there is a much better chance that .+, .join,
and other will return the type the user expects from a subclass.

 - Evan

On 3/27/06, Evan Phoenix <evanwebb@gmail.com> wrote:
> I think that this behavior is perfectly acceptable. The return value
> is the same class as the primary Pathname (ie, the left hand side
> one). Unlike numerics where coerce can decide what the return value
> should be, the programmer here is the one that really should decide
> anyway. If they use a Pathname with an EnhancedPathname as an
> argument, they should expect to get a Pathname back.
>
> As you've said, Array has the same issue, and I think that ruby
> programmers have become accustom to this kind of behavior (I know I
> have).
>
> All fix the getwd and glob, add 2 tests, and send in the new patch in
> a few minutes.
>
>  - Evan
>
> On 3/27/06, Tanaka Akira <akr@m17n.org> wrote:
> > In article <92f5f81d0603270903g2fb02244i6a395be708dfffa3@mail.gmail.com=
>,
> >   "Evan Phoenix" <evan@fallingsnow.net> writes:
> >
> > > Quite right on the .glob and .getwd. I guess the tests don't test hit
> > > those methods, I'll add some quick tests for those. Also, why is it
> > > not so simple when a Pathname is an argument?
> > >
> > > For instance, the code for join tests that each argument is a Pathnam=
e
> > > (which will come up as true for subclasses). The it simply uses
> > > self.class.new to create the return value.
> >
> > The class of an argument may be different from
> > self.class.new.
> >
> > For example, if P inherits Pathname,
> > Pathname.new("...") + P.new("...") returns an instance of
> > Pathname.
> >
> > Is it useful? intuitive? ...
> >
> > Note that Array has a similar issue too.
> >
> > % ruby -ve '
> > class A1 < Array; end
> > class A2 < Array; end
> > p((A1.new + A2.new).class)
> > '
> > ruby 1.9.0 (2006-03-04) [i686-linux]
> > Array
> > --
> > Tanaka Akira
> >
> >
>
>
> --
> When I do good, I feel good;  when I do bad, I feel bad,
> and that is my religion.
>     -- Abraham Lincoln (1809 - 1865)
>
>


--
When I do good, I feel good;  when I do bad, I feel bad,
and that is my religion.
    -- Abraham Lincoln (1809 - 1865)

------=_Part_3404_4421603.1143482523990
Content-Type: application/octet-stream; name=subclassable-pathname.diff
Content-Transfer-Encoding: 7bit
X-Attachment-Id: f_gvxggj
Content-Disposition: attachment; filename="subclassable-pathname.diff"

--- /usr/local/lib/ruby/1.8/pathname.rb	2005-07-10 08:17:35.000000000 -0700
+++ ./pathname.rb	2006-03-27 09:49:31.000000000 -0800
@@ -258,7 +258,7 @@
     # cleanpath_aggressive assumes:
     # * no symlink
     # * all pathname prefix contained in the pathname is existing directory
-    return Pathname.new('') if @path == ''
+    return self.class.new('') if @path == ''
     absolute = absolute?
     names = []
     @path.scan(%r{[^/]+}) {|name|
@@ -275,20 +275,20 @@
       end
       names << name
     }
-    return Pathname.new(absolute ? '/' : '.') if names.empty?
+    return self.class.new(absolute ? '/' : '.') if names.empty?
     path = absolute ? '/' : ''
     path << names.join('/')
-    Pathname.new(path)
+    self.class.new(path)
   end
   private :cleanpath_aggressive
 
   def cleanpath_conservative
-    return Pathname.new('') if @path == ''
+    return self.class.new('') if @path == ''
     names = @path.scan(%r{[^/]+})
     last_dot = names.last == '.'
     names.delete('.')
     names.shift while names.first == '..' if absolute?
-    return Pathname.new(absolute? ? '/' : '.') if names.empty?
+    return self.class.new(absolute? ? '/' : '.') if names.empty?
     path = absolute? ? '/' : ''
     path << names.join('/')
     if names.last != '..'
@@ -298,7 +298,7 @@
         path << '/'
       end
     end
-    Pathname.new(path)
+    self.class.new(path)
   end
   private :cleanpath_conservative
 
@@ -363,9 +363,9 @@
     end
     
     if resolved.empty?
-      Pathname.new(top.empty? ? '.' : '/')
+      self.class.new(top.empty? ? '.' : '/')
     else
-      Pathname.new(top + resolved.join('/'))
+      self.class.new(top + resolved.join('/'))
     end
   end
 
@@ -431,7 +431,7 @@
   # This method doesn't access the file system; it is pure string manipulation. 
   #
   def +(other)
-    other = Pathname.new(other) unless Pathname === other
+    other = self.class.new(other) unless Pathname === other
 
     return other if other.absolute?
 
@@ -449,13 +449,13 @@
       end
     end
 
-    return Pathname.new(path2) if path1 == '.'
-    return Pathname.new(path1) if path2 == '.'
+    return self.class.new(path2) if path1 == '.'
+    return self.class.new(path1) if path2 == '.'
 
     if %r{/\z} =~ path1
-      Pathname.new(path1 + path2)
+      self.class.new(path1 + path2)
     else
-      Pathname.new(path1 + '/' + path2)
+      self.class.new(path1 + '/' + path2)
     end
   end
 
@@ -468,10 +468,10 @@
   def join(*args)
     args.unshift self
     result = args.pop
-    result = Pathname.new(result) unless Pathname === result
+    result = self.class.new(result) unless Pathname === result
     return result if result.absolute?
     args.reverse_each {|arg|
-      arg = Pathname.new(arg) unless Pathname === arg
+      arg = self.class.new(arg) unless Pathname === arg
       result = arg + result
       return result if result.absolute?
     }
@@ -505,9 +505,9 @@
     Dir.foreach(@path) {|e|
       next if e == '.' || e == '..'
       if with_directory
-        result << Pathname.new(File.join(@path, e))
+        result << self.class.new(File.join(@path, e))
       else
-        result << Pathname.new(e)
+        result << self.class.new(e)
       end
     }
     result
@@ -554,9 +554,9 @@
     base.fill '..'
     relpath = base + dest
     if relpath.empty?
-      Pathname.new(".")
+      self.class.new(".")
     else
-      Pathname.new(relpath.join('/'))
+      self.class.new(relpath.join('/'))
     end
   end
 
@@ -635,7 +635,7 @@
   end
 
   # See <tt>File.readlink</tt>.  Read symbolic link.
-  def readlink() Pathname.new(File.readlink(@path)) end
+  def readlink() self.class.new(File.readlink(@path)) end
 
   # See <tt>File.rename</tt>.  Rename the file.
   def rename(to) File.rename(@path, to) end
@@ -656,20 +656,20 @@
   def utime(atime, mtime) File.utime(atime, mtime, @path) end
 
   # See <tt>File.basename</tt>.  Returns the last component of the path.
-  def basename(*args) Pathname.new(File.basename(@path, *args)) end
+  def basename(*args) self.class.new(File.basename(@path, *args)) end
 
   # See <tt>File.dirname</tt>.  Returns all but the last component of the path.
-  def dirname() Pathname.new(File.dirname(@path)) end
+  def dirname() self.class.new(File.dirname(@path)) end
 
   # See <tt>File.extname</tt>.  Returns the file's extension.
   def extname() File.extname(@path) end
 
   # See <tt>File.expand_path</tt>.
-  def expand_path(*args) Pathname.new(File.expand_path(@path, *args)) end
+  def expand_path(*args) self.class.new(File.expand_path(@path, *args)) end
 
   # See <tt>File.split</tt>.  Returns the #dirname and the #basename in an
   # Array.
-  def split() File.split(@path).map {|f| Pathname.new(f) } end
+  def split() File.split(@path).map {|f| self.class.new(f) } end
 
   # Pathname#link is confusing and *obsoleted* because the receiver/argument
   # order is inverted to corresponding system call.
@@ -761,14 +761,14 @@
   # See <tt>Dir.glob</tt>.  Returns or yields Pathname objects.
   def Pathname.glob(*args) # :yield: p
     if block_given?
-      Dir.glob(*args) {|f| yield Pathname.new(f) }
+      Dir.glob(*args) {|f| yield new(f) }
     else
-      Dir.glob(*args).map {|f| Pathname.new(f) }
+      Dir.glob(*args).map {|f| new(f) }
     end
   end
 
   # See <tt>Dir.getwd</tt>.  Returns the current working directory as a Pathname.
-  def Pathname.getwd() Pathname.new(Dir.getwd) end
+  def Pathname.getwd() new(Dir.getwd) end
   class << self; alias pwd getwd end
 
   # Pathname#chdir is *obsoleted* at 1.8.1.
@@ -785,14 +785,14 @@
 
   # Return the entries (files and subdirectories) in the directory, each as a
   # Pathname object.
-  def entries() Dir.entries(@path).map {|f| Pathname.new(f) } end
+  def entries() Dir.entries(@path).map {|f| self.class.new(f) } end
 
   # Iterates over the entries (files and subdirectories) in the directory.  It
   # yields a Pathname object for each entry.
   #
   # This method has existed since 1.8.1.
   def each_entry(&block) # :yield: p
-    Dir.foreach(@path) {|f| yield Pathname.new(f) }
+    Dir.foreach(@path) {|f| yield self.class.new(f) }
   end
 
   # Pathname#dir_foreach is *obsoleted* at 1.8.1.
@@ -828,9 +828,9 @@
   def find(&block) # :yield: p
     require 'find'
     if @path == '.'
-      Find.find(@path) {|f| yield Pathname.new(f.sub(%r{\A\./}, '')) }
+      Find.find(@path) {|f| yield self.class.new(f.sub(%r{\A\./}, '')) }
     else
-      Find.find(@path) {|f| yield Pathname.new(f) }
+      Find.find(@path) {|f| yield self.class.new(f) }
     end
   end
 end
@@ -1195,5 +1195,17 @@
       assert_equal(1, count)
       assert_equal(2, result)
     end
+    
+    def test_s_glob
+      ary = []
+      Pathname.glob("*") { |f| ary << f }
+      slf = ary.find { |e| e.to_s == __FILE__ }
+      assert slf
+    end
+    
+    def test_s_getwd
+      pn = Pathname.getwd
+      assert_equal Dir.getwd, pn.to_s
+    end
   end
 end


------=_Part_3404_4421603.1143482523990--