Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Align rake notes.

  • Loading branch information...
commit 9299bfdcd387253d83b645c205b8df477f2d0940 1 parent 9fffef5
@mptre mptre authored
View
3  railties/lib/rails/source_annotation_extractor.rb
@@ -22,7 +22,7 @@ class Annotation < Struct.new(:line, :tag, :text)
# If +options+ has a flag <tt>:tag</tt> the tag is shown as in the example above.
# Otherwise the string contains just line and text.
def to_s(options={})
- s = "[%3d] " % line
+ s = "[#{line.to_s.rjust(options[:indent])}]"

Is it intentional that this change introduces a subtle modification of the output format of rake notes as it leaves out the space between line number and tag.
I'm asking because this new format breaks the Ruby Metrics Plugin in Jenkins that would no longer parse the output correcty.

(Just in case, I have also proposed a change to the Metrics Plugin parser code: jenkinsci/rubymetrics-plugin#10)

@mptre
mptre added a note

Yikes! This was not my intention at all. I just submitted another pull request (mptre/rails@c5f4b20) with a fix and corresponding tests in order hopefully not introduce this bug again.

Sorry for the inconvenience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
s << "[#{tag}] " if options[:tag]
s << text
end
@@ -93,6 +93,7 @@ def extract_annotations_from(file, pattern)
# Prints the mapping from filenames to annotations in +results+ ordered by filename.
# The +options+ hash is passed to each annotation's +to_s+.
def display(results, options={})
+ options[:indent] = results.map { |f, a| a.map(&:line) }.flatten.max.to_s.size
results.keys.sort.each do |file|
puts "#{file}:"
results[file].each do |note|
View
9 railties/test/application/rake/notes_test.rb
@@ -17,6 +17,7 @@ def teardown
app_file "app/views/home/index.html.erb", "<% # TODO: note in erb %>"
app_file "app/views/home/index.html.haml", "-# TODO: note in haml"
app_file "app/views/home/index.html.slim", "/ TODO: note in slim"
+ app_file "app/controllers/application_controller.rb", 1000.times.map { "" }.join("\n") << "# TODO: note in ruby"
boot_rails
require 'rake'
@@ -27,10 +28,18 @@ def teardown
Dir.chdir(app_path) do
output = `bundle exec rake notes`
+ lines = output.scan(/\[([0-9\s]+)\]/).flatten
assert_match /note in erb/, output
assert_match /note in haml/, output
assert_match /note in slim/, output
+ assert_match /note in ruby/, output
+
+ assert_equal 4, lines.size
+ assert_equal 4, lines[0].size
+ assert_equal 4, lines[1].size
+ assert_equal 4, lines[2].size
+ assert_equal 4, lines[3].size
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.