Skip to content
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

refactor: move @doc_status into compiler #1701

Open
wants to merge 4 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
26 changes: 12 additions & 14 deletions lib/review/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,12 @@ def post_paragraph
nil
end

attr_accessor :doc_status
attr_reader :location

def initialize(strict = false, *_args, img_math: nil)
@strict = strict
@output = nil
@logger = ReVIEW.logger
@doc_status = {}
@dictionary = {}
@img_math = img_math
end
Expand Down Expand Up @@ -88,7 +86,9 @@ def bind(compiler, chapter, location)

def builder_init_file
@sec_counter = SecCounter.new(5, @chapter)
@doc_status = {}
if @compiler.doc_status
Copy link
Owner

Choose a reason for hiding this comment

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

あ、もうしわけない。
builder側については私の手元のextをこのように直すほうで対処しますので、compilerのattrだけでOKです。ただし、(compilerのほうのコメントへ)

@doc_status = @compiler.doc_status ## for compatibility
end
end
private :builder_init_file

Expand Down Expand Up @@ -552,32 +552,30 @@ def captionblock(_type, _lines, _caption, _specialstyle = nil)
CAPTION_TITLES.each do |name|
class_eval %Q(
def #{name}(lines, caption = nil)
check_nested_minicolumn
captionblock("#{name}", lines, caption)
end

def #{name}_begin(caption = nil)
check_nested_minicolumn
@doc_status[:minicolumn] = '#{name}'
if caption
puts compile_inline(caption)
end
end

def #{name}_end
@doc_status[:minicolumn] = nil
end
), __FILE__, __LINE__ - 17
), __FILE__, __LINE__ - 13
end

def check_nested_minicolumn
if @doc_status[:minicolumn]
app_error "#{@location}: nested mini-column is not allowed"
end
def in_minicolumn?
@compiler.in_minicolumn?
end

def in_minicolumn?
@doc_status[:minicolumn]
def in_column?
@compiler.in_minicolumn?
end

def in_dt?
@compiler.in_dt?
end

def minicolumn_block_name?(name)
Expand Down
39 changes: 30 additions & 9 deletions lib/review/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,16 @@ def initialize(builder)
## to decide escaping/non-escaping for text
@command_name_stack = []

@doc_status = nil
Copy link
Owner

@kmuto kmuto May 30, 2021

Choose a reason for hiding this comment

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

@compiler に移動するのはOKですが、過去のreview-ext.rbの修正が少なく済むよう、attr_accessor :doc_status で公開してほしいです(既存のextは先頭で以下を対処するだけで済むので)

def builder_init_file
  super
  if @compiler.doc_status
     @doc_status = @compiler.doc_status
   end
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

なるほどです、とりあえず上記の参照は追加しておきます。
とはいえ将来的な互換性を考えるのであれば、下のような in_caption?等のメソッドに置き換えていただきたいところです… 🙏


@logger = ReVIEW.logger

@ignore_errors = builder.is_a?(ReVIEW::IndexBuilder)

@compile_errors = nil
end

attr_reader :builder, :previous_list_type
attr_reader :builder, :previous_list_type, :doc_status
Copy link
Owner

Choose a reason for hiding this comment

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

書きたいこともありそうなので、attr_accessorにしていただけると…

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ここで書きたいのはおそらく compiler.doc_status[:foo] = bar みたいなやつで、compiler.doc_status = {foo: bar}みたいなのではないですよね。
前者であれば@doc_statusに対しては参照しかしてない(参照した結果のオブジェクトに対して変更を加える)ので現状で可能です。後者だと気をつけないといろいろ壊れるので、許さない方が安全かな…と。なお何かの理由でリセットしたいときはcompiler.doc_status.clearで可能です。

Copy link
Owner

Choose a reason for hiding this comment

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

そうですね。なるほど、ではこれはこのままでOKです


def strategy
error 'Compiler#strategy is obsoleted. Use Compiler#builder.'
Expand All @@ -52,6 +54,7 @@ def non_escaped_commands

def compile(chap)
@chapter = chap
@doc_status = {}
do_compile
if @compile_errors
raise ApplicationError, "#{location.filename} cannot be compiled."
Expand Down Expand Up @@ -269,6 +272,18 @@ def inline_defined?(name)
definline :w
definline :wb

def in_minicolumn?
@doc_status[:minicolumn]
end

def in_column?
@doc_status[:column]
end

def in_dt?
@doc_status[:dt]
end

private

def do_compile
Expand Down Expand Up @@ -362,6 +377,7 @@ def compile_minicolumn_begin(name, caption = nil)
return
end
@minicolumn_name = name
@doc_status[:minicolumn] = name

@builder.__send__(mid, caption)
end
Expand All @@ -375,6 +391,8 @@ def compile_minicolumn_end

mid = "#{name}_end"
@builder.__send__(mid)

@doc_status[:minicolumn] = nil
@minicolumn_name = nil
end

Expand Down Expand Up @@ -443,12 +461,19 @@ def open_tagged_section(tag, level, label, caption)
return
end
@tagged_section.push([tag, level])
if tag == 'column'
@doc_status[:column] = true
end
@builder.__send__(mid, level, label, caption)
end

def close_tagged_section(tag, level)
mid = "#{tag}_end"
if @builder.respond_to?(mid)
if tag == 'column'
@doc_status[:column] = nil
end

