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.

Sign up for GitHub

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

refactor: move @doc_status into compiler #1701

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d6eb070
move @doc_status into compiler
takahashim May 29, 2021
59b8a51
fix caption
takahashim May 29, 2021
378be47
left @doc_status for compatibility
takahashim May 30, 2021
50c9282
fix tests
takahashim May 30, 2021
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
3 changes: 3 additions & 0 deletions lib/review/builder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ def bind(compiler, chapter, location)

def builder_init_file
@sec_counter = SecCounter.new(5, @chapter)
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
2 changes: 1 addition & 1 deletion lib/review/compiler.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 Down