From e710fc8de87baf41e1a9712035d3b0dc04722497 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 4 Oct 2026 20:51:50 +0200 Subject: [PATCH 1/4] Use compiled templates --- lib/rdoc/generator/compiled_template.rb | 29 +++ lib/rdoc/generator/darkfish.rb | 30 +++ .../generator/template/aliki/_footer.rhtml | 2 +- lib/rdoc/generator/template/aliki/_head.rhtml | 22 +- .../generator/template/darkfish/_footer.rhtml | 2 +- test/rdoc/generator/template_test.rb | 218 ++++++++++++++++++ test/rdoc/rdoc_server_test.rb | 38 +++ 7 files changed, 328 insertions(+), 13 deletions(-) create mode 100644 lib/rdoc/generator/compiled_template.rb create mode 100644 test/rdoc/generator/template_test.rb diff --git a/lib/rdoc/generator/compiled_template.rb b/lib/rdoc/generator/compiled_template.rb new file mode 100644 index 0000000000..2d23667ea7 --- /dev/null +++ b/lib/rdoc/generator/compiled_template.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module RDoc + module Generator + # Caches executable renderers for the bundled HTML templates. Extended onto + # ERB objects so the renderer cache has the same lifetime as the template. + module CompiledTemplate # :nodoc: + + attr_writer :generator + + #: (?Binding?) -> untyped + def result(context = nil) + return super unless context && context.receiver.equal?(@generator) && @generator.compiled_template_context?(context) + + variables = context.local_variables.sort + @compiled_renderers ||= {} + renderer = @compiled_renderers[variables] ||= compile_renderer(variables) + renderer.call(*variables.map { |name| context.local_variable_get(name) }) + end + + private + + #: (Array[Symbol]) -> Method + def compile_renderer(variables) + def_module("render(#{variables.join(', ')})").instance_method(:render).bind(@generator) + end + end + end +end diff --git a/lib/rdoc/generator/darkfish.rb b/lib/rdoc/generator/darkfish.rb index c097bab70c..3243676de7 100644 --- a/lib/rdoc/generator/darkfish.rb +++ b/lib/rdoc/generator/darkfish.rb @@ -5,6 +5,7 @@ require 'fileutils' require 'pathname' require_relative 'markup' +require_relative 'compiled_template' module RDoc module Generator @@ -160,6 +161,7 @@ def initialize(store, options) @classes = nil @context = nil + @compiled_template_context = nil @files = nil @methods = nil @modsort = nil @@ -642,6 +644,8 @@ def render(file_name) # An io will be yielded which must be captured by binding in the caller. def render_template(template_file, out_file = nil) # :yield: io + previous_compiled_context = @compiled_template_context + @compiled_template_context = nil io_output = out_file && !@dry_run && @file_output erb_klass = io_output ? ERBIO : ERB @@ -655,11 +659,13 @@ def render_template(template_file, out_file = nil) # :yield: io io.set_encoding @options.encoding @context = yield io + @compiled_template_context = @context if template.is_a?(CompiledTemplate) template_result template, @context, template_file end else @context = yield nil + @compiled_template_context = @context if template.is_a?(CompiledTemplate) output = template_result template, @context, template_file @@ -669,6 +675,15 @@ def render_template(template_file, out_file = nil) # :yield: io output end + ensure + @compiled_template_context = previous_compiled_context + end + + # Only bundled pages may use compiled partials. Custom pages can depend + # on the original shared-binding behavior even when using bundled partials. + #: (Binding) -> bool + def compiled_template_context?(context) # :nodoc: + @compiled_template_context.equal?(context) end ## @@ -706,6 +721,10 @@ def template_for(file, page = true, klass = ERB) end template = klass.new template, trim_mode: '-', eoutvar: erbout + if [ERB, ERBIO, ERBPartial].include?(klass) && builtin_template?(file) + template.extend CompiledTemplate + template.generator = self + end @template_cache[file] = template template end @@ -809,6 +828,17 @@ def group_classes_by_namespace_for_sidebar(classes) private + #: (Pathname) -> bool + def builtin_template?(file) + # Custom generators and templates may depend on shared-binding mutation + # or a different lexical scope, so keep their original ERB evaluation. + return false unless self.class == Darkfish || self.class == Aliki + + directory = @template_dir.expand_path + file.dirname.expand_path == directory && + %w[aliki darkfish].any? { |name| directory == Pathname.new(__dir__) + 'template' + name } + end + def template_encoding nil end diff --git a/lib/rdoc/generator/template/aliki/_footer.rhtml b/lib/rdoc/generator/template/aliki/_footer.rhtml index ae2f831443..d64c3797d3 100644 --- a/lib/rdoc/generator/template/aliki/_footer.rhtml +++ b/lib/rdoc/generator/template/aliki/_footer.rhtml @@ -16,7 +16,7 @@

- Generated by RDoc <%= VERSION %> + Generated by RDoc <%= ::RDoc::VERSION %> using the Aliki theme by Stan Lo

