rubydoc: fix sidebar navigation links on nested pages

[GunniBusch]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

---

• AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

---

This fixes #21588
It headers.erb and adds a base tag

[Copilot]

Pull request overview

Updates Homebrew’s overridden YARD app.js to fix broken sidebar navigation when browsing nested RubyDoc pages (as reported in #21588), and makes minor docstring formatting tweaks in an autogenerated Sorbet RBI.

Changes:

• Add/override YARD template app.js with a postMessage-driven navigation handler that fetches and swaps #main content for sidebar navigation.
• Re-run/adjust parts of the page-init logic after dynamic navigation.
• Minor comment formatting changes in parser@3.3.8.0.rbi.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 6 comments.

File	Description
Library/Homebrew/yard/templates/default/fulldoc/html/js/app.js	Overrides YARD JS to handle sidebar navigation on nested pages by resolving URLs and swapping main content.
Library/Homebrew/sorbet/rbi/parser@3.3.8.0.rbi	Small docstring formatting changes in an autogenerated RBI file.

Files not reviewed (1)

• Library/Homebrew/sorbet/rbi/parser@3.3.8.0.rbi: Language not supported

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[MikeMcQuaid]

This is a lot of extra code added for what should be a relatively simple regression?

@p-linnane does this related to the recent GitHub Pages work perhaps?

[GunniBusch]

Ok I managed to remove the big app.js file.

I overrode https://github.com/lsegal/yard/blob/eddd10c3948021c34a887db403824ce5a892708b/templates/default/layout/html/headers.erb and added <base href='<%= u = url_for(''); u + (u != '' ? '/' : '') %>';'>. The content of the href part, has to be the relative url pointing to the base i.e. how to get to /rbdoc? or locally brew/docs` from the current path. To achieve this, I just reused the base bath detection from this template https://github.com/lsegal/yard/blob/eddd10c3948021c34a887db403824ce5a892708b/templates/default/layout/html/script_setup.erb#L3

The rbi file could be removed, but I could not generate the docs without the correct rbi of parser.

[MikeMcQuaid]

Thanks, much better, almost there!

[p-linnane]

I'm sure this is somehow related to the GitHub Pages migration.

[bevanjkay]

I'm sure this is somehow related to the GitHub Pages migration.

Yeah, I think it likely has something to do with when the basepath url changed when it was nested at docs.brew.sh/rubydoc - because the link to the index page in the breadcrumb links also ends up going to a 404 error.

[GunniBusch]

Ok, this is bigger than I thought. The core error is that the nav side bar, which is an iframe, does not navigate using the correct relative path. This means that if you click on a nav item that url e.g. /Bottle/somehtml.html is relative to the current url not the base url.

Sadly, my fix made it work, but also broke all other links, since those were already relative.

We can trace this back to multiple sources, but I do not think the GitHub Pages move is the cause.

I would like to share the main sources of this error:

1. There is only one class_list.html. This is technically fine, but it means we cannot use YARD templates to fix this at the nav bar source. As a result, the hrefs are always relative to the current base path, but do not point to the actual root.
2. app.js does not make use of the relpath variable defined in https://github.com/lsegal/yard/blob/main/templates/default/layout/html/script_setup.erb even though relpath always correctly points to the root.

I tried to find a small fix for this issue, but I keep running into new problems. The current solution worked, but only for root-level pages. Navigating into nested pages works until you refresh, which I missed testing yesterday.

The best approach might be to integrate app.js into the codebase, make a small targeted change, and then open a PR upstream so we can restore the intended behavior and eventually remove it.

A different solution, I currently test, would be to automatically change append the href with the relpath variable, which doesn't change the app.js, but I currently figuring that out

[MikeMcQuaid]

The best approach might be to integrate app.js into the codebase, make a small targeted change, and then open a PR upstream so we can restore the intended behavior and eventually remove it.

Worth a try. Ideally we'd commit a patch in here and apply only the patch rather than commit the full file so it's easier to review.

Thanks!

[GunniBusch]

The best approach might be to integrate app.js into the codebase, make a small targeted change, and then open a PR upstream so we can restore the intended behavior and eventually remove it.

Worth a try. Ideally we'd commit a patch in here and apply only the patch rather than commit the full file so it's easier to review.

Thanks!

I will try how far a patch brings us. in case we have to add the file, I will clearly indicate the changed lines.

[GunniBusch]

Ok i patched the js file, the changes are minimal, and the pure functional changes are

===================================================================
--- a/templates/default/fulldoc/html/js/app.js	original
+++ b/templates/default/fulldoc/html/js/app.js	modified
@@ -347,7 +347,8 @@
   "message",
   async (e) => {
     if (e.data.action === "navigate") {
-      const response = await fetch(e.data.url);
+      const rel_url = relpath + e.data.url;
+      const response = await fetch(rel_url);
       const text = await response.text();
       const parser = new DOMParser();
       const doc = parser.parseFromString(text, "text/html");
@@ -383,12 +384,12 @@
 
       document.getElementById("class_list_link").classList = classListLink;
 
-      const url = new URL(e.data.url, "https://localhost");
+      const url = new URL(rel_url, "https://localhost");
       const hash = decodeURIComponent(url.hash ?? "");
       if (hash) {
         document.getElementById(hash.substring(1)).scrollIntoView();
       }
-      history.pushState({}, document.title, e.data.url);
+      history.pushState({}, document.title, rel_url);
     }
   },
   false

[MikeMcQuaid]

@GunniBusch Could you apply the patch as part of the CI deploy/Jekyll build process and open it as an upstream PR?

[GunniBusch]

@GunniBusch Could you apply the patch as part of the CI deploy/Jekyll build process and open it as an upstream PR?

Yes I can open upstream pr:

Regarding the patch, I will double check, but I cant promise that I can find a solution especially a clean solution. Will let you know if I dont else if I do I will push a 2nd commit

Also sorry for this back and forth, but I just want to be open about everything 😀

[GunniBusch]

I added a second commit so it's better to compare the changes. Basically what happens now, I added a new js file that also listens for navigation messages, and replaces the data content, which in turn also affects the navigation in app.js.

I am also now opening a pr in yard fixing this issue.

[MikeMcQuaid]

Regarding the patch, I will double check, but I cant promise that I can find a solution especially a clean solution. Will let you know if I dont else if I do I will push a 2nd commit

Basically: wherever you downloaded that file from in the first place: do that in the CI job and then apply a patch to it which you commit to the repository. Make sense?

I am also now opening a pr in yard fixing this issue.

Thanks 🙇🏻

[GunniBusch]

Basically: wherever you downloaded that file from in the first place: do that in the CI job and then apply a patch to it which you commit to the repository. Make sense?

Done. checked the unmodified app.js into the codebase, added to docs.yml a section that will apply the patch, which is checked in to the same dir as app.js.