Skip to content

Loading…

Add code statistics for Javascript and CoffeeScript files to `rake stats` task #2270

Closed
wants to merge 2 commits into from

9 participants

@nfm

First time contributor so I'm not sure if I'm 'doing it right'. Should I rebase on master and submit a second pull request?

@dmathieu

What about the stylesheets then ?

@dmathieu dmathieu commented on an outdated diff
railties/lib/rails/code_statistics.rb
@@ -39,11 +39,19 @@ class CodeStatistics #:nodoc:
f = File.open(directory + "/" + file_name)
+ if file_name =~ /.*\.rb$/
+ comment_pattern = /^\s*#/
+ elsif file_name =~ /.*\.js$/
+ comment_pattern = /^\s*\/\//
+ elsif file_name =~ /.*\.coffee$/
+ comment_pattern = /^\s*#/
+ end

I think a when/case would be more appropriate here (and it'd allow you to avoid repeting the same comment_pattern for rb and coffee.

@nfm
nfm added a note

Do you mean something like this?

    case file_name
    when /.*\.js$/
      comment_pattern = /^\s*\/\//
    else
      comment_pattern = /^\s*#/
    end 
@lwe
lwe added a note

why not just use /^\s*(#|\/\/)/ as comment pattern?

@nfm
nfm added a note

That'd probably suffice. You'd match // comments in rb files which isn't correct, but that's a pretty unlikely way to start a line of code!

I made it explicit so that you get an association between file extensions and comment syntaxes, and because it's more easily extended for other file types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nfm

I'm not sure about including stylesheets. In my mind they fall into a different category to application code and client-side code. I guess this wouldn't matter except that the aggregate stats (Code LOC, Test LOC, Code to Test Ratio) could be skewed significantly by them.

What do you think?

@nfm nfm referenced this pull request
Merged

show stats for app/mailers #4478

@nfm

@josevalim While you're tackling code stats (#4478) can you please take a look at this pull request too?

@steveklabnik
Ruby on Rails member

Should I rebase on master and submit a second pull request?

As a general note, you can rebase whatever in your branch, and then force push, and GitHub will update the pull request properly.

@josevalim
Ruby on Rails member

I think in this case, a new pull request needs to be sent because this one targets branch 3-1-stable and pull requests should be sent for master. Thanks!!

@arunagw arunagw added a commit to arunagw/rails that referenced this pull request
@arunagw arunagw Add code statistics for Javascript and
CoffeeScript files to `rake stats` task

Orignal PR was #2270

Thanks to @nfm
a48b3f1
@rafaelfranca
Ruby on Rails member

Closed by #6566

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 26, 2011
  1. @nfm
Commits on Jul 31, 2011
  1. @nfm
Showing with 10 additions and 2 deletions.
  1. +9 −2 railties/lib/rails/code_statistics.rb
  2. +1 −0 railties/lib/rails/tasks/statistics.rake
View
11 railties/lib/rails/code_statistics.rb
@@ -26,7 +26,7 @@ def calculate_statistics
Hash[@pairs.map{|pair| [pair.first, calculate_directory_statistics(pair.last)]}]
end
- def calculate_directory_statistics(directory, pattern = /.*\.rb$/)
+ def calculate_directory_statistics(directory, pattern = /.*\.(rb|js|coffee)$/)
stats = { "lines" => 0, "codelines" => 0, "classes" => 0, "methods" => 0 }
Dir.foreach(directory) do |file_name|
@@ -39,11 +39,18 @@ def calculate_directory_statistics(directory, pattern = /.*\.rb$/)
f = File.open(directory + "/" + file_name)
+ case file_name
+ when /.*\.js$/
+ comment_pattern = /^\s*\/\//
+ else
+ comment_pattern = /^\s*#/
+ end
+
while line = f.gets
stats["lines"] += 1
stats["classes"] += 1 if line =~ /class [A-Z]/
stats["methods"] += 1 if line =~ /def [a-z]/
- stats["codelines"] += 1 unless line =~ /^\s*$/ || line =~ /^\s*#/
+ stats["codelines"] += 1 unless line =~ /^\s*$/ || line =~ comment_pattern
end
end
View
1 railties/lib/rails/tasks/statistics.rake
@@ -2,6 +2,7 @@ STATS_DIRECTORIES = [
%w(Controllers app/controllers),
%w(Helpers app/helpers),
%w(Models app/models),
+ %w(Javascripts app/assets/javascripts),
%w(Libraries lib/),
%w(APIs app/apis),
%w(Integration\ tests test/integration),
Something went wrong with that request. Please try again.