diff --git a/lib/rdoc/generator/template/aliki/_head.rhtml b/lib/rdoc/generator/template/aliki/_head.rhtml index d1ca4b7ad0..b64f5a1436 100644 --- a/lib/rdoc/generator/template/aliki/_head.rhtml +++ b/lib/rdoc/generator/template/aliki/_head.rhtml @@ -112,57 +112,57 @@ <%- @options.template_stylesheets.each do |stylesheet| %> <%- end %> diff --git a/lib/rdoc/generator/template/darkfish/_footer.rhtml b/lib/rdoc/generator/template/darkfish/_footer.rhtml index 3ccaa2d836..db483866ba 100644 --- a/lib/rdoc/generator/template/darkfish/_footer.rhtml +++ b/lib/rdoc/generator/template/darkfish/_footer.rhtml @@ -1,5 +1,5 @@ diff --git a/test/rdoc/generator/template_test.rb b/test/rdoc/generator/template_test.rb new file mode 100644 index 0000000000..495b82cfab --- /dev/null +++ b/test/rdoc/generator/template_test.rb @@ -0,0 +1,218 @@ +# frozen_string_literal: true +require_relative '../helper' + +class RDocGeneratorTemplateTest < RDoc::TestCase + + def setup + super + + @options.op_dir = @test_home + @top_level = @store.add_file 'example.rb' + @top_level.parser = RDoc::Parser::Ruby + @alpha = @top_level.add_class RDoc::NormalClass, 'Alpha' + @beta = @top_level.add_class RDoc::NormalClass, 'Beta' + @alpha.add_comment 'First page documentation: café.', @top_level + @beta.add_comment 'Second page documentation.', @top_level + @store.complete :private + end + + def test_builtin_pages_and_partials_are_evaluated_only_once + %w[aliki darkfish].each do |theme| + generator = generator_for theme + + first_evaluations = count_evaluations { generator.generate_class @alpha } + second_evaluations = count_evaluations { generator.generate_class @beta } + + assert_operator first_evaluations, :>, 0, theme + assert_equal 0, second_evaluations, theme + end + end + + def test_cached_renderers_are_methods_bound_to_the_generator + generator = generator_for 'aliki' + generator.generate_class @alpha + + templates = generator.instance_variable_get(:@template_cache).values + renderers = templates.flat_map do |template| + (template.instance_variable_get(:@compiled_renderers) || {}).values + end + + assert_not_empty renderers + renderers.each do |renderer| + assert_kind_of Method, renderer + assert_same generator, renderer.receiver + end + end + + def test_clearing_the_template_cache_discards_its_compiled_renderers + generator = generator_for 'aliki' + generator.generate_class @alpha + file = generator.template_dir + 'class.rhtml' + first_template = generator.template_for file + + generator.instance_variable_get(:@template_cache).clear + evaluations = count_evaluations { generator.generate_class @beta } + + assert_operator evaluations, :>, 0 + assert_not_same first_template, generator.template_for(file) + assert_equal 0, count_evaluations { generator.generate_class @alpha } + end + + def test_template_result_still_accepts_an_implicit_binding + generator = generator_for 'aliki' + template = generator.template_for generator.template_dir + '_sidebar_toggle.rhtml', false, RDoc::ERBPartial + + assert_include template.result, 'sidebar-navigation-toggle' + end + + def test_template_result_preserves_a_foreign_binding + generator = generator_for 'aliki' + template = generator.template_for generator.template_dir + '_sidebar_toggle.rhtml', false, RDoc::ERBPartial + + assert_include template.result(binding), 'sidebar-navigation-toggle' + assert_nil template.instance_variable_get(:@compiled_renderers) + end + + def test_cached_templates_use_current_class_and_path + %w[aliki darkfish].each do |theme| + generator = generator_for theme + alpha_html = generator.generate_class @alpha + beta_html = generator.generate_class @beta + + assert_include alpha_html, 'class Alpha -' + assert_include alpha_html, 'First page documentation: café.' + assert_include beta_html, '<title>class Beta -' + assert_include beta_html, 'Second page documentation.' + assert_not_include beta_html, 'First page documentation: café.' + assert_include beta_html, RDoc::VERSION + end + end + + def test_cached_templates_write_to_current_output_stream + generator = generator_for 'aliki' + generator.file_output = true + generator.generate_class @alpha + generator.generate_class @beta + + alpha_html = File.read File.join(@test_home, 'Alpha.html') + beta_html = File.read File.join(@test_home, 'Beta.html') + assert_include alpha_html, 'First page documentation: café.' + assert_include beta_html, 'Second page documentation.' + assert_not_include beta_html, 'First page documentation: café.' + end + + def test_index_handles_a_new_local_variable_without_reusing_the_old_renderer + generator = generator_for 'darkfish' + generator.generate_index + + readme = @store.add_file 'README.rdoc', parser: RDoc::Parser::Simple + readme.comment = '= Main page heading' + @options.main_page = 'README.rdoc' + generator.refresh_store_data + + html = generator.generate_index + assert_include html, 'Main page heading' + template = generator.template_for generator.template_dir + 'index.rhtml' + assert_equal 2, template.instance_variable_get(:@compiled_renderers).size + end + + def test_cached_templates_work_in_dry_run_mode + generator = generator_for 'aliki' + generator.file_output = true + generator.dry_run = true + generator.generate_class @alpha + + evaluations = count_evaluations { generator.generate_class @beta } + + assert_equal 0, evaluations + refute_file File.join(@test_home, 'Alpha.html') + refute_file File.join(@test_home, 'Beta.html') + end + + def test_custom_templates_preserve_shared_binding_mutations + @options.template_dir = @test_home + generator = RDoc::Generator::Darkfish.new @store, @options + generator.file_output = false + + page = Pathname.new(@test_home) + 'custom.rhtml' + File.write page, '<html><% page_value = "changed" %><%= render "_custom.rhtml" %></html>' + File.write File.join(@test_home, '_custom.rhtml'), '<%= page_value %>' + context = generator.instance_eval { binding } + context.local_variable_set :page_value, 'original' + + 2.times do |index| + html = generator.render_template(page) { context } + assert_equal "<html>#{'changed' * (index + 1)}</html>", html + assert_equal 'changed', context.local_variable_get(:page_value) + end + end + + def test_custom_page_using_builtin_partials_keeps_shared_binding_behavior + generator = generator_for 'darkfish' + generator.generate_class @beta + page = Pathname.new(@test_home) + 'custom.rhtml' + File.write page, '<html><% render "_footer.rhtml" %><%= @context.local_variable_get(:_erbout__footer) %></html>' + + html = generator.generate_class @alpha, page + + assert_include html, 'Generated by' + assert_include html, RDoc::VERSION + partial = generator.template_for generator.template_dir + '_footer.rhtml', false, RDoc::ERBPartial + assert_equal 1, partial.instance_variable_get(:@compiled_renderers).size + assert_nil generator.instance_variable_get(:@compiled_template_context) + end + + def test_custom_erb_classes_keep_their_result_implementation + generator = generator_for 'aliki' + erb_class = Class.new(ERB) do + def result(context = nil) + 'custom result' + end + end + template = generator.template_for generator.template_dir + '_sidebar_toggle.rhtml', false, erb_class + + assert_equal 'custom result', template.result(generator.instance_eval { binding }) + end + + def test_generator_subclasses_keep_binding_based_rendering + generator = generator_for 'darkfish', Class.new(RDoc::Generator::Darkfish) + generator.generate_class @alpha + + evaluations = count_evaluations { generator.generate_class @beta } + + assert_operator evaluations, :>, 0 + end + + def test_template_errors_keep_the_template_filename + generator = generator_for 'aliki' + @alpha.define_singleton_method(:description) { self.no_such_method } + + error = assert_raise(RDoc::Error) { generator.generate_class @alpha } + + assert_include error.message, 'class.rhtml' + file = generator.template_dir + 'class.rhtml' + line = generator.assemble_template(file).lines.index { |text| text.include?('klass.description') } + 1 + assert error.backtrace.any? { |entry| entry.include?("#{file}:#{line}:") } + end + + private + + def generator_for(theme, generator_class = nil) + generator_class ||= theme == 'aliki' ? RDoc::Generator::Aliki : RDoc::Generator::Darkfish + @options.template_dir = File.join @pwd, 'lib/rdoc/generator/template', theme + generator = generator_class.new @store, @options + @rdoc.generator = generator + generator.file_output = false + generator.setup + generator + end + + def count_evaluations + count = 0 + trace = TracePoint.new(:c_call) do |event| + count += 1 if event.method_id == :eval + end + trace.enable { yield } + count + end +end diff --git a/test/rdoc/rdoc_server_test.rb b/test/rdoc/rdoc_server_test.rb index 9f9a715713..d8a9d12441 100644 --- a/test/rdoc/rdoc_server_test.rb +++ b/test/rdoc/rdoc_server_test.rb @@ -79,6 +79,44 @@ def test_route_returns_404_for_missing_page assert_equal 'text/html', content_type end + def test_cached_templates_render_updated_and_new_classes_after_reparse + with_running_server do |port| + original = get port, '/Example.html' + assert_equal '200', original.code + assert_include original.body, 'method-i-greet' + + generator = @server.instance_variable_get :@generator + template = generator.template_for generator.template_dir + 'class.rhtml' + renderer = template.instance_variable_get(:@compiled_renderers).values.first + + File.write File.join(@dir, 'example.rb'), <<~RUBY + # Updated documentation from reparsed source. + class Example + def changed + end + end + + # Documentation for a newly added class. + class Another + end + RUBY + + wait_for('cached class page to reflect reparsed source') do + get(port, '/Example.html').body.include?('Updated documentation from reparsed source.') + end + + updated = get port, '/Example.html' + assert_include updated.body, 'method-i-changed' + assert_not_include updated.body, 'method-i-greet' + + added = get port, '/Another.html' + assert_equal '200', added.code + assert_include added.body, 'Documentation for a newly added class.' + assert_not_include added.body, 'Updated documentation from reparsed source.' + assert_same renderer, template.instance_variable_get(:@compiled_renderers).values.first + end + end + def test_check_for_changes_parses_and_reloads_rbs_signatures with_running_server do |port| sig_dir = File.join @dir, 'sig' From 7278762cdd401cf91510f916a054fba2fc32273a Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" <git@skatkov.com> Date: Sun, 4 Oct 2026 21:05:09 +0200 Subject: [PATCH 2/4] fixing TruffleRuby failure --- test/rdoc/generator/template_test.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/rdoc/generator/template_test.rb b/test/rdoc/generator/template_test.rb index 495b82cfab..112d52b2d8 100644 --- a/test/rdoc/generator/template_test.rb +++ b/test/rdoc/generator/template_test.rb @@ -209,8 +209,9 @@ def generator_for(theme, generator_class = nil) def count_evaluations count = 0 - trace = TracePoint.new(:c_call) do |event| - count += 1 if event.method_id == :eval + erb_file = ERB.instance_method(:result).source_location.first + trace = TracePoint.new(:line) do |event| + count += 1 if event.path == erb_file && %i[def_method result].include?(event.method_id) end trace.enable { yield } count From 1fbca18ef94b32334a75572bc77c3eac60716e80 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" <git@skatkov.com> Date: Sun, 4 Oct 2026 21:21:48 +0200 Subject: [PATCH 3/4] each template cache one renderer now --- lib/rdoc/generator/aliki.rb | 8 + lib/rdoc/generator/compiled_template.rb | 25 +--- lib/rdoc/generator/darkfish.rb | 103 ++++++++----- lib/rdoc/generator/template/aliki/_head.rhtml | 16 +- .../generator/template/darkfish/_head.rhtml | 6 +- .../template/darkfish/_sidebar_pages.rhtml | 2 +- .../generator/template/darkfish/index.rhtml | 2 +- test/rdoc/generator/template_test.rb | 139 +++++++++++++++--- test/rdoc/rdoc_server_test.rb | 5 +- 9 files changed, 222 insertions(+), 84 deletions(-) diff --git a/lib/rdoc/generator/aliki.rb b/lib/rdoc/generator/aliki.rb index 5157283b3a..410753b38f 100644 --- a/lib/rdoc/generator/aliki.rb +++ b/lib/rdoc/generator/aliki.rb @@ -13,6 +13,14 @@ module Generator class Aliki < Generator::Darkfish DESCRIPTION = 'HTML generator, written by Stan Lo' + TEMPLATE_INPUTS = Darkfish::TEMPLATE_INPUTS.merge( # :nodoc: + '_aside_toc.rhtml' => [], + '_footer.rhtml' => [:rel_prefix], + '_header.rhtml' => [:rel_prefix], + '_icons.rhtml' => [], + '_sidebar_ancestors.rhtml' => [:klass], + ).transform_values(&:freeze).freeze + RDoc.add_generator self def initialize(store, options) diff --git a/lib/rdoc/generator/compiled_template.rb b/lib/rdoc/generator/compiled_template.rb index 2d23667ea7..63b193e7ed 100644 --- a/lib/rdoc/generator/compiled_template.rb +++ b/lib/rdoc/generator/compiled_template.rb @@ -2,27 +2,16 @@ module RDoc module Generator - # Caches executable renderers for the bundled HTML templates. Extended onto - # ERB objects so the renderer cache has the same lifetime as the template. + # Caches an executable renderer with declared inputs for a bundled HTML + # template. ERB#result remains available for binding-based evaluation. module CompiledTemplate # :nodoc: - attr_writer :generator + attr_writer :generator, :render_inputs - #: (?Binding?) -> untyped - def result(context = nil) - return super unless context && context.receiver.equal?(@generator) && @generator.compiled_template_context?(context) - - variables = context.local_variables.sort - @compiled_renderers ||= {} - renderer = @compiled_renderers[variables] ||= compile_renderer(variables) - renderer.call(*variables.map { |name| context.local_variable_get(name) }) - end - - private - - #: (Array[Symbol]) -> Method - def compile_renderer(variables) - def_module("render(#{variables.join(', ')})").instance_method(:render).bind(@generator) + #: (Hash[Symbol, untyped]) -> untyped + def render(inputs) + @compiled_renderer ||= def_module("render(#{@render_inputs.join(', ')})").instance_method(:render).bind(@generator) + @compiled_renderer.call(*@render_inputs.map { |name| inputs.fetch(name) }) end end end diff --git a/lib/rdoc/generator/darkfish.rb b/lib/rdoc/generator/darkfish.rb index 3243676de7..d7b321f81c 100644 --- a/lib/rdoc/generator/darkfish.rb +++ b/lib/rdoc/generator/darkfish.rb @@ -81,6 +81,32 @@ class Darkfish DESCRIPTION = 'HTML generator, written by Michael Granger' + # Page inputs include the assembled head's contract. Absent page objects + # are nil, not missing locals. Partials only receive their own inputs. + PAGE_INPUTS = %i[io rel_prefix asset_rel_prefix current klass file].freeze # :nodoc: + TEMPLATE_INPUTS = { # :nodoc: + 'class.rhtml' => PAGE_INPUTS + [:breadcrumb], + 'index.rhtml' => PAGE_INPUTS, + 'page.rhtml' => PAGE_INPUTS, + 'servlet_not_found.rhtml' => PAGE_INPUTS + [:message], + 'servlet_root.rhtml' => PAGE_INPUTS + [:installed], + 'table_of_contents.rhtml' => PAGE_INPUTS, + '_head.rhtml' => %i[rel_prefix asset_rel_prefix current klass file], + '_footer.rhtml' => [], + '_sidebar_classes.rhtml' => [:rel_prefix], + '_sidebar_extends.rhtml' => [:klass], + '_sidebar_includes.rhtml' => [:klass], + '_sidebar_installed.rhtml' => [:installed], + '_sidebar_methods.rhtml' => [:klass], + '_sidebar_navigation.rhtml' => [:rel_prefix], + '_sidebar_pages.rhtml' => %i[rel_prefix current], + '_sidebar_parent.rhtml' => [:klass], + '_sidebar_search.rhtml' => [], + '_sidebar_sections.rhtml' => [:klass], + '_sidebar_table_of_contents.rhtml' => [:current], + '_sidebar_toggle.rhtml' => [], + }.transform_values(&:freeze).freeze + ## # The relative path to style sheets and javascript. By default this is set # the same as the rel_prefix. @@ -161,7 +187,6 @@ def initialize(store, options) @classes = nil @context = nil - @compiled_template_context = nil @files = nil @methods = nil @modsort = nil @@ -295,7 +320,8 @@ def generate_index @title = @options.title @main_page = @files.find { |f| f.full_name == @options.main_page } - render_template template_file, out_file do |io| + inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, current: @main_page } + render_template template_file, out_file, inputs: inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -335,7 +361,11 @@ def generate_class(klass, template_file = nil) @title = "#{klass.type} #{klass.full_name} - #{@options.title}" debug_msg " rendering #{out_file}" - render_template template_file, out_file do |io| + inputs = { + rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, + current: current, klass: klass, breadcrumb: breadcrumb, + } + render_template template_file, out_file, inputs: inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -459,7 +489,11 @@ def generate_page(file) @title = "#{file.page_name} - #{@options.title}" debug_msg " rendering #{out_file}" - render_template template_file, out_file do |io| + inputs = { + rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, + current: current, file: file, + } + render_template template_file, out_file, inputs: inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:current, current) @@ -485,7 +519,8 @@ def generate_servlet_not_found(message) @title = 'Not Found' - render_template template_file do |io| + inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, message: message } + render_template template_file, inputs: inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -515,7 +550,8 @@ def generate_servlet_root(installed) @title = 'Local RDoc Documentation' - render_template template_file do |io| binding end + inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, installed: installed } + render_template template_file, inputs: inputs do |io| binding end rescue => e error = Error.new \ "error generating servlet_root: #{e.message} (#{e.class})" @@ -542,7 +578,8 @@ def generate_table_of_contents @title = "Table of Contents - #{@options.title}" - render_template template_file, out_file do |io| + inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix } + render_template template_file, out_file, inputs: inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -630,9 +667,7 @@ def render(file_name) template = template_for template_file, false, ERBPartial - template.filename = template_file.to_s - - template.result @context + template_result template, @context, template_file end ## @@ -641,15 +676,17 @@ def render(file_name) # # Both +template_file+ and +out_file+ should be Pathname-like objects. # - # An io will be yielded which must be captured by binding in the caller. + # Bundled templates render with explicit +inputs+. For legacy/custom + # templates an io is yielded and must be captured by binding in the caller. - def render_template(template_file, out_file = nil) # :yield: io - previous_compiled_context = @compiled_template_context - @compiled_template_context = nil + def render_template(template_file, out_file = nil, inputs: nil) # :yield: io io_output = out_file && !@dry_run && @file_output erb_klass = io_output ? ERBIO : ERB template = template_for template_file, true, erb_klass + inputs = if inputs && template.is_a?(CompiledTemplate) + { current: nil, klass: nil, file: nil }.merge(inputs) + end if io_output debug_msg "Outputting to %s" % [out_file.expand_path] @@ -658,14 +695,12 @@ def render_template(template_file, out_file = nil) # :yield: io out_file.open 'w', 0644 do |io| io.set_encoding @options.encoding - @context = yield io - @compiled_template_context = @context if template.is_a?(CompiledTemplate) + @context = inputs ? inputs.merge(io: io) : yield(io) template_result template, @context, template_file end else - @context = yield nil - @compiled_template_context = @context if template.is_a?(CompiledTemplate) + @context = inputs ? inputs.merge(io: nil) : yield(nil) output = template_result template, @context, template_file @@ -675,15 +710,6 @@ def render_template(template_file, out_file = nil) # :yield: io output end - ensure - @compiled_template_context = previous_compiled_context - end - - # Only bundled pages may use compiled partials. Custom pages can depend - # on the original shared-binding behavior even when using bundled partials. - #: (Binding) -> bool - def compiled_template_context?(context) # :nodoc: - @compiled_template_context.equal?(context) end ## @@ -692,7 +718,11 @@ def compiled_template_context?(context) # :nodoc: def template_result(template, context, template_file) template.filename = template_file.to_s - template.result context + if template.is_a?(CompiledTemplate) && context.is_a?(Hash) + template.render context + else + template.result context + end rescue NoMethodError => e raise Error, "Error while evaluating %s: %s" % [ template_file.expand_path, @@ -721,9 +751,10 @@ def template_for(file, page = true, klass = ERB) end template = klass.new template, trim_mode: '-', eoutvar: erbout - if [ERB, ERBIO, ERBPartial].include?(klass) && builtin_template?(file) + if [ERB, ERBIO, ERBPartial].include?(klass) && (inputs = builtin_template_inputs(file)) template.extend CompiledTemplate template.generator = self + template.render_inputs = inputs end @template_cache[file] = template template @@ -828,15 +859,19 @@ def group_classes_by_namespace_for_sidebar(classes) private - #: (Pathname) -> bool - def builtin_template?(file) + #: (Pathname) -> Array[Symbol]? + def builtin_template_inputs(file) # Custom generators and templates may depend on shared-binding mutation # or a different lexical scope, so keep their original ERB evaluation. - return false unless self.class == Darkfish || self.class == Aliki + return unless self.class == Darkfish || self.class == Aliki directory = @template_dir.expand_path - file.dirname.expand_path == directory && - %w[aliki darkfish].any? { |name| directory == Pathname.new(__dir__) + 'template' + name } + return unless file.dirname.expand_path == directory + theme = directory.basename.to_s + return unless %w[aliki darkfish].include?(theme) && directory == Pathname.new(__dir__) + 'template' + theme + + contracts = theme == 'aliki' ? Aliki::TEMPLATE_INPUTS : TEMPLATE_INPUTS + contracts[file.basename.to_s] end def template_encoding diff --git a/lib/rdoc/generator/template/aliki/_head.rhtml b/lib/rdoc/generator/template/aliki/_head.rhtml index b64f5a1436..6957bf749c 100644 --- a/lib/rdoc/generator/template/aliki/_head.rhtml +++ b/lib/rdoc/generator/template/aliki/_head.rhtml @@ -3,7 +3,7 @@ <title><%= h @title %> -<%- if defined?(klass) %> +<%- if defined?(klass) && klass %> " @@ -19,7 +19,7 @@ content="<%= h "#{klass.type} #{klass.full_name}: #{excerpt(klass.comment)}" %>" > <%- end %> -<%- elsif defined?(file) %> +<%- elsif defined?(file) && file %> <%- if canonical_url = @options.canonical_root %> - <% canonical_url = current.canonical_url if defined?(current) %> + <% canonical_url = current.canonical_url if defined?(current) && current %> <%- end %> @@ -47,7 +47,7 @@ -<%- if defined?(klass) %> +<%- if defined?(klass) && klass %> <%- if klass.comment.empty? %> <%- end %> -<%- elsif defined?(file) %> +<%- elsif defined?(file) && file %> <%- if file.comment.empty? %> <%- if canonical_url = @options.canonical_root %> - <% canonical_url = current.canonical_url if defined?(current) %> + <% canonical_url = current.canonical_url if defined?(current) && current %> <%- end %> @@ -81,7 +81,7 @@ -<%- if defined?(klass) %> +<%- if defined?(klass) && klass %> <%- if klass.comment.empty? %> <%- end %> -<%- elsif defined?(file) %> +<%- elsif defined?(file) && file %> <%- if file.comment.empty? %> <%= h @title %> -<%- if defined?(klass) %> +<%- if defined?(klass) && klass %> "> <%- if klass.comment.empty? %> @@ -11,7 +11,7 @@ <%- else %> "> <%- end %> -<%- elsif defined?(file) %> +<%- elsif defined?(file) && file %> "> <%- elsif @title %> @@ -21,7 +21,7 @@ <%- end %> <%- if canonical_url = @options.canonical_root %> -<% canonical_url = current.canonical_url if defined?(current) %> +<% canonical_url = current.canonical_url if defined?(current) && current %> <%- end %> diff --git a/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml b/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml index 7de407e999..9dd1f6812a 100644 --- a/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml +++ b/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml @@ -1,5 +1,5 @@ <%- simple_files = @files.select { |f| f.text? } %> -<%- if defined?(current) %> +<%- if defined?(current) && current %> <%- dir = current.full_name[%r{\A[^/]+(?=/)}] || current.page_name %> <%- end %> <%- unless simple_files.empty? %> diff --git a/lib/rdoc/generator/template/darkfish/index.rhtml b/lib/rdoc/generator/template/darkfish/index.rhtml index d2ebcf69f7..6a75bebd35 100644 --- a/lib/rdoc/generator/template/darkfish/index.rhtml +++ b/lib/rdoc/generator/template/darkfish/index.rhtml @@ -7,7 +7,7 @@ <%= render '_sidebar_search.rhtml' %> - <%= render '_sidebar_table_of_contents.rhtml' if defined?(current) %> + <%= render '_sidebar_table_of_contents.rhtml' if defined?(current) && current %> <%= render '_sidebar_pages.rhtml' %> <%= render '_sidebar_classes.rhtml' %> diff --git a/test/rdoc/generator/template_test.rb b/test/rdoc/generator/template_test.rb index 112d52b2d8..da85a44afa 100644 --- a/test/rdoc/generator/template_test.rb +++ b/test/rdoc/generator/template_test.rb @@ -33,9 +33,7 @@ def test_cached_renderers_are_methods_bound_to_the_generator generator.generate_class @alpha templates = generator.instance_variable_get(:@template_cache).values - renderers = templates.flat_map do |template| - (template.instance_variable_get(:@compiled_renderers) || {}).values - end + renderers = templates.filter_map { |template| template.instance_variable_get(:@compiled_renderer) } assert_not_empty renderers renderers.each do |renderer| @@ -44,6 +42,43 @@ def test_cached_renderers_are_methods_bound_to_the_generator end end + def test_compiled_renderers_only_accept_declared_inputs + %w[aliki darkfish].each do |theme| + generator = generator_for theme + generator.generate_class @alpha + + page = generator.template_for generator.template_dir + 'class.rhtml' + renderer = page.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer, theme + assert_equal %i[io rel_prefix asset_rel_prefix current klass file breadcrumb], + renderer.parameters.map(&:last), theme + + partial = generator.template_for generator.template_dir + '_sidebar_sections.rhtml', false, RDoc::ERBPartial + renderer = partial.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer, theme + assert_equal [:klass], renderer.parameters.map(&:last), theme + end + end + + def test_builtin_rendering_does_not_inspect_bindings + %w[aliki darkfish].each do |theme| + generator = generator_for theme + binding_reads = [] + trace = TracePoint.new(:call, :c_call) do |event| + if event.self.is_a?(Binding) && %i[local_variables local_variable_get].include?(event.method_id) + binding_reads << event.method_id + end + end + + trace.enable do + generator.generate_class @alpha + generator.generate_class @beta + end + + assert_empty binding_reads, theme + end + end + def test_clearing_the_template_cache_discards_its_compiled_renderers generator = generator_for 'aliki' generator.generate_class @alpha @@ -70,7 +105,7 @@ def test_template_result_preserves_a_foreign_binding template = generator.template_for generator.template_dir + '_sidebar_toggle.rhtml', false, RDoc::ERBPartial assert_include template.result(binding), 'sidebar-navigation-toggle' - assert_nil template.instance_variable_get(:@compiled_renderers) + assert_nil template.instance_variable_get(:@compiled_renderer) end def test_cached_templates_use_current_class_and_path @@ -101,19 +136,59 @@ def test_cached_templates_write_to_current_output_stream assert_not_include beta_html, 'First page documentation: café.' end - def test_index_handles_a_new_local_variable_without_reusing_the_old_renderer - generator = generator_for 'darkfish' - generator.generate_index + def test_index_reuses_its_renderer_when_the_main_page_changes + %w[aliki darkfish].each do |theme| + @options.main_page = nil + generator = generator_for theme + html = generator.generate_index + assert_include html, 'This is the API documentation' + assert_not_include html, '

