-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
RFC: Add support for dynamic doc strings #20142
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
Conversation
MichaelHatherly
left a comment
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.
Thanks @malmaud.
base/docs/Docs.jl
Outdated
| var = getfield(binding.mod, binding.var) | ||
| if method_exists(getdoc, (typeof(var),)) | ||
| return Base.Markdown.parse(getdoc(var)) | ||
| end |
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.
Can be a bit shorter/clearer as
function doc(binding::Binding, sig::Type = Union{})
+ if defined(binding)
+ result = getdoc(resolve(binding), sig)
+ result === nothing || return Markdown.parse(result)
+ end
results, groups = DocStr[], MultiDoc[]with a default getdoc(obj, sig) = nothing method. Passing sig through to getdoc as well may be useful?
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.
Good call.
base/docs/Docs.jl
Outdated
| doc(object, sig::Type = Union{}) = doc(aliasof(object, typeof(object)), sig) | ||
|
|
||
| function doc(object, sig::Type = Union{}) | ||
| doc(aliasof(object, typeof(object)), sig) |
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.
Unintentional formatting change?
| Docs.doc(binding, sig) | ||
| Returns all documentation that matches both `binding` and `sig`. | ||
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.
wrap long lines
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.
Done
|
@MichaelHatherly look good now? |
| """ | ||
| function doc(binding::Binding, sig::Type = Union{}) | ||
| if defined(binding) | ||
| result = getdoc(resolve(binding), sig) |
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.
Would it make sense to not call Markdown.parse here and instead just return result? That way getdocs could return a more suitable object, such as an RST object for rst-formatted python docstrings.
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.
Ya, that's a good call.
| """ | ||
| getdoc(x::T, sig) -> String | ||
| Return the dynamic docstring associated with object `x`, or `nothing` to use |
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 docstring doesn't explain what the sig argument is for.
Useful for eg JuliaPy/PyCall.jl#46.
Allows instances of the same type to have different docstrings which can only be determined at runtime. Useful for interlanguage support where the same Julia type (eg
PyObject) corresponds to different types in the guest language that might have different docstrings.