ruby-core

Issue #17294 has been updated by hsbt (Hiroshi SHIBATA).

Status changed from Assigned to Closed

This proposal has been approved by @akr. I merged https://github.com/ruby/ruby/pull/3705 now.

----------------------------------------
Feature #17294: Feature: Allow method chaining with Pathname#mkpath Pathname#rmtree
https://bugs.ruby-lang.org/issues/17294#change-110060

* Author: schneems (Richard Schneeman)
* Status: Closed
* Assignee: akr (Akira Tanaka)
----------------------------------------

Currently in my code when I want to create a pathname object and create a path at the same time I must use tap

```
path = Pathname.new("/tmp/new").tap(&:mkpath)
```

I think it would be cleaner to be able to chain on the results of these methods instead:

```
path = Pathname.new("/tmp/new").mkpath
```

This is a change in return value but after research on github I do not believe many (if any) are relying on the current behavior to return nil https://github.com/search?l=&p=1&q=.mkpath+language%3ARuby&ref=advsearch&type=Code.

Here is my diff: 

```
$ git diff master schneems/return-self-pathname
diff --git a/ext/pathname/lib/pathname.rb b/ext/pathname/lib/pathname.rb
index e6fb90277d..f1eb1e00ae 100644
--- a/ext/pathname/lib/pathname.rb
+++ b/ext/pathname/lib/pathname.rb
@@ -582,7 +582,7 @@ class Pathname    # * FileUtils *
   def mkpath
     require 'fileutils'
     FileUtils.mkpath(@path)
-    nil
+    self
   end

   # Recursively deletes a directory, including all directories beneath it.
@@ -593,7 +593,7 @@ def rmtree
     # File::Path provides "mkpath" and "rmtree".
     require 'fileutils'
     FileUtils.rm_r(@path)
-    nil
+    self
   end
 end

diff --git a/test/pathname/test_pathname.rb b/test/pathname/test_pathname.rb
index 43cef4849f..149fe15c3a 100644
--- a/test/pathname/test_pathname.rb
+++ b/test/pathname/test_pathname.rb
@@ -1389,7 +1389,8 @@ def test_find

   def test_mkpath
     with_tmpchdir('rubytest-pathname') {|dir|
-      Pathname("a/b/c/d").mkpath
+      path = Pathname("a/b/c/d")
+      assert_equal(path, path.mkpath)
       assert_file.directory?("a/b/c/d")
     }
   end
@@ -1398,7 +1399,8 @@ def test_rmtree
     with_tmpchdir('rubytest-pathname') {|dir|
       Pathname("a/b/c/d").mkpath
       assert_file.exist?("a/b/c/d")
-      Pathname("a").rmtree
+      path = Pathname("a")
+      assert_equal(path, path.rmtree)
       assert_file.not_exist?("a")
     }
   end
```

Github PR: https://github.com/ruby/ruby/pull/3705. If accepted I will make a pr to update the tests here as well https://github.com/ruby/rbs/blob/b0dee64fdd00cc41c0729fa2c239fc2dcb9c3b18/test/stdlib/Pathname_test.rb#L456-L463.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- [email protected]
 To unsubscribe send an email to [email protected]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

Thread

Prev Next

In This Thread

Prev Next