From 15b2a5f03e4fd55f690993d7f82ae91826fe13f5 Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Wed, 16 Aug 2023 22:27:09 -0500 Subject: [PATCH] Fix choice of main page Prior to this commit, `RDoc::Generator::SDoc#index` exhibited the following incorrect behaviors: 1. When `--main` was not specified, `index` would choose the alphabetically-first file as the main page, instead of the first file in the list of files. 2. When `--main` was specified but was prefixed with `--root`, and `--root` did not start with "rails/", `index` would choose the alphabetically-first file. (Such as when `--root` was an absolute path, or a deeply nested relative path, or simply a path for a non-Rails project.) 3. When `--main` was specified but was not in the list of files, `index` would choose the alphabetically-first file. This commit fixes `RDoc::Generator::SDoc#index` such that it: 1. Chooses the first file in the list of files as the default main page. 2. Handles any kind of `--main` path. 3. Raises an error when `--main` isn't among the rendered files. --- Rakefile | 7 +++--- lib/sdoc/generator.rb | 28 +++++++-------------- spec/rdoc_generator_spec.rb | 50 +++++++++++++++++++++++++++++++++++++ spec/spec_helper.rb | 10 +++++--- 4 files changed, 70 insertions(+), 25 deletions(-) diff --git a/Rakefile b/Rakefile index 4860fa0f..1ab138e7 100644 --- a/Rakefile +++ b/Rakefile @@ -43,13 +43,14 @@ class RailsTask < Rails::API::EdgeTask "doc/rails" end + # This file has been renamed in Rails 7.1. + # TODO: Remove this override some time after Rails 7.1 is released. def api_main - "rails/railties/RDOC_MAIN.md" + super.sub("RDOC_MAIN.rdoc", "RDOC_MAIN.md") end def component_root_dir(component) - path = File.join("rails", component) - return path + File.join("rails", component) end def setup_horo_variables diff --git a/lib/sdoc/generator.rb b/lib/sdoc/generator.rb index 41b0cefd..8434debc 100644 --- a/lib/sdoc/generator.rb +++ b/lib/sdoc/generator.rb @@ -87,6 +87,7 @@ def initialize(store, options) end @options.pipe = true + @original_dir = Pathname.pwd @template_dir = Pathname.new(options.template_dir) @base_dir = options.root @@ -119,6 +120,14 @@ def file_dir FILE_DIR end + ### Determines index page based on @options.main_page (or lack thereof) + def index + path = @original_dir.join(@options.main_page || @options.files.first || "") + file = @files.find { |file| @options.root.join(file.full_name) == path } + raise "Could not find main page #{path.to_s.inspect} among rendered files" if !file + file + end + protected ### Output progress information if debugging is enabled def debug_msg( *msg ) @@ -202,25 +211,6 @@ def generate_class_tree_level(classes, visited = {}) tree end - ### Determines index page based on @options.main_page (or lack thereof) - def index - # Break early to avoid a big if block when no main page is specified - default = @files.first - return default unless @options.main_page - - # TODO: Total hack to strip the source directory from the main page - # Since the file list does not include the root source path - clean_main = @options.main_page.gsub("rails/", "") - - if file = @files.find { |f| f.full_name == clean_main } - debug_msg "Found main at #{file}" - file - else - debug_msg "Falling back to default main at #{default}" - default - end - end - ### Copy all the resource files to output dir def copy_resources resources_path = @template_dir + RESOURCES_DIR diff --git a/spec/rdoc_generator_spec.rb b/spec/rdoc_generator_spec.rb index 4769a767..0d1957af 100644 --- a/spec/rdoc_generator_spec.rb +++ b/spec/rdoc_generator_spec.rb @@ -74,4 +74,54 @@ def parse_options(*options) _(parse_options("--title", "Docs Docs Docs!").title).must_equal "Docs Docs Docs!" end end + + describe "#index" do + before do + @dir = File.expand_path("../lib", __dir__) + @files = ["sdoc.rb", "sdoc/version.rb"].sort.reverse + end + + it "defaults to the first --files value" do + Dir.chdir(@dir) do + sdoc = rdoc_dry_run("--files", *@files).generator + _(sdoc.index.absolute_name).must_equal @files.first + end + end + + it "raises when the default value is not a file" do + sdoc = rdoc_dry_run("--files", @dir, "--exclude=(js|css|svg)$").generator + error = _{ sdoc.index }.must_raise + _(error.message).must_include @dir + end + + it "uses the value of --main" do + Dir.chdir(@dir) do + sdoc = rdoc_dry_run("--main", @files.first, "--files", *@files).generator + _(sdoc.index.absolute_name).must_equal @files.first + end + end + + it "raises when the main page is not among the rendered files" do + Dir.chdir(@dir) do + sdoc = rdoc_dry_run("--main", @files.first, "--files", @files.last).generator + error = _{ sdoc.index }.must_raise + _(error.message).must_include @files.first + end + end + + it "works when --root is specified" do + Dir.chdir(File.dirname(@dir)) do + root = File.basename(@dir) + @files.map! { |file| File.join(root, file) } + sdoc = rdoc_dry_run("--root", root, "--main", @files.first, "--files", *@files).generator + _(sdoc.index.absolute_name).must_equal @files.first + end + end + + it "works with absolute paths" do + @files.map! { |file| File.join(@dir, file) } + sdoc = rdoc_dry_run("--main", @files.first, "--files", *@files).generator + _(sdoc.index.absolute_name).must_equal @files.first + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ad8db8c9..f19b4419 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,13 +12,17 @@ def with_env(env, &block) ENV.replace(original_env) end +def rdoc_dry_run(*options) + RDoc::RDoc.new.tap do |rdoc| + rdoc.document(%W[--dry-run --quiet --format=sdoc --template=rails] + options.flatten) + end +end + # Returns an RDoc::TopLevel instance for the given Ruby code. def rdoc_top_level_for(ruby_code) # RDoc has a lot of internal state that needs to be initialized. The most # foolproof way to initialize it is by simply running it with a dummy file. - $rdoc_for_specs ||= RDoc::RDoc.new.tap do |rdoc| - rdoc.document(%W[--dry-run --quiet --format=sdoc --template=rails --files #{__FILE__}]) - end + $rdoc_for_specs ||= rdoc_dry_run("--files", __FILE__) $rdoc_for_specs.store = RDoc::Store.new