From c9cdf32fd0f5e3f22b899ada425ac23073cc554c Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Thu, 23 Jun 2016 23:57:10 +0100 Subject: [PATCH] basename: Improve performance and style More idiomatic, faster, extracted from #66 --- lib/faster_path/optional/monkeypatches.rb | 4 +- lib/faster_path/optional/refinements.rb | 4 +- spec/ruby_spec | 1 + src/basename.rs | 93 +++++------------------ test/basename_test.rb | 13 ++-- 5 files changed, 31 insertions(+), 84 deletions(-) create mode 160000 spec/ruby_spec diff --git a/lib/faster_path/optional/monkeypatches.rb b/lib/faster_path/optional/monkeypatches.rb index be96bbe..3691402 100644 --- a/lib/faster_path/optional/monkeypatches.rb +++ b/lib/faster_path/optional/monkeypatches.rb @@ -1,8 +1,8 @@ module FasterPath def self.sledgehammer_everything! ::File.class_eval do - def self.basename(pth) - FasterPath.basename(pth) + def self.basename(pth, ext = '') + FasterPath.basename(pth, ext) end if ENV['WITH_REGRESSION'] def self.extname(pth) diff --git a/lib/faster_path/optional/refinements.rb b/lib/faster_path/optional/refinements.rb index d8878e9..5b4e541 100644 --- a/lib/faster_path/optional/refinements.rb +++ b/lib/faster_path/optional/refinements.rb @@ -1,8 +1,8 @@ module FasterPath module RefineFile refine File do - def self.basename(pth) - FasterPath.basename(pth) + def self.basename(pth, ext = '') + FasterPath.basename(pth, ext) end if ENV['WITH_REGRESSION'] def self.extname(pth) diff --git a/spec/ruby_spec b/spec/ruby_spec new file mode 160000 index 0000000..8190a7c --- /dev/null +++ b/spec/ruby_spec @@ -0,0 +1 @@ +Subproject commit 8190a7c915fbcf4776b99390418fe63fc6086c02 diff --git a/src/basename.rs b/src/basename.rs index 7d69f89..9d0117f 100644 --- a/src/basename.rs +++ b/src/basename.rs @@ -1,86 +1,31 @@ use std::path::MAIN_SEPARATOR; use libc::c_char; use std::ffi::{CStr,CString}; -use std::str; -#[allow(dead_code)] -fn rubyish_basename(string: &str, globish_string: &str) -> String { - let result = if globish_string == ".*" { - let base = string.rsplit_terminator(MAIN_SEPARATOR).nth(0).unwrap_or(""); - let index = base.rfind("."); - let (first, _) = base.split_at(index.unwrap()); - first - } else { - if &string[string.len()-globish_string.len()..] == globish_string { - &string[0..string.len()-globish_string.len()] - } else { - string - }.rsplit_terminator(MAIN_SEPARATOR).nth(0).unwrap_or("") - }; - result.to_string() -} +#[no_mangle] +pub extern "C" fn basename(c_pth: *const c_char, c_ext: *const c_char) -> *const c_char { + // TODO: rb_raise on type or encoding errors + // TODO: support objects that respond to `to_path` + if c_pth.is_null() || c_ext.is_null() { + return c_pth; + } + let pth = unsafe { CStr::from_ptr(c_pth) }.to_str().unwrap(); + let ext = unsafe { CStr::from_ptr(c_ext) }.to_str().unwrap(); -#[test] -fn it_chomps_strings_correctly(){ - assert_eq!(rubyish_basename("","") , ""); - assert_eq!(rubyish_basename("ruby","") , "ruby"); - assert_eq!(rubyish_basename("ruby.rb",".rb") , "ruby"); - assert_eq!(rubyish_basename("ruby.rb",".*") , "ruby"); - assert_eq!(rubyish_basename(".ruby/ruby.rb",".rb") , "ruby"); - assert_eq!(rubyish_basename(".ruby/ruby.rb.swp",".rb") , "ruby.rb.swp"); - assert_eq!(rubyish_basename(".ruby/ruby.rb.swp",".swp") , "ruby.rb"); - assert_eq!(rubyish_basename(".ruby/ruby.rb.swp",".*") , "ruby.rb"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp","") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".*") , "asdf.rb"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", "*") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".*") , "asdf.rb"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".rb") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".swp") , "asdf.rb"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".sw") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".sw*") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".rb.s*") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".s*") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".s**") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".**") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".*") , "asdf.rb"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".*.*") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".rb.swp") , "asdf"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".rb.s*p") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".r*.s*p") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".r*.sw*p") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", ".r*b.sw*p") , "asdf.rb.swp"); - assert_eq!(rubyish_basename("asdf/asdf.rb.swp", "rb.swp") , "asdf."); -} + let filename = pth.trim_right_matches(MAIN_SEPARATOR).rsplit(MAIN_SEPARATOR).next().unwrap_or(""); -#[no_mangle] -pub extern fn basename(str_pth: *const c_char, comp_ext: *const c_char) -> *const c_char { - let c_str1 = unsafe { - if str_pth.is_null(){ - return str_pth; - } - CStr::from_ptr(str_pth) - }; - let c_str2 = unsafe { - if comp_ext.is_null() { - return str_pth; + let result = if ext == ".*" { + match filename.rfind('.') { + Some(dot_i) => &filename[0..dot_i], + _ => filename, } - CStr::from_ptr(comp_ext) - }; - let string = str::from_utf8(c_str1.to_bytes()).unwrap(); - let globish_string = str::from_utf8(c_str2.to_bytes()).unwrap(); - - let result = if globish_string == ".*" { - let base = string.rsplit_terminator(MAIN_SEPARATOR).nth(0).unwrap_or(""); - let index = base.rfind("."); - let (first, _) = base.split_at(index.unwrap()); - first } else { - if &string[string.len()-globish_string.len()..] == globish_string { - &string[0..string.len()-globish_string.len()] + if filename.ends_with(ext) { + &filename[..filename.len() - ext.len()] } else { - string - }.rsplit_terminator(MAIN_SEPARATOR).nth(0).unwrap_or("") + filename + } }; + CString::new(result).unwrap().into_raw() } diff --git a/test/basename_test.rb b/test/basename_test.rb index 6d8eea5..8113dae 100644 --- a/test/basename_test.rb +++ b/test/basename_test.rb @@ -2,7 +2,7 @@ class BasenameTest < Minitest::Test def test_nil_inputs - assert_nil FasterPath.basename(nil,nil) + assert_nil FasterPath.basename(nil,nil) assert_equal FasterPath.basename('',nil), "" assert_nil FasterPath.basename(nil,'') assert_equal FasterPath.basename('asdf',nil), "asdf" @@ -21,18 +21,19 @@ def test_it_creates_basename_correctly assert_equal FasterPath.basename('ruby.rb', ''), 'ruby.rb' assert_equal FasterPath.basename('ruby.rbx', '.rb*'), 'ruby.rbx' assert_equal FasterPath.basename('ruby.rbx'), 'ruby.rbx' - + # Try some extensions w/o a '.' assert_equal FasterPath.basename('ruby.rbx', 'rbx'), 'ruby.' assert_equal FasterPath.basename('ruby.rbx', 'x'), 'ruby.rb' assert_equal FasterPath.basename('ruby.rbx', '*'), 'ruby.rbx' - + # A couple of regressions: assert_equal FasterPath.basename('', ''), '' assert_equal FasterPath.basename('/'), '/' assert_equal FasterPath.basename('//'), '/' assert_equal FasterPath.basename('//dir///base//'), 'base' - end + assert_equal FasterPath.basename('.x', '.x'), '.x' + end def test_it_does_the_same_as_file_basename assert_equal File.basename('/home/gumby/work/ruby.rb'), 'ruby.rb' @@ -45,12 +46,12 @@ def test_it_does_the_same_as_file_basename assert_equal File.basename('ruby.rb', ''), 'ruby.rb' assert_equal File.basename('ruby.rbx', '.rb*'), 'ruby.rbx' assert_equal File.basename('ruby.rbx'), 'ruby.rbx' - + # Try some extensions w/o a '.' assert_equal File.basename('ruby.rbx', 'rbx'), 'ruby.' assert_equal File.basename('ruby.rbx', 'x'), 'ruby.rb' assert_equal File.basename('ruby.rbx', '*'), 'ruby.rbx' - + # A couple of regressions: assert_equal File.basename('', ''), '' assert_equal File.basename('/'), '/'