Table of Contents

' + + template = generator.template_for generator.template_dir + 'index.rhtml' + renderer = template.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer, theme + + readme = @store.add_file "README_#{theme}.rdoc", parser: RDoc::Parser::Simple + readme.comment = "= Main page heading\n\n== Second heading" + @options.main_page = readme.full_name + generator.refresh_store_data + html = generator.generate_index + assert_include html, 'Main page heading' + assert_include html, '

Table of Contents

' if theme == 'darkfish' + assert_same renderer, template.instance_variable_get(:@compiled_renderer), theme + + @options.main_page = nil + html = generator.generate_index + assert_include html, 'This is the API documentation' + assert_not_include html, '

Table of Contents

' + assert_same renderer, template.instance_variable_get(:@compiled_renderer), theme + end + end - readme = @store.add_file 'README.rdoc', parser: RDoc::Parser::Simple - readme.comment = '= Main page heading' - @options.main_page = 'README.rdoc' - generator.refresh_store_data + def test_partials_reuse_their_renderer_across_page_types + page = @store.add_file 'guides/USAGE.rdoc', parser: RDoc::Parser::Simple + page.comment = '= Nested page documentation' - html = generator.generate_index - assert_include html, 'Main page heading' - template = generator.template_for generator.template_dir + 'index.rhtml' - assert_equal 2, template.instance_variable_get(:@compiled_renderers).size + %w[aliki darkfish].each do |theme| + generator = generator_for theme + generator.generate_index + partial = generator.template_for generator.template_dir + '_sidebar_pages.rhtml', false, RDoc::ERBPartial + renderer = partial.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer, theme + + generator.generate_class @alpha + assert_same renderer, partial.instance_variable_get(:@compiled_renderer), theme + + html = generator.generate_page page + assert_include html, 'Nested page documentation' + assert_include html, '../css/rdoc.css' + assert_same renderer, partial.instance_variable_get(:@compiled_renderer), theme + + html = generator.generate_servlet_not_found 'Missing page' + assert_include html, 'Missing page' + assert_same renderer, partial.instance_variable_get(:@compiled_renderer), theme + assert_equal 0, count_evaluations { generator.generate_class @beta }, theme + end end def test_cached_templates_work_in_dry_run_mode @@ -147,6 +222,36 @@ def test_custom_templates_preserve_shared_binding_mutations end end + def test_custom_class_templates_can_access_incidental_caller_locals + generator = generator_for 'darkfish' + page = Pathname.new(@test_home) + 'custom.rhtml' + File.write page, <<~ERB + <%= [klass.equal?(current), out_file.basename.to_s, + template_file.basename.to_s, here.is_a?(Binding), + search_index_rel_prefix == rel_prefix].join('|') %> + ERB + + assert_equal "true|Alpha.html|custom.rhtml|true|true\n", + generator.generate_class(@alpha, page) + end + + def test_builtin_templates_preserve_explicit_binding_evaluation_without_inputs + generator = generator_for 'darkfish' + generator.generate_index + template_file = generator.template_dir + 'index.rhtml' + template = generator.template_for template_file + renderer = template.instance_variable_get :@compiled_renderer + context = generator.instance_eval { binding } + context.local_variable_set :rel_prefix, '.' + context.local_variable_set :asset_rel_prefix, '.' + + html = generator.render_template(template_file) { context } + + assert_include html, 'This is the API documentation' + assert_equal html, context.local_variable_get(:io) + assert_same renderer, template.instance_variable_get(:@compiled_renderer) + end + def test_custom_page_using_builtin_partials_keeps_shared_binding_behavior generator = generator_for 'darkfish' generator.generate_class @beta @@ -158,8 +263,8 @@ def test_custom_page_using_builtin_partials_keeps_shared_binding_behavior assert_include html, 'Generated by' assert_include html, RDoc::VERSION partial = generator.template_for generator.template_dir + '_footer.rhtml', false, RDoc::ERBPartial - assert_equal 1, partial.instance_variable_get(:@compiled_renderers).size - assert_nil generator.instance_variable_get(:@compiled_template_context) + assert_kind_of Method, partial.instance_variable_get(:@compiled_renderer) + assert_kind_of Binding, generator.instance_variable_get(:@context) end def test_custom_erb_classes_keep_their_result_implementation diff --git a/test/rdoc/rdoc_server_test.rb b/test/rdoc/rdoc_server_test.rb index d8a9d12441..283abc23ac 100644 --- a/test/rdoc/rdoc_server_test.rb +++ b/test/rdoc/rdoc_server_test.rb @@ -87,7 +87,8 @@ def test_cached_templates_render_updated_and_new_classes_after_reparse generator = @server.instance_variable_get :@generator template = generator.template_for generator.template_dir + 'class.rhtml' - renderer = template.instance_variable_get(:@compiled_renderers).values.first + renderer = template.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer File.write File.join(@dir, 'example.rb'), <<~RUBY # Updated documentation from reparsed source. @@ -113,7 +114,7 @@ class Another assert_equal '200', added.code assert_include added.body, 'Documentation for a newly added class.' assert_not_include added.body, 'Updated documentation from reparsed source.' - assert_same renderer, template.instance_variable_get(:@compiled_renderers).values.first + assert_same renderer, template.instance_variable_get(:@compiled_renderer) end end From bbd81c2c1ccbc1004d9c35d670f1b1883b844785 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 4 Oct 2026 21:41:10 +0200 Subject: [PATCH 4/4] Only use compiled templates with Aliki --- lib/rdoc/generator/aliki.rb | 69 +++++- lib/rdoc/generator/compiled_template.rb | 17 +- lib/rdoc/generator/darkfish.rb | 93 ++------ .../generator/template/darkfish/_head.rhtml | 6 +- .../template/darkfish/_sidebar_pages.rhtml | 2 +- .../generator/template/darkfish/index.rhtml | 2 +- test/rdoc/generator/template_test.rb | 220 +++++++++++------- 7 files changed, 248 insertions(+), 161 deletions(-) diff --git a/lib/rdoc/generator/aliki.rb b/lib/rdoc/generator/aliki.rb index 410753b38f..ae040f90ad 100644 --- a/lib/rdoc/generator/aliki.rb +++ b/lib/rdoc/generator/aliki.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'uri' +require_relative 'compiled_template' module RDoc module Generator @@ -13,20 +14,37 @@ module Generator class Aliki < Generator::Darkfish DESCRIPTION = 'HTML generator, written by Stan Lo' - TEMPLATE_INPUTS = Darkfish::TEMPLATE_INPUTS.merge( # :nodoc: + TEMPLATE_DIR = (Pathname.new(__dir__) + 'template' + 'aliki').freeze # :nodoc: + # Pages include the assembled head's inputs; absent page objects are nil. + PAGE_INPUTS = %i[io rel_prefix asset_rel_prefix current klass file].freeze # :nodoc: + TEMPLATE_INPUTS = { # :nodoc: + 'class.rhtml' => PAGE_INPUTS + [:breadcrumb], + 'index.rhtml' => PAGE_INPUTS, + 'page.rhtml' => PAGE_INPUTS, + 'servlet_not_found.rhtml' => PAGE_INPUTS + [:message], + 'servlet_root.rhtml' => PAGE_INPUTS + [:installed], + '_head.rhtml' => %i[rel_prefix asset_rel_prefix current klass file], '_aside_toc.rhtml' => [], '_footer.rhtml' => [:rel_prefix], '_header.rhtml' => [:rel_prefix], '_icons.rhtml' => [], '_sidebar_ancestors.rhtml' => [:klass], - ).transform_values(&:freeze).freeze + '_sidebar_classes.rhtml' => [:rel_prefix], + '_sidebar_extends.rhtml' => [:klass], + '_sidebar_includes.rhtml' => [:klass], + '_sidebar_installed.rhtml' => [:installed], + '_sidebar_methods.rhtml' => [:klass], + '_sidebar_pages.rhtml' => %i[rel_prefix current], + '_sidebar_search.rhtml' => [], + '_sidebar_sections.rhtml' => [:klass], + '_sidebar_toggle.rhtml' => [], + }.transform_values(&:freeze).freeze RDoc.add_generator self def initialize(store, options) super - aliki_template_dir = File.expand_path(File.join(__dir__, 'template', 'aliki')) - @template_dir = Pathname.new(aliki_template_dir) + @template_dir = TEMPLATE_DIR end ## @@ -159,8 +177,51 @@ def resolve_url(rel_prefix, url) "#{rel_prefix}/#{url}" end + #: (Pathname, ?bool, ?Class[ERB]) -> ERB + def template_for(file, page = true, klass = ERB) + cached = @template_cache[file] + # String and streaming ERB compile different output operations. Replace + # only owned compiled templates; custom ERB caches keep legacy behavior. + if cached.is_a?(CompiledTemplate) && cached.class != klass + @template_cache.delete(file) + end + + template = super + return template if template.is_a?(CompiledTemplate) + return template unless [ERB, ERBIO, ERBPartial].include?(template.class) + + if inputs = builtin_template_inputs(file) + template.extend CompiledTemplate + template.generator = self + template.render_inputs = inputs + end + template + end + private + #: (Pathname, ?Pathname?, **untyped) -> untyped + def render_page_template(template_file, out_file = nil, **inputs, &block) + return super unless builtin_template_inputs(template_file) + + erb_class = erb_class_for out_file + template = template_for template_file, true, erb_class + return super unless template.is_a?(CompiledTemplate) + + render_template template_file, out_file do |io| + CompiledTemplate::Inputs.new(io: io, **inputs) + end + end + + #: (Pathname) -> Array[Symbol]? + def builtin_template_inputs(file) + # Custom generators and templates retain their shared-binding behavior. + return unless self.class == Aliki && @template_dir.expand_path == TEMPLATE_DIR + return unless file.dirname.expand_path == TEMPLATE_DIR + + TEMPLATE_INPUTS[file.basename.to_s] + end + def template_encoding ::Encoding::UTF_8 end diff --git a/lib/rdoc/generator/compiled_template.rb b/lib/rdoc/generator/compiled_template.rb index 63b193e7ed..e8f372cde3 100644 --- a/lib/rdoc/generator/compiled_template.rb +++ b/lib/rdoc/generator/compiled_template.rb @@ -2,16 +2,23 @@ module RDoc module Generator - # Caches an executable renderer with declared inputs for a bundled HTML - # template. ERB#result remains available for binding-based evaluation. + # Caches a renderer with declared inputs for an Aliki template. Ordinary + # bindings still use ERB's original evaluation and shared-local semantics. module CompiledTemplate # :nodoc: + Inputs = Struct.new( + :io, :rel_prefix, :asset_rel_prefix, :current, :klass, :file, + :breadcrumb, :message, :installed, keyword_init: true + ) + attr_writer :generator, :render_inputs - #: (Hash[Symbol, untyped]) -> untyped - def render(inputs) + #: (?(Binding | Inputs)?) -> untyped + def result(context = nil) + return super unless context.is_a?(Inputs) + @compiled_renderer ||= def_module("render(#{@render_inputs.join(', ')})").instance_method(:render).bind(@generator) - @compiled_renderer.call(*@render_inputs.map { |name| inputs.fetch(name) }) + @compiled_renderer.call(*@render_inputs.map { |name| context[name] }) end end end diff --git a/lib/rdoc/generator/darkfish.rb b/lib/rdoc/generator/darkfish.rb index d7b321f81c..5ce314b1d4 100644 --- a/lib/rdoc/generator/darkfish.rb +++ b/lib/rdoc/generator/darkfish.rb @@ -5,7 +5,6 @@ require 'fileutils' require 'pathname' require_relative 'markup' -require_relative 'compiled_template' module RDoc module Generator @@ -81,32 +80,6 @@ class Darkfish DESCRIPTION = 'HTML generator, written by Michael Granger' - # Page inputs include the assembled head's contract. Absent page objects - # are nil, not missing locals. Partials only receive their own inputs. - PAGE_INPUTS = %i[io rel_prefix asset_rel_prefix current klass file].freeze # :nodoc: - TEMPLATE_INPUTS = { # :nodoc: - 'class.rhtml' => PAGE_INPUTS + [:breadcrumb], - 'index.rhtml' => PAGE_INPUTS, - 'page.rhtml' => PAGE_INPUTS, - 'servlet_not_found.rhtml' => PAGE_INPUTS + [:message], - 'servlet_root.rhtml' => PAGE_INPUTS + [:installed], - 'table_of_contents.rhtml' => PAGE_INPUTS, - '_head.rhtml' => %i[rel_prefix asset_rel_prefix current klass file], - '_footer.rhtml' => [], - '_sidebar_classes.rhtml' => [:rel_prefix], - '_sidebar_extends.rhtml' => [:klass], - '_sidebar_includes.rhtml' => [:klass], - '_sidebar_installed.rhtml' => [:installed], - '_sidebar_methods.rhtml' => [:klass], - '_sidebar_navigation.rhtml' => [:rel_prefix], - '_sidebar_pages.rhtml' => %i[rel_prefix current], - '_sidebar_parent.rhtml' => [:klass], - '_sidebar_search.rhtml' => [], - '_sidebar_sections.rhtml' => [:klass], - '_sidebar_table_of_contents.rhtml' => [:current], - '_sidebar_toggle.rhtml' => [], - }.transform_values(&:freeze).freeze - ## # The relative path to style sheets and javascript. By default this is set # the same as the rel_prefix. @@ -321,7 +294,7 @@ def generate_index @main_page = @files.find { |f| f.full_name == @options.main_page } inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, current: @main_page } - render_template template_file, out_file, inputs: inputs do |io| + render_page_template template_file, out_file, **inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -365,7 +338,7 @@ def generate_class(klass, template_file = nil) rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, current: current, klass: klass, breadcrumb: breadcrumb, } - render_template template_file, out_file, inputs: inputs do |io| + render_page_template template_file, out_file, **inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -493,7 +466,7 @@ def generate_page(file) rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, current: current, file: file, } - render_template template_file, out_file, inputs: inputs do |io| + render_page_template template_file, out_file, **inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:current, current) @@ -520,7 +493,7 @@ def generate_servlet_not_found(message) @title = 'Not Found' inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, message: message } - render_template template_file, inputs: inputs do |io| + render_page_template template_file, **inputs do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -551,7 +524,7 @@ def generate_servlet_root(installed) @title = 'Local RDoc Documentation' inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix, installed: installed } - render_template template_file, inputs: inputs do |io| binding end + render_page_template template_file, **inputs do |io| binding end rescue => e error = Error.new \ "error generating servlet_root: #{e.message} (#{e.class})" @@ -578,8 +551,7 @@ def generate_table_of_contents @title = "Table of Contents - #{@options.title}" - inputs = { rel_prefix: rel_prefix, asset_rel_prefix: asset_rel_prefix } - render_template template_file, out_file, inputs: inputs do |io| + render_template template_file, out_file do |io| here = binding # suppress 1.9.3 warning here.local_variable_set(:asset_rel_prefix, asset_rel_prefix) @@ -667,7 +639,9 @@ def render(file_name) template = template_for template_file, false, ERBPartial - template_result template, @context, template_file + template.filename = template_file.to_s + + template.result @context end ## @@ -676,31 +650,26 @@ def render(file_name) # # Both +template_file+ and +out_file+ should be Pathname-like objects. # - # Bundled templates render with explicit +inputs+. For legacy/custom - # templates an io is yielded and must be captured by binding in the caller. + # An io will be yielded which must be captured by binding in the caller. - def render_template(template_file, out_file = nil, inputs: nil) # :yield: io - io_output = out_file && !@dry_run && @file_output - erb_klass = io_output ? ERBIO : ERB + def render_template(template_file, out_file = nil) # :yield: io + erb_klass = erb_class_for out_file template = template_for template_file, true, erb_klass - inputs = if inputs && template.is_a?(CompiledTemplate) - { current: nil, klass: nil, file: nil }.merge(inputs) - end - if io_output + if erb_klass == ERBIO debug_msg "Outputting to %s" % [out_file.expand_path] out_file.dirname.mkpath out_file.open 'w', 0644 do |io| io.set_encoding @options.encoding - @context = inputs ? inputs.merge(io: io) : yield(io) + @context = yield io template_result template, @context, template_file end else - @context = inputs ? inputs.merge(io: nil) : yield(nil) + @context = yield nil output = template_result template, @context, template_file @@ -718,11 +687,7 @@ def render_template(template_file, out_file = nil, inputs: nil) # :yield: io def template_result(template, context, template_file) template.filename = template_file.to_s - if template.is_a?(CompiledTemplate) && context.is_a?(Hash) - template.render context - else - template.result context - end + template.result context rescue NoMethodError => e raise Error, "Error while evaluating %s: %s" % [ template_file.expand_path, @@ -751,11 +716,6 @@ def template_for(file, page = true, klass = ERB) end template = klass.new template, trim_mode: '-', eoutvar: erbout - if [ERB, ERBIO, ERBPartial].include?(klass) && (inputs = builtin_template_inputs(file)) - template.extend CompiledTemplate - template.generator = self - template.render_inputs = inputs - end @template_cache[file] = template template end @@ -859,19 +819,16 @@ def group_classes_by_namespace_for_sidebar(classes) private - #: (Pathname) -> Array[Symbol]? - def builtin_template_inputs(file) - # Custom generators and templates may depend on shared-binding mutation - # or a different lexical scope, so keep their original ERB evaluation. - return unless self.class == Darkfish || self.class == Aliki - - directory = @template_dir.expand_path - return unless file.dirname.expand_path == directory - theme = directory.basename.to_s - return unless %w[aliki darkfish].include?(theme) && directory == Pathname.new(__dir__) + 'template' + theme + #: (Pathname?) -> Class[ERB] + def erb_class_for(out_file) + out_file && !@dry_run && @file_output ? ERBIO : ERB + end - contracts = theme == 'aliki' ? Aliki::TEMPLATE_INPUTS : TEMPLATE_INPUTS - contracts[file.basename.to_s] + # Themes can render with explicit inputs without changing the legacy + # render_template API or restricting the binding supplied by its caller. + #: (Pathname, ?Pathname?, **untyped) -> untyped + def render_page_template(template_file, out_file = nil, **inputs, &block) + render_template(template_file, out_file, &block) end def template_encoding diff --git a/lib/rdoc/generator/template/darkfish/_head.rhtml b/lib/rdoc/generator/template/darkfish/_head.rhtml index a6d9129ee2..c90635877a 100644 --- a/lib/rdoc/generator/template/darkfish/_head.rhtml +++ b/lib/rdoc/generator/template/darkfish/_head.rhtml @@ -3,7 +3,7 @@ <%= h @title %> -<%- if defined?(klass) && klass %> +<%- if defined?(klass) %> "> <%- if klass.comment.empty? %> @@ -11,7 +11,7 @@ <%- else %> "> <%- end %> -<%- elsif defined?(file) && file %> +<%- elsif defined?(file) %> "> <%- elsif @title %> @@ -21,7 +21,7 @@ <%- end %> <%- if canonical_url = @options.canonical_root %> -<% canonical_url = current.canonical_url if defined?(current) && current %> +<% canonical_url = current.canonical_url if defined?(current) %> <%- end %> diff --git a/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml b/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml index 9dd1f6812a..7de407e999 100644 --- a/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml +++ b/lib/rdoc/generator/template/darkfish/_sidebar_pages.rhtml @@ -1,5 +1,5 @@ <%- simple_files = @files.select { |f| f.text? } %> -<%- if defined?(current) && current %> +<%- if defined?(current) %> <%- dir = current.full_name[%r{\A[^/]+(?=/)}] || current.page_name %> <%- end %> <%- unless simple_files.empty? %> diff --git a/lib/rdoc/generator/template/darkfish/index.rhtml b/lib/rdoc/generator/template/darkfish/index.rhtml index 6a75bebd35..d2ebcf69f7 100644 --- a/lib/rdoc/generator/template/darkfish/index.rhtml +++ b/lib/rdoc/generator/template/darkfish/index.rhtml @@ -7,7 +7,7 @@ <%= render '_sidebar_search.rhtml' %> - <%= render '_sidebar_table_of_contents.rhtml' if defined?(current) && current %> + <%= render '_sidebar_table_of_contents.rhtml' if defined?(current) %> <%= render '_sidebar_pages.rhtml' %> <%= render '_sidebar_classes.rhtml' %> diff --git a/test/rdoc/generator/template_test.rb b/test/rdoc/generator/template_test.rb index da85a44afa..3fdf65a959 100644 --- a/test/rdoc/generator/template_test.rb +++ b/test/rdoc/generator/template_test.rb @@ -17,14 +17,23 @@ def setup end def test_builtin_pages_and_partials_are_evaluated_only_once - %w[aliki darkfish].each do |theme| - generator = generator_for theme + generator = generator_for 'aliki' - first_evaluations = count_evaluations { generator.generate_class @alpha } - second_evaluations = count_evaluations { generator.generate_class @beta } + first_evaluations = count_evaluations { generator.generate_class @alpha } + second_evaluations = count_evaluations { generator.generate_class @beta } - assert_operator first_evaluations, :>, 0, theme - assert_equal 0, second_evaluations, theme + assert_operator first_evaluations, :>, 0 + assert_equal 0, second_evaluations + end + + def test_darkfish_keeps_binding_based_rendering + generator = generator_for 'darkfish' + generator.generate_class @alpha + + assert_operator count_evaluations { generator.generate_class @beta }, :>, 0 + assert_kind_of Binding, generator.instance_variable_get(:@context) + generator.instance_variable_get(:@template_cache).each_value do |template| + assert_not_kind_of RDoc::Generator::CompiledTemplate, template end end @@ -43,40 +52,43 @@ def test_cached_renderers_are_methods_bound_to_the_generator end def test_compiled_renderers_only_accept_declared_inputs - %w[aliki darkfish].each do |theme| - generator = generator_for theme - generator.generate_class @alpha + generator = generator_for 'aliki' + generator.generate_class @alpha - page = generator.template_for generator.template_dir + 'class.rhtml' - renderer = page.instance_variable_get :@compiled_renderer - assert_kind_of Method, renderer, theme - assert_equal %i[io rel_prefix asset_rel_prefix current klass file breadcrumb], - renderer.parameters.map(&:last), theme + page = generator.template_for generator.template_dir + 'class.rhtml' + renderer = page.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer + assert_equal %i[io rel_prefix asset_rel_prefix current klass file breadcrumb], + renderer.parameters.map(&:last) - partial = generator.template_for generator.template_dir + '_sidebar_sections.rhtml', false, RDoc::ERBPartial - renderer = partial.instance_variable_get :@compiled_renderer - assert_kind_of Method, renderer, theme - assert_equal [:klass], renderer.parameters.map(&:last), theme - end + partial = generator.template_for generator.template_dir + '_sidebar_sections.rhtml', false, RDoc::ERBPartial + renderer = partial.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer + assert_equal [:klass], renderer.parameters.map(&:last) end - def test_builtin_rendering_does_not_inspect_bindings - %w[aliki darkfish].each do |theme| - generator = generator_for theme - binding_reads = [] - trace = TracePoint.new(:call, :c_call) do |event| - if event.self.is_a?(Binding) && %i[local_variables local_variable_get].include?(event.method_id) - binding_reads << event.method_id - end - end + def test_aliki_contracts_match_its_owned_templates + generator = generator_for 'aliki' + templates = generator.template_dir.children.select { |file| file.extname == '.rhtml' }.map { |file| file.basename.to_s } + + assert_equal templates.sort, RDoc::Generator::Aliki::TEMPLATE_INPUTS.keys.sort + end - trace.enable do - generator.generate_class @alpha - generator.generate_class @beta + def test_builtin_rendering_does_not_inspect_bindings + generator = generator_for 'aliki' + binding_reads = [] + trace = TracePoint.new(:call, :c_call) do |event| + if event.self.is_a?(Binding) && %i[local_variables local_variable_get].include?(event.method_id) + binding_reads << event.method_id end + end - assert_empty binding_reads, theme + trace.enable do + generator.generate_class @alpha + generator.generate_class @beta end + + assert_empty binding_reads end def test_clearing_the_template_cache_discards_its_compiled_renderers @@ -136,59 +148,79 @@ def test_cached_templates_write_to_current_output_stream assert_not_include beta_html, 'First page documentation: café.' end + def test_cached_templates_follow_output_mode_changes + generator = generator_for 'aliki' + generator.generate_class @alpha + + generator.file_output = true + generator.generate_class @beta + beta_html = File.read File.join(@test_home, 'Beta.html') + assert_include beta_html, 'Second page documentation.' + + generator.file_output = false + assert_include generator.generate_class(@alpha), 'First page documentation: café.' + assert_equal 0, count_evaluations { generator.generate_class @beta } + end + + def test_cached_streaming_templates_can_switch_to_dry_run + generator = generator_for 'aliki' + generator.file_output = true + generator.generate_class @alpha + + generator.dry_run = true + html = generator.generate_class @beta + + assert_include html, 'Second page documentation.' + refute_file File.join(@test_home, 'Beta.html') + assert_equal 0, count_evaluations { generator.generate_class @alpha } + end + def test_index_reuses_its_renderer_when_the_main_page_changes - %w[aliki darkfish].each do |theme| - @options.main_page = nil - generator = generator_for theme - html = generator.generate_index - assert_include html, 'This is the API documentation' - assert_not_include html, '

