-
Notifications
You must be signed in to change notification settings - Fork 438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow customizing path prefix through options #1330
Conversation
Deploying rdoc with
|
Latest commit: |
271fff0
|
Status: | ✅ Deploy successful! |
Preview URL: | https://227da1f8.rdoc-6cd.pages.dev |
Branch Preview URL: | https://allow-customizing-path-prefi.rdoc-6cd.pages.dev |
1515099
to
76f604c
Compare
@jonathanhefner I think with these changes (Ignore Gemfile diffs) diff --git a/Gemfile b/Gemfile
index 0802717..8b0e609 100644
--- a/Gemfile
+++ b/Gemfile
@@ -8,9 +8,11 @@ gem "hoe"
gem "minitest"
gem "puma"
-if ENV["rdoc"] == "master"
- gem "rdoc", :github => "ruby/rdoc"
-end
+# if ENV["rdoc"] == "master"
+# gem "rdoc", :github => "ruby/rdoc"
+# end
gem "importmap-rails"
gem "railties"
+
+gem "rdoc", :github => "ruby/rdoc", branch: "allow-customizing-path-prefix-through-options"
diff --git a/lib/sdoc/generator.rb b/lib/sdoc/generator.rb
index 023d066..884cb53 100644
--- a/lib/sdoc/generator.rb
+++ b/lib/sdoc/generator.rb
@@ -73,6 +73,9 @@ class RDoc::Generator::SDoc
end
@options.pipe = true
+ @options.class_module_path_prefix = "classes"
+ @options.file_path_prefix = "files"
+
@original_dir = Pathname.pwd
@template_dir = Pathname(options.template_dir)
@output_dir = Pathname(@options.op_dir).expand_path(options.root)
diff --git a/lib/sdoc/rdoc_monkey_patches.rb b/lib/sdoc/rdoc_monkey_patches.rb
index fac1297..c8ab744 100644
--- a/lib/sdoc/rdoc_monkey_patches.rb
+++ b/lib/sdoc/rdoc_monkey_patches.rb
@@ -1,19 +1,5 @@
require "rdoc"
-RDoc::TopLevel.prepend(Module.new do
- def path
- File.join("files", super)
- end
-end)
-
-
-RDoc::ClassModule.prepend(Module.new do
- def path
- File.join("classes", super)
- end
-end)
-
-
RDoc::TopLevel.prepend(Module.new do
attr_writer :path
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index ada0ca7..a7acfb1 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -28,7 +28,11 @@ def rdoc_top_level_for(ruby_code)
# foolproof way to initialize it is by simply running it with a dummy file.
$rdoc_for_specs ||= rdoc_dry_run("--files", __FILE__)
- $rdoc_for_specs.store = RDoc::Store.new(RDoc::Options.new)
+ options = RDoc::Options.new
+ options.class_module_path_prefix = "classes"
+ options.file_path_prefix = "files"
+
+ $rdoc_for_specs.store = RDoc::Store.new(options)
Dir.mktmpdir do |dir|
path = "#{dir}/ruby_code.rb" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
In #1304, I removed the ability to set path prefix through patching Darkfish generator. But it turns out that it's used in `sdoc`. See #1304 (comment) But the original implementation was brittle and confusing. So instead of just restoring it, I think allowing the customization through options is a better approach.
76f604c
to
271fff0
Compare
Yes, something like that could work. Would these |
I'm open to both 6.13.1 and 6.14 but prefer the latter as it's technically a new feature. Can you give me more context on how sdoc manages:
|
Normally, I would agree that 6.14 is more appropriate. However, I think this feature arguably should have been a part of 6.13.0, so it seems reasonable to include it in 6.13.1. Also, 6.13.0 already deviated semantic versioning by including breaking changes; adding a feature in 6.13.1 isn't much more of a deviation.
To be honest, I'm not sure if there is a policy for those things. I do not know what all is downstream of SDoc, so I am just trying to keep things in working order. |
I'm asking because we could focus on upstreaming/reducing sdoc's rdoc patches next, which I think will be beneficial to both sides. But if sdoc still needs to keep the patches around due to not being able to bump the required rdoc version, then it just won't be as effective. |
Ah, I see. In that case, I agree it would be beneficial to upstream some of the patches. All of the monkey patches apply idempotent changes, so even if SDoc can't drop them now, it won't be impacted by RDoc adopting them. And then SDoc could drop them at some point in the future. List of patches:
|
@jonathanhefner Thank you for the detailed list. I've created #1333 to remind ourselves to act on them later. |
Diff: v6.13.0...master I consider #1330 a fix too because the new options are introduced as an better alternative for a accidental breaking change.
@jonathanhefner |
Thank you, @st0012! ❤️ |
In #1304, I removed the ability to set path prefix through patching Darkfish generator. But it turns out that it's used in
sdoc
.See #1304 (comment)
But the original implementation was brittle and confusing. So instead of just restoring it, I think allowing the customization through options is a better approach.