-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix links to unexisting types sites #13917
Fix links to unexisting types sites #13917
Conversation
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.
The code looks good.
Could you fix the build, so I can click through it?
"-external-mappings:" + | ||
".*scala/.*::scaladoc3::https://dotty.epfl.ch/api/," + | ||
".*java/.*::javadoc::https://docs.oracle.com/javase/8/docs/api/", | ||
def scalacOptionsDocSettings(includeExternalMappings: Boolean = true) = { |
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.
Is this part needed for this fix?
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.
Yes, because by default we were adding a external mapping to dotty.epfl.ch in Scala documentation. Our mechanism to resolve links tries to match external mapping for symbols that are not present in docs. Since the private API was not present in docs, Scaladoc matched external mapping for it and generated unexisting link
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.
ok, makes sense
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.
Isn't it a general problem? I mean, maybe we should not try to match any link for members that are private, not workaround it this way
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.
That's why I introduced changes with filtering symbols that shouldn't be visible. Maybe we should once more go through whole code that gathers symbols and check if we filter all symbols correctly.
I removed the external link to dotty because it was redundant and should never be there.
47d52fc
to
40da3a1
Compare
closes #13867