Skip to content

Commit 9e9150b

Browse files
committed
Avoid using block binding to capture template variables
Under the current implementation, the template variables are captured through the `render_template` method's block. This makes it hard to understand if a variable is intended to be used in the template or not. This commit changes the `render_template` method to require passing a hash of local variables to the template. This makes the variable scope explicit and easier to reason about.
1 parent 9066c02 commit 9e9150b

File tree

1 file changed

+51
-46
lines changed

1 file changed

+51
-46
lines changed

lib/rdoc/generator/darkfish.rb

Lines changed: 51 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -296,12 +296,11 @@ def generate_index
296296
@title = @options.title
297297
@main_page = @files.find { |f| f.full_name == @options.main_page }
298298

299-
render_template template_file, out_file do |io|
300-
here = binding
301-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
302-
here.local_variable_set(:target, @main_page)
303-
here
304-
end
299+
render_template template_file, out_file: out_file, locals: {
300+
rel_prefix: rel_prefix,
301+
asset_rel_prefix: asset_rel_prefix,
302+
target: @main_page,
303+
}
305304
rescue => e
306305
error = RDoc::Error.new \
307306
"error generating index.html: #{e.message} (#{e.class})"
@@ -340,18 +339,19 @@ def generate_class klass, template_file = nil
340339
klass_sections = klass.sort_sections
341340

342341
debug_msg " rendering #{out_file}"
343-
render_template template_file, out_file do |io|
344-
here = binding
345-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
346-
here.local_variable_set(:svninfo, svninfo)
347-
here.local_variable_set(:breadcrumb, breadcrumb)
348-
here.local_variable_set(:klass_class_methods, klass_class_methods)
349-
here.local_variable_set(:klass_instance_methods, klass_instance_methods)
350-
here.local_variable_set(:klass_extends, klass_extends)
351-
here.local_variable_set(:klass_includes, klass_includes)
352-
here.local_variable_set(:klass_sections, klass_sections)
353-
here
354-
end
342+
render_template template_file, out_file: out_file, locals: {
343+
asset_rel_prefix: asset_rel_prefix,
344+
rel_prefix: rel_prefix,
345+
target: target,
346+
svninfo: svninfo,
347+
klass: klass,
348+
breadcrumb: breadcrumb,
349+
klass_class_methods: klass_class_methods,
350+
klass_instance_methods: klass_instance_methods,
351+
klass_extends: klass_extends,
352+
klass_includes: klass_includes,
353+
klass_sections: klass_sections,
354+
}
355355
end
356356

357357
##
@@ -434,12 +434,12 @@ def generate_file_files
434434
@title += " - #{@options.title}"
435435
template_file ||= filepage_file
436436

437-
render_template template_file, out_file do |io|
438-
here = binding
439-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
440-
here.local_variable_set(:target, target)
441-
here
442-
end
437+
render_template template_file, out_file: out_file, locals: {
438+
rel_prefix: rel_prefix,
439+
asset_rel_prefix: asset_rel_prefix,
440+
file: file,
441+
target: target,
442+
}
443443
end
444444
rescue => e
445445
error =
@@ -469,12 +469,12 @@ def generate_page file
469469
@title = "#{file.page_name} - #{@options.title}"
470470

471471
debug_msg " rendering #{out_file}"
472-
render_template template_file, out_file do |io|
473-
here = binding
474-
here.local_variable_set(:target, target)
475-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
476-
here
477-
end
472+
render_template template_file, out_file: out_file, locals: {
473+
file: file,
474+
target: target,
475+
asset_rel_prefix: asset_rel_prefix,
476+
rel_prefix: rel_prefix,
477+
}
478478
end
479479

480480
##
@@ -496,11 +496,11 @@ def generate_servlet_not_found message
496496

497497
@title = 'Not Found'
498498

499-
render_template template_file do |io|
500-
here = binding
501-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
502-
here
503-
end
499+
render_template template_file, locals: {
500+
asset_rel_prefix: asset_rel_prefix,
501+
rel_prefix: rel_prefix,
502+
message: message
503+
}
504504
rescue => e
505505
error = RDoc::Error.new \
506506
"error generating servlet_not_found: #{e.message} (#{e.class})"
@@ -527,7 +527,11 @@ def generate_servlet_root installed
527527

528528
@title = 'Local RDoc Documentation'
529529

530-
render_template template_file do |io| binding end
530+
render_template template_file, locals: {
531+
asset_rel_prefix: asset_rel_prefix,
532+
rel_prefix: rel_prefix,
533+
installed: installed
534+
}
531535
rescue => e
532536
error = RDoc::Error.new \
533537
"error generating servlet_root: #{e.message} (#{e.class})"
@@ -556,11 +560,10 @@ def generate_table_of_contents
556560

557561
@title = "Table of Contents - #{@options.title}"
558562

559-
render_template template_file, out_file do |io|
560-
here = binding
561-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
562-
here
563-
end
563+
render_template template_file, out_file: out_file, locals: {
564+
rel_prefix: rel_prefix,
565+
asset_rel_prefix: asset_rel_prefix,
566+
}
564567
rescue => e
565568
error = RDoc::Error.new \
566569
"error generating table_of_contents.html: #{e.message} (#{e.class})"
@@ -698,26 +701,28 @@ def render file_name
698701
#
699702
# An io will be yielded which must be captured by binding in the caller.
700703

701-
def render_template template_file, out_file = nil # :yield: io
704+
def render_template template_file, out_file: nil, locals: {}
702705
io_output = out_file && !@dry_run && @file_output
703706
erb_klass = io_output ? RDoc::ERBIO : ERB
704707

705708
template = template_for template_file, true, erb_klass
706709

710+
@context = binding
711+
locals.each do |key, value|
712+
@context.local_variable_set(key, value)
713+
end
714+
707715
if io_output then
708716
debug_msg "Outputting to %s" % [out_file.expand_path]
709717

710718
out_file.dirname.mkpath
711719
out_file.open 'w', 0644 do |io|
712720
io.set_encoding @options.encoding
713-
714-
@context = yield io
721+
@context.local_variable_set(:io, io)
715722

716723
template_result template, @context, template_file
717724
end
718725
else
719-
@context = yield nil
720-
721726
output = template_result template, @context, template_file
722727

723728
debug_msg " would have written %d characters to %s" % [

0 commit comments

Comments
 (0)