Skip to content

Commit a8dee57

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 1d8fff1 commit a8dee57

File tree

1 file changed

+50
-45
lines changed

1 file changed

+50
-45
lines changed

lib/rdoc/generator/darkfish.rb

Lines changed: 50 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -294,12 +294,11 @@ def generate_index
294294
@title = @options.title
295295
@main_page = @files.find { |f| f.full_name == @options.main_page }
296296

297-
render_template template_file, out_file do |io|
298-
here = binding
299-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
300-
here.local_variable_set(:target, @main_page)
301-
here
302-
end
297+
render_template template_file, out_file: out_file, locals: {
298+
rel_prefix: rel_prefix,
299+
asset_rel_prefix: asset_rel_prefix,
300+
target: @main_page,
301+
}
303302
rescue => e
304303
error = RDoc::Error.new \
305304
"error generating index.html: #{e.message} (#{e.class})"
@@ -335,17 +334,18 @@ def generate_class klass, template_file = nil
335334
klass_sections = klass.sort_sections
336335

337336
debug_msg " rendering #{out_file}"
338-
render_template template_file, out_file do |io|
339-
here = binding
340-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
341-
here.local_variable_set(:breadcrumb, breadcrumb)
342-
here.local_variable_set(:klass_class_methods, klass_class_methods)
343-
here.local_variable_set(:klass_instance_methods, klass_instance_methods)
344-
here.local_variable_set(:klass_extends, klass_extends)
345-
here.local_variable_set(:klass_includes, klass_includes)
346-
here.local_variable_set(:klass_sections, klass_sections)
347-
here
348-
end
337+
render_template template_file, out_file: out_file, locals: {
338+
asset_rel_prefix: asset_rel_prefix,
339+
rel_prefix: rel_prefix,
340+
target: target,
341+
klass: klass,
342+
breadcrumb: breadcrumb,
343+
klass_class_methods: klass_class_methods,
344+
klass_instance_methods: klass_instance_methods,
345+
klass_extends: klass_extends,
346+
klass_includes: klass_includes,
347+
klass_sections: klass_sections,
348+
}
349349
end
350350

351351
##
@@ -424,12 +424,12 @@ def generate_file_files
424424
@title += " - #{@options.title}"
425425
template_file ||= filepage_file
426426

427-
render_template template_file, out_file do |io|
428-
here = binding
429-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
430-
here.local_variable_set(:target, target)
431-
here
432-
end
427+
render_template template_file, out_file: out_file, locals: {
428+
rel_prefix: rel_prefix,
429+
asset_rel_prefix: asset_rel_prefix,
430+
file: file,
431+
target: target,
432+
}
433433
end
434434
rescue => e
435435
error =
@@ -457,12 +457,12 @@ def generate_page file
457457
@title = "#{file.page_name} - #{@options.title}"
458458

459459
debug_msg " rendering #{out_file}"
460-
render_template template_file, out_file do |io|
461-
here = binding
462-
here.local_variable_set(:target, target)
463-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
464-
here
465-
end
460+
render_template template_file, out_file: out_file, locals: {
461+
file: file,
462+
target: target,
463+
asset_rel_prefix: asset_rel_prefix,
464+
rel_prefix: rel_prefix,
465+
}
466466
end
467467

468468
##
@@ -482,11 +482,11 @@ def generate_servlet_not_found message
482482

483483
@title = 'Not Found'
484484

485-
render_template template_file do |io|
486-
here = binding
487-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
488-
here
489-
end
485+
render_template template_file, locals: {
486+
asset_rel_prefix: asset_rel_prefix,
487+
rel_prefix: rel_prefix,
488+
message: message
489+
}
490490
rescue => e
491491
error = RDoc::Error.new \
492492
"error generating servlet_not_found: #{e.message} (#{e.class})"
@@ -511,7 +511,11 @@ def generate_servlet_root installed
511511

512512
@title = 'Local RDoc Documentation'
513513

514-
render_template template_file do |io| binding end
514+
render_template template_file, locals: {
515+
asset_rel_prefix: asset_rel_prefix,
516+
rel_prefix: rel_prefix,
517+
installed: installed
518+
}
515519
rescue => e
516520
error = RDoc::Error.new \
517521
"error generating servlet_root: #{e.message} (#{e.class})"
@@ -538,11 +542,10 @@ def generate_table_of_contents
538542

539543
@title = "Table of Contents - #{@options.title}"
540544

541-
render_template template_file, out_file do |io|
542-
here = binding
543-
here.local_variable_set(:asset_rel_prefix, asset_rel_prefix)
544-
here
545-
end
545+
render_template template_file, out_file: out_file, locals: {
546+
rel_prefix: rel_prefix,
547+
asset_rel_prefix: asset_rel_prefix,
548+
}
546549
rescue => e
547550
error = RDoc::Error.new \
548551
"error generating table_of_contents.html: #{e.message} (#{e.class})"
@@ -628,26 +631,28 @@ def render file_name
628631
#
629632
# An io will be yielded which must be captured by binding in the caller.
630633

631-
def render_template template_file, out_file = nil # :yield: io
634+
def render_template template_file, out_file: nil, locals: {}
632635
io_output = out_file && !@dry_run && @file_output
633636
erb_klass = io_output ? RDoc::ERBIO : ERB
634637

635638
template = template_for template_file, true, erb_klass
636639

640+
@context = binding
641+
locals.each do |key, value|
642+
@context.local_variable_set(key, value)
643+
end
644+
637645
if io_output then
638646
debug_msg "Outputting to %s" % [out_file.expand_path]
639647

640648
out_file.dirname.mkpath
641649
out_file.open 'w', 0644 do |io|
642650
io.set_encoding @options.encoding
643-
644-
@context = yield io
651+
@context.local_variable_set(:io, io)
645652

646653
template_result template, @context, template_file
647654
end
648655
else
649-
@context = yield nil
650-
651656
output = template_result template, @context, template_file
652657

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

0 commit comments

Comments
 (0)