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

Scaladoc: fix issues with incorrect external links and special characters #14516

Merged
merged 3 commits into from
Feb 28, 2022

Conversation

jchyb
Copy link
Contributor

@jchyb jchyb commented Feb 18, 2022

This includes escaping certain characters found in searchbar links (1st commit) and correctly resolving external links (2nd commit).

Fixes #14505 and #14504.

Unit tests for links with "#" character were also added.
@jchyb jchyb requested a review from pikinier20 February 18, 2022 16:22
Copy link
Contributor

@pikinier20 pikinier20 left a comment

Choose a reason for hiding this comment

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

The changes look good but something is broken. Searchbar in generated docs doesn't work (no entries after opening) and is throwing errors to console:
image

@jchyb jchyb force-pushed the scaladoc/fix-i14505-i14504 branch 2 times, most recently from e834986 to 6ad301f Compare February 22, 2022 09:11
This includes both fixing links found in searchbar, and ones accessible
from method names. In searchbar, links are resolved based on whether
they are absolute (pointing to an external website, like java docs) or
relative (pointing to another sub-website, with root being appended on
runtime). This is achieved by serializing another boolean field in
PageEntry.
@jchyb jchyb force-pushed the scaladoc/fix-i14505-i14504 branch from 6ad301f to 213e275 Compare February 22, 2022 09:28
@jchyb
Copy link
Contributor Author

jchyb commented Feb 22, 2022

Thank you for spotting this as this was not something that happened locally, now I've updated the PR with fixes. I no longer use new URI(_).isAbsolute() as URI parsing seemed to be the issue. Instead, an additional filed for searchData was added describing whether the searchData contains an internal link or one to the external website.

A similar technique with URIs was employed in a previous PR with Inkuire and, as it turns out, the issues would also happen there (just much more rarely), so I updated that in a separate commit. Unfortunately, as I could not add any new fields there the solution ended up being a little "hacky", but I hope it is at least explained elegantly in the comment.

@pikinier20 pikinier20 self-requested a review February 22, 2022 10:47
@pikinier20 pikinier20 self-assigned this Feb 22, 2022
Copy link
Contributor

@pikinier20 pikinier20 left a comment

Choose a reason for hiding this comment

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

Good job!

@TheElectronWill TheElectronWill merged commit 39b635c into scala:main Feb 28, 2022
@Nicolapps
Copy link

Thank you for the fixes!

@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken link for #:: on the Scaladoc website search results
5 participants