Skip to content
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

Add breadcrumb list #973

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add breadcrumb list #973

wants to merge 6 commits into from

Conversation

unasuke
Copy link

@unasuke unasuke commented Feb 6, 2023

Motivation

When reading several modules has deeply nested namespaces like OpenSSL, the breadcrumb list is very useful.
In Japanese document (called rurema) has breadcrumb like this.

image

So, I introduce breadcrumb into rdoc.
I tested on ruby/rdoc and ruby/ruby.

ruby/rdoc

3f19b7a572b883291b7e8f66ec1726fd.mp4

ruby/ruby

26e76f9e48ba1567d606c2efa9001ee7.mp4

Changes

Add methods to RDoc::ClassModule to build breadcrumb metadata.

  • RDoc::ClassModule#nesting_namespaces
    • returns array of namespaces that splitted full_name by ::
  • RDoc::ClassModule#fully_qualified_nesting_namespaces
    • returns an array of fully qualified namespaces
    • To find RDoc::ClassModule instance from RDoc::Store#classes_hash and RDoc::Store#modules_hash

Add private methods to RDoc::Generator::Darkfish to build breadcrumb data for easy-to-use template erb.

  • namespaces_to_class_modules
    • generate hash e.g. { "RDoc" => `RDoc::NormalModule` instance, "ClassModule" => `RDoc::NormalModule` instance }
  • generate_namespaces_breadcrumb
    • generate array e.g. [{:name=>"RDoc", :path=>"../RDoc.html", :self=>false}, {:name=>"ClassModule", :path=>"../RDoc/ClassModule.html", :self=>true}]

Then, display the breadcrumb in the class template.

My concerns

  • Should be displayed breadcrumb on top-level module/class?
  • Should be escape class/module name?
  • Should be made configurable to enable or disable breadcrumb?

@unasuke unasuke changed the title Add breadcrumb Add breadcrumb list Feb 6, 2023
@unasuke
Copy link
Author

unasuke commented Feb 8, 2023

In Japanese document (called rurema) has breadcrumb like this.

To be precise, what is displayed in "rurema" is not a breadcrumb list but an inherited list of classes/modules. And this change don't provide an inherited list.

Comment on lines 310 to 313
return namespaces if namespaces.length < 2
@fqns ||= namespaces.map.with_index do |_, i|
namespaces[0..i].join("::")
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about

Suggested change
return namespaces if namespaces.length < 2
@fqns ||= namespaces.map.with_index do |_, i|
namespaces[0..i].join("::")
end
@fqns ||= namespaces.inject([]) do |list, n|
list << (list.empty? ? n : "#{list.last}::#{n}")
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed at 0e05c99

@nobu
Copy link
Member

nobu commented Jul 5, 2023

I'm a little concerned about whether the plural term "namespaces" is appropriate here.

@unasuke
Copy link
Author

unasuke commented Jul 5, 2023

@nobu Is it better for rename to namespace?

@nobu
Copy link
Member

nobu commented Jul 6, 2023

The word "namespaces" feels ambiguous; it doesn't represent that it contains what namespaces.
What about "nesting namespaces" or something?

@unasuke
Copy link
Author

unasuke commented Jul 6, 2023

@nobu Renamed at 11d3c95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants