Merge pull request #138 from danielpclark/del_trailing_separator

Del trailing separator

Author: danielpclark

README.md

@@ -136,14 +136,15 @@ improvement result  for the `chop_basename` method.
 
 Current methods implemented:
 
-|FasterPath Rust Implementation|Ruby 2.3.4 Implementation|Time Shaved Off|
+|FasterPath Rust Implementation|Ruby 2.5.0 Implementation|Time Shaved Off|
 |---|---|:---:|
 | `FasterPath.absolute?` | `Pathname#absolute?` | 91.9% |
 | `FasterPath.add_trailing_separator` | `Pathname#add_trailing_separator` | 31.2% |
 | `FasterPath.children` | `Pathname#children` | 13.2% |
 | `FasterPath.chop_basename` | `Pathname#chop_basename` | 54.5% |
 | `FasterPath.cleanpath_aggressive` | `Pathname#cleanpath_aggressive` | 73.8% |
 | `FasterPath.cleanpath_conservative` | `Pathname#cleanpath_conservative` | 70.7% |
+| `FasterPath.del_trailing_separator` | `Pathname#del_trailing_separator` | 80.6% |
 | `FasterPath.directory?` | `Pathname#directory?` | 11.3% |
 | `FasterPath.entries` | `Pathname#entries` | 8.4% |
 | `FasterPath.has_trailing_separator?` | `Pathname#has_trailing_separator` | 67.6% |

lib/faster_path/optional/monkeypatches.rb

@@ -57,6 +57,11 @@ def cleanpath_conservative
         end
         private :cleanpath_conservative
 
+        def del_trailing_separator(pth)
+          FasterPath.del_trailing_separator(pth)
+        end
+        private :del_trailing_separator
+
         def directory?
           FasterPath.directory?(@path)
         end

lib/faster_path/optional/refinements.rb

@@ -53,6 +53,11 @@ def cleanpath_conservative
       end
       private :cleanpath_conservative
 
+      def del_trailing_separator(pth)
+        FasterPath.del_trailing_separator(pth)
+      end
+      private :del_trailing_separator
+
       def directory?
         FasterPath.directory?(@path)
       end

src/helpers.rs

@@ -1,12 +1,5 @@
 use ruru::{RString, Object, Class, AnyObject};
-
-#[inline]
-pub fn new_pathname_instance(path: &str) -> Class {
-  let mut instance = Class::from_existing("Pathname").allocate();
-  instance.instance_variable_set("@path", RString::new(path).to_any_object());
-
-  instance
-}
+use pathname::Pathname;
 
 pub fn anyobject_to_string(item: AnyObject) -> Result<String, RubyDebugInfo> {
   let result = &item;
@@ -35,7 +28,7 @@ pub fn anyobject_to_string(item: AnyObject) -> Result<String, RubyDebugInfo> {
 #[allow(dead_code)]
 pub fn into_pathname(itself: AnyObject) -> Result<AnyObject, RubyDebugInfo> {
   if Class::from_existing("String").case_equals(&itself) {
-    Ok(new_pathname_instance(
+    Ok(Pathname::new(
         &RString::from(itself.value()).to_string()
     ).to_any_object())
   } else if Class::from_existing("Pathname").case_equals(&itself) {

src/lib.rs

@@ -71,7 +71,9 @@ methods!(
     pathname::pn_cleanpath_conservative(pth)
   }
 
-  // fn r_del_trailing_separator(pth: RString){}
+  fn pub_del_trailing_separator(pth: RString) -> RString {
+    pathname::pn_del_trailing_separator(pth)
+  }
 
   // fn r_descend(){}
 
@@ -151,6 +153,7 @@ pub extern "C" fn Init_faster_pathname(){
   Module::from_existing("FasterPath").define(|itself| {
     itself.def_self("absolute?", pub_is_absolute);
     itself.def_self("add_trailing_separator", pub_add_trailing_separator);
+    itself.def_self("del_trailing_separator", pub_del_trailing_separator);
     itself.def_self("cleanpath_aggressive", pub_cleanpath_aggressive);
     itself.def_self("cleanpath_conservative", pub_cleanpath_conservative);
     itself.def_self("directory?", pub_is_directory);

src/pathname.rs

@@ -8,14 +8,56 @@ use extname;
 use plus;
 
 use ruru;
-use ruru::{RString, Boolean, Array, AnyObject, NilClass, Object};
+use ruru::{RString, Boolean, Array, AnyObject, NilClass, Object, Class, VerifiedObject};
+use ruru::types::{Value, ValueType};
 use std::path::{MAIN_SEPARATOR,Path};
 use std::fs;
 
 type MaybeArray = Result<ruru::Array, ruru::result::Error>;
 type MaybeString = Result<ruru::RString, ruru::result::Error>;
 type MaybeBoolean = Result<ruru::Boolean, ruru::result::Error>;
 
+pub struct Pathname {
+  value: Value
+}
+
+impl Pathname {
+  pub fn new(path: &str) -> Pathname {
+    let mut instance = Class::from_existing("Pathname").allocate();
+    instance.instance_variable_set("@path", RString::new(path).to_any_object());
+    
+    Pathname { value: instance.value() }
+  }
+
+  pub fn to_any_object(&self) -> AnyObject {
+    AnyObject::from(self.value())
+  }
+}
+
+impl From<Value> for Pathname {
+  fn from(value: Value) -> Self {
+    Pathname { value: value }
+  }
+}
+
+impl Object for Pathname {
+  #[inline]
+  fn value(&self) -> Value {
+    self.value
+  }
+}
+
+impl VerifiedObject for Pathname {
+  fn is_correct_type<T: Object>(object: &T) -> bool {
+    object.value().ty() == ValueType::Class &&
+      Class::from_existing("Pathname").case_equals(object)
+  }
+
+  fn error_message() -> &'static str {
+    "Error converting to Pathname"
+  }
+}
+
 pub fn pn_add_trailing_separator(pth: MaybeString) -> RString {
   let p = pth.ok().unwrap();
   let x = format!("{}{}", p.to_str(), "a");
@@ -90,11 +132,11 @@ pub fn pn_children_compat(pth: MaybeString, with_dir: MaybeBoolean) -> AnyObject
     for entry in entries {
       if with_directory {
         if let Ok(v) = entry {
-          arr.push(new_pathname_instance(v.path().to_str().unwrap()));
+          arr.push(Pathname::new(v.path().to_str().unwrap()));
         };
       } else {
         if let Ok(v) = entry {
-          arr.push(new_pathname_instance(v.file_name().to_str().unwrap()));
+          arr.push(Pathname::new(v.file_name().to_str().unwrap()));
         };
       }
     }
@@ -138,7 +180,25 @@ pub fn pn_cleanpath_conservative(pth: MaybeString) -> RString {
   RString::new(&path)
 }
 
-// pub fn pn_del_trailing_separator(pth: MaybeString){}
+pub fn pn_del_trailing_separator(pth: MaybeString) -> RString {
+  if let &Ok(ref path) = &pth {
+    let path = path.to_str();
+
+    if !path.is_empty() {
+      let path = path.trim_right_matches('/');
+
+      if path.is_empty() {
+        return RString::new("/");
+      } else {
+        return RString::new(path);
+      }
+    }
+  } else {
+    return RString::new("");
+  }
+
+  pth.unwrap()
+}
 
 // pub fn pn_descend(){}
 
@@ -188,12 +248,12 @@ pub fn pn_entries_compat(pth: MaybeString) -> AnyObject {
   if let Ok(files) = fs::read_dir(pth.ok().unwrap_or(RString::new("")).to_str()) {
     let mut arr = Array::new();
 
-    arr.push(new_pathname_instance("."));
-    arr.push(new_pathname_instance(".."));
+    arr.push(Pathname::new("."));
+    arr.push(Pathname::new(".."));
 
     for file in files {
       let file_name_str = file.unwrap().file_name().into_string().unwrap();
-      arr.push(new_pathname_instance(&file_name_str));
+      arr.push(Pathname::new(&file_name_str));
     }
 
     arr.to_any_object()
@@ -227,7 +287,7 @@ pub fn pn_join(args: MaybeArray) -> AnyObject {
   let path_self = anyobject_to_string(args.shift()).unwrap();
   let mut qty = args.length();
   if qty <= 0 {
-    return new_pathname_instance(&path_self).to_any_object();
+    return Pathname::new(&path_self).to_any_object();
   }
 
   let mut result = String::new();
@@ -238,15 +298,15 @@ pub fn pn_join(args: MaybeArray) -> AnyObject {
     let item = args.pop();
     result = plus::plus_paths(&anyobject_to_string(item).unwrap(), &result);
     if result.chars().next() == Some(MAIN_SEPARATOR) {
-      return new_pathname_instance(&result).to_any_object()
+      return Pathname::new(&result).to_any_object()
     }
 
     qty -= 1;
   }
 
   let result = plus::plus_paths(&path_self, &result);
 
-  new_pathname_instance(&result).to_any_object()
+  Pathname::new(&result).to_any_object()
 }
 
 // pub fn pn_mkpath(pth: MaybeString) -> NilClass {

test/benches/del_trailing_separator_benchmark.rb

@@ -0,0 +1,27 @@
+require 'benchmark_helper'
+
+class DelTrailingSeparatorBenchmark < BenchmarkHelper
+  def setup
+    @file ||= __FILE__
+  end
+
+  def teardown
+    super
+    graph_benchmarks
+  end
+
+  def bench_rust_del_trailing_separator
+    benchmark :rust do
+      FasterPath.del_trailing_separator('/hello/world')
+      FasterPath.del_trailing_separator('/hello/world/')
+    end
+  end
+
+  def bench_ruby_del_trailing_separator
+    benchmark :ruby do
+      Pathname.allocate.send(:del_trailing_separator, '/hello/world')
+      Pathname.allocate.send(:del_trailing_separator, '/hello/world/')
+    end
+  end
+
+end

test/benches/join_benchmark.rb

@@ -3,6 +3,7 @@
 class JoinBenchmark < BenchmarkHelper
   def setup
     @file ||= __FILE__
+    @pathname = Pathname.new(".").freeze
   end
 
   def teardown
@@ -12,17 +13,15 @@ def teardown
 
   def bench_rust_join
     benchmark :rust do
-      FasterPath.join('a', 'b')
       FasterPath.join('.', 'b')
-      FasterPath.join('a//b/c', '../d//e')
+      FasterPath.join('.', '../d//e')
     end
   end
 
   def bench_ruby_join
     benchmark :ruby do
-      Pathname.allocate.send(:join, 'a', 'b')
-      Pathname.allocate.send(:join, '.', 'b')
-      Pathname.allocate.send(:join, 'a//b/c', '../d//e')
+      @pathname.send(:join, 'b')
+      @pathname.send(:join, '../d//e')
     end
   end
 end

test/del_trailing_separator_test.rb

@@ -0,0 +1,51 @@
+require 'test_helper'
+
+class DelTrailingSeparatorTest < Minitest::Test
+  def test_del_trailing_separator
+    assert_equal FasterPath.del_trailing_separator(""),      ""
+    assert_equal FasterPath.del_trailing_separator("/"),     "/"
+    assert_equal FasterPath.del_trailing_separator("/a"),    "/a"
+    assert_equal FasterPath.del_trailing_separator("/a/"),   "/a"
+    assert_equal FasterPath.del_trailing_separator("/a//"),  "/a"
+    assert_equal FasterPath.del_trailing_separator("."),     "."
+    assert_equal FasterPath.del_trailing_separator("./"),    "."
+    assert_equal FasterPath.del_trailing_separator(".//"),   "."
+  end
+
+  if File.dirname("A:") == "A:."
+    def test_del_trailing_separator_dos_drive_letter
+      assert_equal FasterPath.del_trailing_separator("A:"),    "A:"
+      assert_equal FasterPath.del_trailing_separator("A:/"),   "A:/"
+      assert_equal FasterPath.del_trailing_separator("A://"),  "A:/"
+      assert_equal FasterPath.del_trailing_separator("A:."),   "A:."
+      assert_equal FasterPath.del_trailing_separator("A:./"),  "A:."
+      assert_equal FasterPath.del_trailing_separator("A:.//"), "A:."
+    end
+  end
+
+  def test_del_trailing_separator_dos_unc
+    if File.dirname("//") == "//"
+      assert_equal FasterPath.del_trailing_separator("//"),        "//"
+      assert_equal FasterPath.del_trailing_separator("//a"),       "//a"
+      assert_equal FasterPath.del_trailing_separator("//a/"),      "//a"
+      assert_equal FasterPath.del_trailing_separator("//a//"),     "//a"
+      assert_equal FasterPath.del_trailing_separator("//a/b"),     "//a/b"
+      assert_equal FasterPath.del_trailing_separator("//a/b/"),    "//a/b"
+      assert_equal FasterPath.del_trailing_separator("//a/b//"),   "//a/b"
+      assert_equal FasterPath.del_trailing_separator("//a/b/c"),   "//a/b/c"
+      assert_equal FasterPath.del_trailing_separator("//a/b/c/"),  "//a/b/c"
+      assert_equal FasterPath.del_trailing_separator("//a/b/c//"), "//a/b/c"
+    else
+      assert_equal FasterPath.del_trailing_separator("///"),       "/"
+      assert_equal FasterPath.del_trailing_separator("///a/"),     "///a"
+    end
+  end
+
+  if !File::ALT_SEPARATOR.nil?
+    def test_del_trailing_separator_dos
+      assert_equal FasterPath.del_trailing_separator("a\\"), "a"
+      assert_equal FasterPath.del_trailing_separator("\225\\\\".dup.force_encoding("cp932")), "\225\\".dup.force_encoding("cp932")
+      assert_equal FasterPath.del_trailing_separator("\225\\\\".dup.force_encoding("cp437")), "\225".dup.force_encoding("cp437")
+    end
+  end
+end

test/pbench/pbench_suite.rb

@@ -150,6 +150,20 @@
     end
   end
 }
+PBENCHES[:del_trailing_separator] = {
+  new: lambda do |x|
+    x.times do
+      FasterPath.del_trailing_separator('/hello/world')
+      FasterPath.del_trailing_separator('/hello/world/')
+    end
+  end,
+  old: lambda do |x|
+    x.times do
+      PATHNAME_DOT.send(:del_trailing_separator, '/hello/world')
+      PATHNAME_DOT.send(:del_trailing_separator, '/hello/world/')
+    end
+  end
+}
 PBENCHES[:"directory?"] = {
   new: lambda do |x|
     x.times do