Table of Contents

' - - template = generator.template_for generator.template_dir + 'index.rhtml' - renderer = template.instance_variable_get :@compiled_renderer - assert_kind_of Method, renderer, theme - - readme = @store.add_file "README_#{theme}.rdoc", parser: RDoc::Parser::Simple - readme.comment = "= Main page heading\n\n== Second heading" - @options.main_page = readme.full_name - generator.refresh_store_data - html = generator.generate_index - assert_include html, 'Main page heading' - assert_include html, '

Table of Contents

' if theme == 'darkfish' - assert_same renderer, template.instance_variable_get(:@compiled_renderer), theme - - @options.main_page = nil - html = generator.generate_index - assert_include html, 'This is the API documentation' - assert_not_include html, '

Table of Contents

' - assert_same renderer, template.instance_variable_get(:@compiled_renderer), theme - end + generator = generator_for 'aliki' + html = generator.generate_index + assert_include html, 'This is the API documentation' + + template = generator.template_for generator.template_dir + 'index.rhtml' + renderer = template.instance_variable_get :@compiled_renderer + assert_kind_of Method, renderer + + readme = @store.add_file 'README.rdoc', parser: RDoc::Parser::Simple + readme.comment = "= Main page heading\n\n== Second heading" + @options.main_page = readme.full_name + generator.refresh_store_data + html = generator.generate_index + assert_include html, 'Main page heading' + assert_same renderer, template.instance_variable_get(:@compiled_renderer) + + @options.main_page = nil + html = generator.generate_index + assert_include html, 'This is the API documentation' + assert_not_include html, '

<%= [klass.equal?(current), out_file.basename.to_s, @@ -236,7 +268,7 @@ def test_custom_class_templates_can_access_incidental_caller_locals end def test_builtin_templates_preserve_explicit_binding_evaluation_without_inputs - generator = generator_for 'darkfish' + generator = generator_for 'aliki' generator.generate_index template_file = generator.template_dir + 'index.rhtml' template = generator.template_for template_file @@ -253,7 +285,7 @@ def test_builtin_templates_preserve_explicit_binding_evaluation_without_inputs end def test_custom_page_using_builtin_partials_keeps_shared_binding_behavior - generator = generator_for 'darkfish' + generator = generator_for 'aliki' generator.generate_class @beta page = Pathname.new(@test_home) + 'custom.rhtml' File.write page, '<% render "_footer.rhtml" %><%= @context.local_variable_get(:_erbout__footer) %>' @@ -279,8 +311,27 @@ def result(context = nil) assert_equal 'custom result', template.result(generator.instance_eval { binding }) end + def test_cached_custom_erb_classes_receive_the_original_page_binding + generator = generator_for 'aliki' + erb_class = Class.new(ERB) do + def result(context = nil) + "custom #{context.local_variable_get(:current).full_name}" + end + end + generator.template_for generator.template_dir + 'class.rhtml', true, erb_class + + assert_equal 'custom Alpha', generator.generate_class(@alpha) + assert_equal 'custom Beta', generator.generate_class(@beta) + end + + def test_render_inputs_reject_unknown_fields + assert_raise(ArgumentError) do + RDoc::Generator::CompiledTemplate::Inputs.new(incidental_local: true) + end + end + def test_generator_subclasses_keep_binding_based_rendering - generator = generator_for 'darkfish', Class.new(RDoc::Generator::Darkfish) + generator = generator_for 'aliki', Class.new(RDoc::Generator::Aliki) generator.generate_class @alpha evaluations = count_evaluations { generator.generate_class @beta } @@ -288,6 +339,17 @@ def test_generator_subclasses_keep_binding_based_rendering assert_operator evaluations, :>, 0 end + def test_generator_subclasses_can_override_the_legacy_render_template_signature + generator_class = Class.new(RDoc::Generator::Aliki) do + def render_template(template_file, out_file = nil, &block) + super + end + end + generator = generator_for 'aliki', generator_class + + assert_include generator.generate_class(@alpha), 'First page documentation: café.' + end + def test_template_errors_keep_the_template_filename generator = generator_for 'aliki' @alpha.define_singleton_method(:description) { self.no_such_method }