Skip to content

Commit

Permalink
Fix choice of main page
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jonathanhefner committed Aug 17, 2023
1 parent edb3ec8 commit 15b2a5f
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 25 deletions.
7 changes: 4 additions & 3 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 9 additions & 19 deletions lib/sdoc/generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 )
Expand Down Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions spec/rdoc_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 7 additions & 3 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 15b2a5f

Please sign in to comment.