diff --git a/CHANGELOG.md b/CHANGELOG.md index c220baf12e43..bbbab3e7dff3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,7 @@ Compatibility: * Update to JCodings 1.0.58 and Joni 2.1.44 (@eregon). * Add `MatchData#match` and `MatchData#match_length` (#2733, @horakivo). * Add `StructClass#keyword_init?` method (#2377, @moste00). +* Support optional `level` argument for `File.dirname` method (#2733, @moste00). Performance: diff --git a/spec/ruby/core/file/dirname_spec.rb b/spec/ruby/core/file/dirname_spec.rb index cf0f909f5937..8dd6c4ca8815 100644 --- a/spec/ruby/core/file/dirname_spec.rb +++ b/spec/ruby/core/file/dirname_spec.rb @@ -12,18 +12,33 @@ end ruby_version_is '3.1' do - it "returns all the components of filename except the last parts by the level" do - File.dirname('/home/jason', 2).should == '/' - File.dirname('/home/jason/poot.txt', 2).should == '/home' - end - - it "returns the same string if the level is 0" do - File.dirname('poot.txt', 0).should == 'poot.txt' - File.dirname('/', 0).should == '/' - end - - it "raises ArgumentError if the level is negative" do - -> {File.dirname('/home/jason', -1)}.should raise_error(ArgumentError) + context "when level is passed" do + it "returns all the components of filename except the last parts by the level" do + File.dirname('/home/jason', 2).should == '/' + File.dirname('/home/jason/poot.txt', 2).should == '/home' + end + + it "returns the same String if the level is 0" do + File.dirname('poot.txt', 0).should == 'poot.txt' + File.dirname('/', 0).should == '/' + end + + it "raises ArgumentError if the level is negative" do + -> { + File.dirname('/home/jason', -1) + }.should raise_error(ArgumentError, "negative level: -1") + end + + it "returns '/' when level exceeds the number of segments in the path" do + File.dirname("/home/jason", 100).should == '/' + end + + it "calls #to_int if passed not numeric value" do + object = Object.new + def object.to_int; 2; end + + File.dirname("/a/b/c/d", object).should == '/a/b' + end end end diff --git a/spec/tags/core/file/dirname_tags.txt b/spec/tags/core/file/dirname_tags.txt deleted file mode 100644 index 3db43d88f94e..000000000000 --- a/spec/tags/core/file/dirname_tags.txt +++ /dev/null @@ -1,2 +0,0 @@ -fails:File.dirname returns all the components of filename except the last parts by the level -fails:File.dirname returns the same string if the level is 0 diff --git a/src/main/ruby/truffleruby/core/file.rb b/src/main/ruby/truffleruby/core/file.rb index c90e0ed980f4..82f1ca510268 100644 --- a/src/main/ruby/truffleruby/core/file.rb +++ b/src/main/ruby/truffleruby/core/file.rb @@ -395,19 +395,6 @@ def self.directory?(io_or_path) Truffle::StatOperations.directory?(mode) end - def self.last_nonslash(path, start = nil) - # Find the first non-/ from the right - data = path.bytes - start ||= (path.size - 1) - - start.downto(0) do |i| - if data[i] != 47 # ?/ - return i - end - end - nil - end - ## # Returns all components of the filename given in # file_name except the last one. The filename must be @@ -415,37 +402,25 @@ def self.last_nonslash(path, start = nil) # the separator used on the local file system. # # File.dirname("/home/gumby/work/ruby.rb") #=> "/home/gumby/work" - def self.dirname(path) + def self.dirname(path, level = 1) path = Truffle::Type.coerce_to_path(path) + level = Primitive.rb_num2int(level) - # edge case - return +'.' if path.empty? - - slash = '/' - - # pull off any /'s at the end to ignore - chunk_size = last_nonslash(path) - return +'/' unless chunk_size + raise ArgumentError, "negative level: #{level}" if level < 0 + return path if level == 0 - if pos = Primitive.find_string_reverse(path, slash, chunk_size) - return +'/' if pos == 0 - - path = path.byteslice(0, pos) - - return +'/' if path == '/' - - return path unless path.end_with? slash - - # prune any trailing /'s - idx = last_nonslash(path, pos) - - # edge case, only /'s, return / - return +'/' unless idx + # fast path + if level == 1 + return +'.' if path.empty? + return Truffle::FileOperations.dirname(path) + end - return path.byteslice(0, idx - 1) + level.times do + return +'.' if path.empty? + path = Truffle::FileOperations.dirname(path) end - +'.' + path end ## diff --git a/src/main/ruby/truffleruby/core/truffle/file_operations.rb b/src/main/ruby/truffleruby/core/truffle/file_operations.rb index 8ff567137258..ec23e37f59e3 100644 --- a/src/main/ruby/truffleruby/core/truffle/file_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/file_operations.rb @@ -78,5 +78,47 @@ def self.expand_path(path, dir, expand_tilde) def self.exist?(path) Truffle::POSIX.truffleposix_stat_mode(path) > 0 end + + def self.dirname(path) + slash = '/' + + # pull off any /'s at the end to ignore + chunk_size = last_nonslash(path) + return +'/' unless chunk_size + + if pos = Primitive.find_string_reverse(path, slash, chunk_size) + return +'/' if pos == 0 + + path = path.byteslice(0, pos) + + return +'/' if path == '/' + + return path unless path.end_with? slash + + # prune any trailing /'s + idx = last_nonslash(path, pos) + + # edge case, only /'s, return / + return +'/' unless idx + + return path.byteslice(0, idx - 1) + end + + +'.' + end + + def self.last_nonslash(path, start = nil) + # Find the first non-/ from the right + data = path.bytes + start ||= (path.size - 1) + + start.downto(0) do |i| + if data[i] != 47 # ?/ + return i + end + end + + nil + end end end diff --git a/test/mri/excludes/TestFileExhaustive.rb b/test/mri/excludes/TestFileExhaustive.rb index d2fd5edd07c2..ed5979be9899 100644 --- a/test/mri/excludes/TestFileExhaustive.rb +++ b/test/mri/excludes/TestFileExhaustive.rb @@ -15,6 +15,5 @@ exclude :test_expand_path_for_existent_username, "needs investigation" exclude :test_readlink_long_path, "needs investigation" exclude :test_utime, "needs investigation" -exclude :test_dirname, "ArgumentError: wrong number of arguments (given 2, expected 1)" exclude :test_flock_shared, "Zlib::InProgressError: zlib stream is in progress" exclude :test_flock_exclusive, "Zlib::InProgressError: zlib stream is in progress"