Skip to content

Standardize table and heading markup elements #1389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions lib/rdoc/markup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ def convert(input, formatter)
autoload :BlankLine, "#{__dir__}/markup/blank_line"
autoload :BlockQuote, "#{__dir__}/markup/block_quote"
autoload :Document, "#{__dir__}/markup/document"
autoload :Element, "#{__dir__}/markup/element"
autoload :HardBreak, "#{__dir__}/markup/hard_break"
autoload :Heading, "#{__dir__}/markup/heading"
autoload :Include, "#{__dir__}/markup/include"
Expand Down
21 changes: 21 additions & 0 deletions lib/rdoc/markup/element.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# frozen_string_literal: true

module RDoc
class Markup
# Base class defining the interface for all markup elements found in documentation
# @abstract
class Element
# @abstract
#: (untyped) -> void
def accept(visitor)
raise NotImplementedError, "#{self.class} must implement the accept method"
end

# @abstract
#: (PP) -> void
def pretty_print(q)
raise NotImplementedError, "#{self.class} must implement the pretty_print method"
end
end
end
end
174 changes: 95 additions & 79 deletions lib/rdoc/markup/heading.rb
Original file line number Diff line number Diff line change
@@ -1,84 +1,100 @@
# frozen_string_literal: true
##
# A heading with a level (1-6) and text

RDoc::Markup::Heading =
Struct.new :level, :text do

@to_html = nil
@to_label = nil

##
# A singleton RDoc::Markup::ToLabel formatter for headings.

def self.to_label
@to_label ||= RDoc::Markup::ToLabel.new
end

##
# A singleton plain HTML formatter for headings. Used for creating labels
# for the Table of Contents

def self.to_html
return @to_html if @to_html

markup = RDoc::Markup.new
markup.add_regexp_handling RDoc::CrossReference::CROSSREF_REGEXP, :CROSSREF

@to_html = RDoc::Markup::ToHtml.new nil

def @to_html.handle_regexp_CROSSREF(target)
target.text.sub(/^\\/, '')
module RDoc
class Markup
# A heading with a level (1-6) and text
#
# RDoc syntax:
# = Heading 1
# == Heading 2
# === Heading 3
#
# Markdown syntax:
# # Heading 1
# ## Heading 2
# ### Heading 3
class Heading < Element
#: String
attr_reader :text

#: Integer
attr_accessor :level

# A singleton RDoc::Markup::ToLabel formatter for headings.
#: () -> RDoc::Markup::ToLabel
def self.to_label
@to_label ||= Markup::ToLabel.new
end

# A singleton plain HTML formatter for headings. Used for creating labels for the Table of Contents
#: () -> RDoc::Markup::ToHtml
def self.to_html
@to_html ||= begin
markup = Markup.new
markup.add_regexp_handling CrossReference::CROSSREF_REGEXP, :CROSSREF

@to_html = Markup::ToHtml.new nil
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be a local variable here @to_htmlto_html
It will be assigned to @to_html at @to_html ||= begin


def @to_html.handle_regexp_CROSSREF(target)
target.text.sub(/^\\/, '')
end

@to_html
end
end

#: (Integer, String) -> void
def initialize(level, text)
@level = level
@text = text
super()
Comment on lines +48 to +50
Copy link
Member

Choose a reason for hiding this comment

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

Initializing the base object (Markup::Element) before this object may be better:

Suggested change
@level = level
@text = text
super()
super()
@level = level
@text = text

Or we may want to remove super() here because the current Markup::Element#initialize is empty.

end

#: (Object) -> bool
def ==(other)
other.is_a?(Heading) && other.level == @level && other.text == @text
end

# @override
#: (untyped) -> void
def accept(visitor)
visitor.accept_heading(self)
end

# An HTML-safe anchor reference for this header.
#: () -> String
def aref
"label-#{self.class.to_label.convert text.dup}"
end

# Creates a fully-qualified label which will include the label from +context+. This helps keep ids unique in HTML.
#: (RDoc::Context?) -> String
def label(context = nil)
label = +""
label << "#{context.aref}-" if context&.respond_to?(:aref)
label << aref
label
end

# HTML markup of the text of this label without the surrounding header element.
#: () -> String
def plain_html
text = self.text.dup
Copy link
Member

Choose a reason for hiding this comment

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

It seems that we don't need dup here.


if matched = text.match(/rdoc-image:[^:]+:(.*)/)
text = matched[1]
end

self.class.to_html.to_html(text)
Comment on lines +82 to +88
Copy link
Member

Choose a reason for hiding this comment

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

How about using more descriptive local variable name and remove self. from self.text something like:

Suggested change
text = self.text.dup
if matched = text.match(/rdoc-image:[^:]+:(.*)/)
text = matched[1]
end
self.class.to_html.to_html(text)
no_image_text = text
if matched = no_image_text.match(/rdoc-image:[^:]+:(.*)/)
no_image_text = matched[1]
end
self.class.to_html.to_html(no_image_text)

end

# @override
#: (PP) -> void
def pretty_print(q)
q.group 2, "[head: #{level} ", ']' do
q.pp text
end
end
end

@to_html
end

##
# Calls #accept_heading on +visitor+

def accept(visitor)
visitor.accept_heading self
end

##
# An HTML-safe anchor reference for this header.

def aref
"label-#{self.class.to_label.convert text.dup}"
end

##
# Creates a fully-qualified label which will include the label from
# +context+. This helps keep ids unique in HTML.

def label(context = nil)
label = aref

label = [context.aref, label].compact.join '-' if
context and context.respond_to? :aref

label
end

##
# HTML markup of the text of this label without the surrounding header
# element.

def plain_html
text = self.text.dup

if matched = text.match(/rdoc-image:[^:]+:(.*)/)
text = matched[1]
end

self.class.to_html.to_html(text)
end

def pretty_print(q) # :nodoc:
q.group 2, "[head: #{level} ", ']' do
q.pp text
end
end

end
88 changes: 48 additions & 40 deletions lib/rdoc/markup/table.rb
Original file line number Diff line number Diff line change
@@ -1,52 +1,60 @@
# frozen_string_literal: true
##
# A section of table

class RDoc::Markup::Table
# headers of each column
attr_accessor :header
module RDoc
class Markup
# A section of table
class Table < Element
# Headers of each column
#: Array[String]
attr_accessor :header

# alignments of each column
attr_accessor :align
# Alignments of each column
#: Array[Symbol?]
attr_accessor :align

# body texts of each column
attr_accessor :body
# Body texts of each column
#: Array[String]
attr_accessor :body

# Creates new instance
def initialize(header, align, body)
@header, @align, @body = header, align, body
end

# :stopdoc:
def ==(other)
self.class == other.class and
@header == other.header and
@align == other.align and
@body == other.body
end
#: (Array[String], Array[Symbol?], Array[String]) -> void
def initialize(header, align, body)
@header, @align, @body = header, align, body
end

def accept(visitor)
visitor.accept_table @header, @body, @align
end
#: (Object) -> bool
def ==(other)
self.class == other.class && @header == other.header &&
@align == other.align && @body == other.body
end

def pretty_print(q)
q.group 2, '[Table: ', ']' do
q.group 2, '[Head: ', ']' do
q.seplist @header.zip(@align) do |text, align|
q.pp text
if align
q.text ":"
q.breakable
q.text align.to_s
end
end
# @override
#: (untyped) -> void
def accept(visitor)
visitor.accept_table(@header, @body, @align)
end
q.breakable
q.group 2, '[Body: ', ']' do
q.seplist @body do |body|
q.group 2, '[', ']' do
q.seplist body do |text|

# @override
#: (untyped) -> String
def pretty_print(q)
q.group 2, '[Table: ', ']' do
q.group 2, '[Head: ', ']' do
q.seplist @header.zip(@align) do |text, align|
q.pp text
if align
q.text ":"
q.breakable
q.text align.to_s
end
end
end
q.breakable
q.group 2, '[Body: ', ']' do
q.seplist @body do |body|
q.group 2, '[', ']' do
q.seplist body do |text|
q.pp text
end
end
end
end
end
Expand Down
Loading