Resolve @ref bindings in current module and Main

[goerz]

For any @ref link in a docstring to a code object, Documenter currently tries to find a binding for the code in the module where the docstring is defined. This often requires importing symbols only because they appear in a @ref. In some cases, this is not even possible, as it would create circular references. See also the very eloquent description in #2462 (comment)

With this change, Documenter now tries to find binding for @ref links in docstrings first in the local module, and then in Main, which is basically docs/make.jl. Thus, anything imported in docs/make.jl can be referenced anywhere.

Closes #2462

[goerz]

This is ready for feedback, although as I mentioned in #2462 (comment), I might want to test this out "in production" for a little bit.

Before this can be merged, I should probably add a specific test. This PR would probably also be a good opportunity to actually document how the bindings for @ref links are found.

[har7an]

@goerz Just saw you picked up work on this, thanks a lot!

After our discussion in #2462 I decided to "fully qualify" all inter-module refs (using the full ref path instead of relative refs), downgrade errors to warnings and live with the messy build output. With the changes from your branch I can build the docs and the links in the resulting HTML all seem to work!

Unfortunately I can't share the code here (it's private...), but I'll happily test any additional changes, if any.

[goerz]

You can play around with https://github.com/goerz/DocumenterResolveXRefInMainPrototype.jl, which is this a packaged version of this PR, so I can test it "in the wild" more easily.

This will take more work, though, because it seems like

Documenter currently tries to find a binding for the code in the module where the docstring is defined

isn't quite right. The code in Documenter for resolving docstrings is quite convoluted, and poorly documented.

For example,

Note that depending on what the CurrentModule is set to, a docstring @ref may need to be prefixed by the module which defines it.

is quite unclear. So I'm going to have to read the code more to really understand the current code, and how it interacts with this proposed PR, and then document that.

That being said, this PR, respectively the packaged DocumenterResolveXRefInMainPrototype seems to do exactly what I want (I just don't fully understand how it does what I want)

[mortenpi]

For the original case, doesn't

[`Blarg`](@ref Main.JDIMD.B.Blarg)

already just work?

[goerz]

I would never have thought of that trick, but it does indeed work!

So that just makes it a policy decision then, and I'm not quite sure what to push for. It might still be a good idea to decide that we always want to try resolving reference in Main if they cannot be resolved in :CurrentModule. The main effect of that would be that all fully qualified names work as long as the package is loaded in docs/main.jl. Realistically, I think this will result in behavior that "just works" for most people when right now they'd get an error, without them ever having to really think about where @ref links get resolved.

So let me know that you think, and I'll either finish up this PR with a test, or make a new PR that just does some refactoring and adds documentation. The refactoring would change the internal function find_docref(code, meta, page) to find_docref(code, curmod, page) with curmod = get(meta, :CurrentModule, Main) (making it more reusable for plugins) and try to improve some rather confusing error messages.

The decision is basically whether, in the documentation, we want to have the paragraph below with or without the crossed-out text:

The code enclosed in the backticks for such a reference will be evaluated first in the CurrentModule given in the @meta block of the current page, and second in Main, i.e., the context of the docs/make.jl file. For @ref links inside a docstring, the CurrentModule is automatically set to the module containing the docstring.

That's basically what it boils down to from the user's perspective, even though the full behavior is quite complicated (see my notes from diving into this)

If we decide to stick to the current behavior (and then we should add a "tip" about Main to the documentation), I might still consider writing myself a plugin to also resolve in Main, If I can't get used to writing

… package must be set as the default AD provider using
[`QuantumControl.set_default_ad_framework`](@ref Main.QuantumControl.set_default_ad_framework).

instead of just [`QuantumControl.set_default_ad_framework`](@ref).

But we'll see… either decision works for me!

[har7an]

Just my personal opinion, as a person that isn't primarily a Julia developer, in case anyone cares: I think that @goerz proposal is good to have from a user perspective and I'd vote for that, so to speak.

Most other languages I've written extensive documentation for so far seem to have a mechanism that resolves "full" references. Having to prepend Main comes unexpected with that background, but I would agree that, so long as it's clearly mentioned in the docs, it's manageable.

[mortenpi]

Hmm. I am not sure we want to encourage referencing objects just because they are available Main. Mostly, this creates a situation where the documentation references are dependent on arbitrary global state.

That said, being able to easily reference stuff outside the current module seems also quite reasonable, and having to prepend Main. is a bit ugly. So.. could we do a slightly restricted middle ground and only allow referencing top-level modules as fallbacks in Main?

I.e. the logic would be that you first do the current logic, where you search from the current module, and then if that fails, you try to see if the reference is a $(module).$(..), and see if $module is (1) available in Main, (2) a module, and (3) a top-level module (i.e. parentmodule(module) == Main.

This should mean that references like

[`set_default_ad_framework`](@ref QuantumControl.set_default_ad_framework)

and

[`QuantumControl.set_default_ad_framework`](@ref)

would work, satisfying

Most other languages I've written extensive documentation for so far seem to have a mechanism that resolves "full" references. Having to prepend Main comes unexpected

But you would not be able to refer to objects just because they are in Main.

[goerz]

I agree: we should restrict the fallback to Main only to fully qualified @ref links.

I.e. the logic would be that you first do the current logic, where you search from the current module, and then if that fails, you try to see if the reference is a $(module).$(..), and see if $module is (1) available in Main, (2) a module, and (3) a top-level module (i.e. parentmodule(module) == Main.

Repeating the "current logic" but with Main instead of CurrentModule should take care of (1) and (2). If we get manage to get a valid binding out of Main, then all we need to do to make sure the code in the @ref is a fully qualified name is to check startswith(code, string(binding.mod)).

I added this fix in the latest commit. I still have to add a test and changelog entry before this is ready to go.

[goerz]

@mortenpi I finally got around to adding the test and changelog.

This should be ready to merge now. As discussed, the fallback resolution to Main is only allowed for fully qualified names. I've been testing this in my QuantumControl packages, and it's been working very nicely: this seems like the final missing piece for being able to link between any two docstrings at all without having to jump through any crazy hoops. So having this will be extremely nice!

This PR should also make the error messages when @ref links fail to resolve a lot nicer and less confusing (see the test).

[mortenpi]

LGTM! Just a couple of phrasing suggestions -- feel free to take them or leave them, and then merge.

[har7an]

I can't comment on the code-side of things, but IMO the new docs along with the examples suggested by @mortenpi make it very clear what's supported and how it works.