-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Docs.jl: Fix documenting parametric functor calls #45367
base: master
Are you sure you want to change the base?
Conversation
Differentiate signature of constructors, functors and normal function calls, thus improving upon PR JuliaLang#25626 and fixing JuliaLang#44889.
It looks like Julia fails to bootstrap itself with these changes:
I'd be happy to review this, but would you mind elaborating what exactly the intended change here is? The As far as I understand, the main issues related to the functors are:
For (1), as far as I can tell though, determining the binding is not the job of the For (2), I presume you are storing a |
Thank you very much for reviewing ♥ It seems like I have to fix that bootstrapping error first, so I convert this to a draft for the meantime.
Oh, I happily elaborate on this (as long as anyone wants to read it :D ). That After spending some time to make the changes in this PR, I very much wished for a way to interact with Julia’s internals to precisely find out, what Julia really defines instead of all this mimicry parsing. This would naturally remove all the bugs and complexity stemming from this mimicry (while hopefully introducing less bugs and less complexity). Do you think this could be viable and anyone could do this? Or anyone could tell me, how to do this?
That is my understanding, too.
Sorry, I forgot to mention this (as it was a rather minor change in julia> Docs.namify(:(function (f::Parametric{T})(x) where {T} end))
:Parametric as you wished for.
Mainly, because I first did not find any way, how using a
Of course, we could also add some other type to the Afterthought: There seems to be a solution exploiting, that
We could actually use, say option 1 for functors and option 2 for constructors. Do you think, this would be better? If you would prefer this approach, I could change my code rather easily. Remaining bugsJulia itself takes semantic information into account, when defining a function. f() = 0
const g = f
"""
f(x)
but documented as `g(x)`.
"""
g(x) = x f() = 0
g = f
"""
f(x)
but documented as `g(x)`.
"""
Main.g(x) = x
g = "My documentation lies!" """
Int8(x)
but documented as `Union{Int8}(x)`.
"""
Union{Int8}(x) = x struct A end
const f = A()
"""
(::A)(x)
but documented as `f(x)`.
"""
f(x) = x Does that elaboration help? Btw. I actually removed my rather elaborative comments in the code, hoping that the code could finally speak for itself. Maybe I shall add short comments again. Shall I? |
The new tests relied upon PR JuliaLang/julia#45367.
That's terrific, thank you for writing this all up!
Ah, cool. If this is orthogonal to the
Hmm.. do you basically mean that, instead of passing the AST to I guess one issue is that, for example, a function definition returns the general I have no idea how doable something like this would be though, and have to defer to somebody else for this.
I think one can argue whether the alias behaviour is a bug or not. It might be useful if you're defining a custom binding for a specialization (e.g. const MyPackageSpecializationType = ParentType{Foo} You probably want to attach docs to this custom name, rather than
I think comments are always good! |
As for the It didn't occur to me that you can have the same argument signature for constructors with different type parameters (e.g. With that in mind, I think having this special One last thing I am slightly concerned about here is breaking packages / code that are already diving into the docsystem metadata (like Documenter; although I am more than happy to just patch Documenter right away, with version guards if need be). But I wonder how much others rely on these internal details, like maybe even Revise etc. |
We now need a where clause when documenting `Vector{T}(...) where T` or accessing its documentation.
Thanks for your replies. I have just now taken the time to look into that bootstrapping error and will come back to your replies tomorrow. The bootstrapping error arose from
without specifying Symmetrically, if we want to get the documentation for As it seems, some tests that want to document |
I have put that in #45529.
Yes, probably. So I would also just keep the behavior as it is.
I will see to that.
That could indeed be problematic, but Documenter only relies on the ability to compare signatures via Regarding Revise, I have seen, that it computes signatures using lowered code. This seems to be kind of the solution I was hoping for, but it depends on LoweredCodeUtils.jl and JuliaInterpreter.jl. So perhaps, we do not want that overkill? Still, I like the way Revise signatures look like way more than how they look like here. Revise’s signatures result in a much cleaner interface without the extra types I introduced. The cost would be to change all normal function signatures (prepending them with As the failing tests reflect, it could potentially be wise to add better error messages or some fallback behavior in Documenter.jl. As for now, interpolating a docstring with missing type parameters or where clauses results in error. |
Differentiate signature of constructors, functors and normal function calls, thus improving upon PR #25626 and fixing #44889. Also allows fixing JuliaDocs/Documenter.jl#558.