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

Handle api links #32

Draft
wants to merge 5 commits into
base: parser
Choose a base branch
from
Draft

Handle api links #32

wants to merge 5 commits into from

Conversation

mkondratek
Copy link
Contributor

@mkondratek mkondratek commented Sep 17, 2020

I did not manage to handle top level functions and functions with params (?) yet

I think that the DRI resolution for API links works fine now however there are still a few cases that do not work 😕

  • top level fun
  • member fun
  • both above (with/without params; not sure if it is the other case actually)

"bad" link resolution gives smth like:

<p class="paragraph"><span data-unresolved-link="com.virtuslab.dokka.site//loadTemplateFile/#java.io.File/PointingToDeclaration/">tolefun</span></p>

@mkondratek
Copy link
Contributor Author

I tried to find out why (only some of) the (proper?) DRIs are not being resolved to the links.
I ended up here:

 override fun resolve(dri: DRI, sourceSets: Set<DisplaySourceSet>, context: PageNode?): String? =
        sourceSets.ifEmpty { setOf(null) }.mapNotNull { sourceSet ->
            val driWithSourceSet = DRIWithSourceSet(dri, sourceSet)
            getLocalLocation(driWithSourceSet, context)
                ?: getLocalLocation(driWithSourceSet.copy(dri = dri.copy(target = PointingToDeclaration)), context)
                // Not found in PageGraph, that means it's an external link
                ?: getExternalLocation(dri, sourceSets)
                ?: getExternalLocation(dri.copy(target = PointingToDeclaration), sourceSets)
        }.distinct().singleOrNull()

To be precise... getLocalLocation(driWithSourceSet, context) returns null for some cases (functions?).

@romanowski do you have any ideas?


}.let(allDRIs::get)

private fun String.resolveLinkToApi() = when {
Copy link
Member

Choose a reason for hiding this comment

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

This will work only partially for Kotlin since DRIs are a bir more complicated. @Kordyjan @kamildoleglo could you help here? Maybe we can reuse dokka code?

We should also create extension point to that will alow users to provide its own mapping String -> DRI?

Choose a reason for hiding this comment

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

TBH I'd personally discourage creating DRIs by hand if possible. Reusing the IntelliJ resolveKDocLink method feels more correct here. It will certainly limit the types of links possible (eg. no linking to overloaded methods, because the KDoc specification does not allow it for now), but will be more fail-proof.

If you have to create the DRIs manually though, the JavaClassReference looks wrong for starters (not really sure what issue it resolves in Dokka TBH, I'll try to find out). Moreover, toplevel methods (classes, etc.) should have a null packageName, not a blank one. Also looks like it's going to fail for elements with generic attributes, because you're not creating TypeReferences anywhere and for extension methods (and properties), because you're not setting the receiver anywhere

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.

Proper support for links
3 participants