Re: PATCH: A subclassable Pathname
From:
"Evan Phoenix" <evan@...>
Date:
2006-03-27 18:03:13 UTC
List:
ruby-core #7622
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 Pathname
> > > (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)
Attachments (1)
subclassable-pathname.diff
(6.79 KB, text/x-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