@builder.__send__(mid, level)
else
error "builder does not support block op: #{mid}", location: location
Expand Down Expand Up @@ -523,9 +548,9 @@ def compile_dlist(f)
@builder.dl_begin
while /\A\s*:/ =~ f.peek
# defer compile_inline to handle footnotes
@builder.doc_status[:dt] = true
@doc_status[:dt] = true
@builder.dt(text(f.gets.sub(/\A\s*:/, '').strip))
@builder.doc_status[:dt] = nil
@doc_status[:dt] = nil
desc = []
f.until_match(/\A(\S|\s*:|\s+\d+\.\s|\s+\*\s)/) do |line|
desc << text(line.strip)
Expand Down Expand Up @@ -553,9 +578,9 @@ def read_command(f)
ignore_inline = @non_parsed_commands.include?(name)
@command_name_stack.push(name)
args = parse_args(line.sub(%r{\A//[a-z]+}, '').rstrip.chomp('{'), name)
@builder.doc_status[name] = true
@doc_status[name] = true
lines = block_open?(line) ? read_block(f, ignore_inline) : nil
@builder.doc_status[name] = nil
@doc_status[name] = nil
[name, args, lines]
end

Expand Down Expand Up @@ -698,10 +723,6 @@ def compile_inline(str)
@builder.nofunc_text(str)
end

def in_minicolumn?
@builder.in_minicolumn?
end

def minicolumn_block_name?(name)
@builder.minicolumn_block_name?(name)
end
Expand Down
6 changes: 1 addition & 5 deletions lib/review/htmlbuilder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ def sup_end(_level)
end

def captionblock(type, lines, caption)
check_nested_minicolumn
puts %Q(<div class="#{type}">)
if caption.present?
puts %Q(<p class="caption">#{compile_inline(caption)}</p>)
Expand Down Expand Up @@ -334,8 +333,6 @@ def note(lines, caption = nil)
CAPTION_TITLES.each do |name|
class_eval %Q(
def #{name}_begin(caption = nil)
check_nested_minicolumn
@doc_status[:minicolumn] = '#{name}'
puts %Q(<div class="#{name}">)
if caption.present?
puts %Q(<p class="caption">\#{compile_inline(caption)}</p>)
Expand All @@ -344,9 +341,8 @@ def #{name}_begin(caption = nil)

def #{name}_end
puts '</div>'
@doc_status[:minicolumn] = nil
end
), __FILE__, __LINE__ - 14
), __FILE__, __LINE__ - 11
end

def ul_begin
Expand Down
19 changes: 5 additions & 14 deletions lib/review/idgxmlbuilder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def builder_init_file
print %Q(<?xml version="1.0" encoding="UTF-8"?>\n)
print %Q(<#{@rootelement} xmlns:aid="http://ns.adobe.com/AdobeInDesign/4.0/">)
@secttags = @book.config['structuredxml']
@minicolumn_with_caption = nil
end
private :builder_init_file

Expand Down Expand Up @@ -955,22 +956,18 @@ def captionblock(type, lines, caption, specialstyle = nil)
end

def note(lines, caption = nil)
check_nested_minicolumn
captionblock('note', lines, caption)
end

def memo(lines, caption = nil)
check_nested_minicolumn
captionblock('memo', lines, caption)
end

def tip(lines, caption = nil)
check_nested_minicolumn
captionblock('tip', lines, caption)
end

def info(lines, caption = nil)
check_nested_minicolumn
captionblock('info', lines, caption)
end

Expand All @@ -983,7 +980,6 @@ def best(lines, caption = nil)
end

def important(lines, caption = nil)
check_nested_minicolumn
captionblock('important', lines, caption)
end

Expand All @@ -992,12 +988,10 @@ def security(lines, caption = nil)
end

def caution(lines, caption = nil)
check_nested_minicolumn
captionblock('caution', lines, caption)
end

def warning(lines, caption = nil)
check_nested_minicolumn
captionblock('warning', lines, caption)
end

Expand All @@ -1010,7 +1004,6 @@ def link(lines, caption = nil)
end

def notice(lines, caption = nil)
check_nested_minicolumn
if caption
captionblock('notice-t', lines, caption, 'notice-title')
else
Expand Down Expand Up @@ -1049,12 +1042,10 @@ def expert(lines)
CAPTION_TITLES.each do |name|
class_eval %Q(
def #{name}_begin(caption = nil)
check_nested_minicolumn
if '#{name}' == 'notice' && caption.present?
@doc_status[:minicolumn] = '#{name}-t'
@minicolumn_with_caption = true
print "<#{name}-t>"
else
@doc_status[:minicolumn] = '#{name}'
print "<#{name}>"
end
if caption.present?
Expand All @@ -1063,14 +1054,14 @@ def #{name}_begin(caption = nil)
end

def #{name}_end
if '#{name}' == 'notice' && @doc_status[:minicolumn] == 'notice-t'
if '#{name}' == 'notice' && @minicolumn_with_caption
print "</#{name}-t>"
@minicolumn_with_caption = nil
else
print "</#{name}>"
end
@doc_status[:minicolumn] = nil
end
), __FILE__, __LINE__ - 23
), __FILE__, __LINE__ - 21
end

def syntaxblock(type, lines, caption)
Expand Down
Loading