Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions lib/rdoc/code_object/class_module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,22 @@ class RDoc::ClassModule < RDoc::Context
attr_accessor :constant_aliases

##
# Comment and the location it came from. Use #add_comment to add comments
# An array of `[comment, location]` pairs documenting this class/module.
# Use #add_comment to add comments.
#
# Before marshalling:
# - +comment+ is a String
# - +location+ is an RDoc::TopLevel
#
# After unmarshalling:
# - +comment+ is an RDoc::Markup::Document
# - +location+ is a filename String
#
# These type changes are acceptable (for now) because:
# - +comment+: Both String and Document respond to #empty?, and #parse
# returns Document as-is (see RDoc::Text#parse)
# - +location+: Only used by #parse to set Document#file, which accepts
# both TopLevel (extracts relative_name) and String

attr_accessor :comment_location

Expand Down Expand Up @@ -110,7 +125,7 @@ def initialize(name, superclass = nil)
@is_alias_for = nil
@name = name
@superclass = superclass
@comment_location = [] # [[comment, location]]
@comment_location = [] # Array of [comment, location] pairs

super()
end
Expand Down Expand Up @@ -379,10 +394,10 @@ def marshal_load(array) # :nodoc:

@comment = RDoc::Comment.from_document document

@comment_location = if RDoc::Markup::Document === document.parts.first then
document
@comment_location = if document.parts.first.is_a?(RDoc::Markup::Document)
document.parts.map { |doc| [doc, doc.file] }
else
RDoc::Markup::Document.new document
[[document, document.file]]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were creating a brand new document before. Was that intentional?

For example, should the updated code be this?

new_doc = RDoc::Markup::Document.new document
[[new_doc, new_doc.file]]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document is not a comment, so I think it should ideally be [[comment, document.file]] for both branch.

@comment_locaation = [[comment, document.file]]

parse([[@comment, document.file]]) will return a parsed document cached in @comment (as attr_reader :document)

Although not fully investigated the effect of doing this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since parse @comment_location seems working, I'm OK with the less-impact change: [[document, location]] for now 👍

end

array[5].each do |name, rw, visibility, singleton, file|
Expand Down
57 changes: 52 additions & 5 deletions test/rdoc/code_object/class_module_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,43 @@ def test_marshal_load_version_3
assert_equal tl, loaded.method_list.first.file
end

def test_search_snippet_after_marshal
@store.path = Dir.tmpdir
tl = @store.add_file 'file.rb'

ns = tl.add_module RDoc::NormalModule, 'Namespace'
cm = ns.add_class RDoc::NormalClass, 'Klass', 'Super'
cm.add_comment 'This is a class comment', tl

loaded = Marshal.load Marshal.dump cm
loaded.store = @store

# search_snippet should be able to handle post-marshalling comment_location
snippet = loaded.search_snippet
assert_kind_of String, snippet
assert_match(/class comment/, snippet)
end

def test_comment_location_is_array_after_marshal
@store.path = Dir.tmpdir
tl = @store.add_file 'file.rb'

ns = tl.add_module RDoc::NormalModule, 'Namespace'
cm = ns.add_class RDoc::NormalClass, 'Klass', 'Super'
cm.add_comment 'This is a class comment', tl

loaded = Marshal.load Marshal.dump cm
loaded.store = @store

assert_kind_of Array, loaded.comment_location
assert_equal 1, loaded.comment_location.length

comment, location = loaded.comment_location.first
assert_kind_of RDoc::Markup::Document, comment
# After marshal, location is the filename string (from doc.file)
assert_equal tl.relative_name, location
end

def test_merge
tl = @store.add_file 'one.rb'
p1 = tl.add_class RDoc::NormalClass, 'Parent'
Expand Down Expand Up @@ -1174,14 +1211,24 @@ def test_parse_comment_location
cm.add_comment 'comment 1', tl1
cm.add_comment 'comment 2', tl2

assert_kind_of Array, cm.comment_location
assert_equal 2, cm.comment_location.length
assert_equal 'comment 1', cm.comment_location[0][0]
assert_equal tl1, cm.comment_location[0][1]
assert_equal 'comment 2', cm.comment_location[1][0]
assert_equal tl2, cm.comment_location[1][1]

cm = Marshal.load Marshal.dump cm

doc1 = @RM::Document.new @RM::Paragraph.new 'comment 1'
doc1.file = tl1
doc2 = @RM::Document.new @RM::Paragraph.new 'comment 2'
doc2.file = tl2
# After marshal, comment_location should still be an array
assert_kind_of Array, cm.comment_location

assert_same cm.comment_location, cm.parse(cm.comment_location)
# parse() produces a Document with parts for each comment
parsed = cm.parse(cm.comment_location)
assert_kind_of RDoc::Markup::Document, parsed
assert_equal 2, parsed.parts.length
assert_equal 'comment 1', parsed.parts[0].parts[0].text
assert_equal 'comment 2', parsed.parts[1].parts[0].text
end

def test_remove_nodoc_children
Expand Down
13 changes: 9 additions & 4 deletions test/rdoc/rdoc_store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -922,12 +922,17 @@ def test_save_class_merge

s = RDoc::RI::Store.new(RDoc::Options.new, path: @tmpdir)

inner = @RM::Document.new @RM::Paragraph.new 'new comment'
inner.file = @top_level
loaded = s.load_class('Object')

document = @RM::Document.new inner
# After loading, comment_location is an array (not a Document)
assert_kind_of Array, loaded.comment_location
assert_equal 1, loaded.comment_location.length

assert_equal document, s.load_class('Object').comment_location
# Verify content is preserved
comment, location = loaded.comment_location.first
assert_kind_of @RM::Document, comment
assert_equal 'new comment', comment.parts[0].text
assert_equal @top_level.relative_name, location
end

# This is a functional test
Expand Down