refactor: move @doc_status into compiler
#1701
Conversation
builder should not have (global) status
| @@ -28,6 +28,8 @@ def initialize(builder) | |||
| ## to decide escaping/non-escaping for text | |||
| @command_name_stack = [] | |||
|
|
|||
| @doc_status = nil | |||
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
なるほどです、とりあえず上記の参照は追加しておきます。
とはいえ将来的な互換性を考えるのであれば、下のような in_caption?等のメソッドに置き換えていただきたいところです… 🙏
| @@ -1140,7 +1114,7 @@ def footnote(id, content) | |||
| def inline_fn(id) | |||
| if @book.config['footnotetext'] | |||
| macro("footnotemark[#{@chapter.footnote(id).number}]", '') | |||
| elsif @doc_status[:caption] || @doc_status[:table] || @doc_status[:column] || @doc_status[:dt] | |||
| elsif in_caption? || in_column? || in_dt? | |||
There was a problem hiding this comment.
:table はキャプションというよりth, tdが主眼なので(これもfootnotemarkにする必要がある)、in_table? を追加する必要がありそうです。
There was a problem hiding this comment.
ここの@doc_status[:table]は誰も代入してない（？）ので消したのですが、むしろth,tdで使うように修正すべきですか？
There was a problem hiding this comment.
Compiler#read_command で任意nameで入っているはずなのですが、なんかおかしいですね。
こちらのテストではmasterとこのブランチとでth,td内のfootnoteの扱いが変わっています。←は今このブランチではtableが消えてるから当然でした。
tcolorboxを使ったなどの理由でfootnotemarkにしないといけないケースは多いです(手元だと:listなど)。in_だとちょっと煩雑かもしれない気がしています。
There was a problem hiding this comment.
under?([:caption, :column, :dt]) みたいなのは気持ち悪いですかね。メソッド名もなんかもっとマシなのはないか…
There was a problem hiding this comment.
あー、そっちの方で代入していたんですね…じゃあどの値を使っているのか不明なのか。
| @@ -86,6 +86,9 @@ def bind(compiler, chapter, location) | |||
|
|
|||
| def builder_init_file | |||
| @sec_counter = SecCounter.new(5, @chapter) | |||
| if @compiler.doc_status | |||
There was a problem hiding this comment.
あ、もうしわけない。
builder側については私の手元のextをこのように直すほうで対処しますので、compilerのattrだけでOKです。ただし、(compilerのほうのコメントへ)
| @@ -37,7 +37,7 @@ def initialize(builder) | |||
| @compile_errors = nil | |||
| end | |||
|
|
|||
| attr_reader :builder, :previous_list_type | |||
| attr_reader :builder, :previous_list_type, :doc_status | |||
There was a problem hiding this comment.
書きたいこともありそうなので、attr_accessorにしていただけると…
There was a problem hiding this comment.
ここで書きたいのはおそらく compiler.doc_status[:foo] = bar みたいなやつで、compiler.doc_status = {foo: bar}みたいなのではないですよね。
前者であれば@doc_statusに対しては参照しかしてない（参照した結果のオブジェクトに対して変更を加える）ので現状で可能です。後者だと気をつけないといろいろ壊れるので、許さない方が安全かな…と。なお何かの理由でリセットしたいときはcompiler.doc_status.clearで可能です。
|
Sorry for bothering, I've already rewritten raw atmark Compiler to quoted string |
#1698 と同じような課題ですが、
@doc_statusやcheck_nested_minicolumnなどがbuilderのあちこちに分散しているのを避けるべく、compiler側に移動してみました。