Permalink
Browse files

simplifies a regexp

The new regexp has less work to do, we anchor a fixed string at the end
and need no group.
  • Loading branch information...
1 parent 2abe950 commit 6a70f2dd6b791c3f4888122d5b7dd9c8f5cac871 @fxn fxn committed Aug 23, 2012
Showing with 1 addition and 1 deletion.
  1. +1 −1 activesupport/lib/active_support/dependencies.rb
@@ -331,7 +331,7 @@ def clear
def require_or_load(file_name, const_path = nil)
log_call file_name, const_path
- file_name = $1 if file_name =~ /^(.*)\.rb$/
+ file_name = $` if file_name =~ /\.rb\z/
expanded = File.expand_path(file_name)
return if loaded.include?(expanded)

6 comments on commit 6a70f2d

@tenderlove
Member

I think you could just do File.basename(file_name, '.rb') and remove the conditional.

@skojin
Contributor
skojin commented on 6a70f2d Aug 24, 2012

file_name in real sometimes is path, so basename not work here

@homakov
Contributor

@fxn
if file_name[-3,3] == '.rb' and no regexp complication/execution is needed? or am i wrong?

@pixeltrix
Member

You could omit the regexp and rewrite the expanded line as:

expanded = File.expand_path("#{File.dirname(file_name)}/#{File.basename(file_name, '.rb')}")

The File.dirname returns '.' where there is no path component of file_name so gives the same result once expanded.

@fxn
Member
fxn commented on 6a70f2d Aug 24, 2012

Yeah, in general the file name is a path.

@homakov you could test with that, and you could also use end_with?, but you then need another slice or something to extract the path without the extension for the assigment.

For my taste the code is fine as is.

@tenderlove
Member

Oops. I didn't realize you could have the full file name (as @skojin says). So ya, this seems good. :-)

Please sign in to comment.