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

Upstream to Base? #92

Open
tk3369 opened this issue Jun 30, 2020 · 4 comments
Open

Upstream to Base? #92

tk3369 opened this issue Jun 30, 2020 · 4 comments

Comments

@tk3369
Copy link

tk3369 commented Jun 30, 2020

The manual says:

This package provides a collection of useful extensions for Julia's built-in docsystem. These are features that are still regarded as "experimental" and not yet mature enough to be considered for inclusion in Base, or that have sufficiently niche use cases that including them with the default Julia installation is not seen as valuable enough at this time.

What would be needed to be considered mature enough for upstreaming this package to Base?

Ref: julia-vscode/julia-vscode#1427

@MichaelHatherly
Copy link
Member

Judging by the number of direct dependents (131) I'd say we're still in the realm of niche usage with the package as it stands considering there's 3824 packages registered in General. Regarding maturity of the code; I've not touched it in a number of years, though there's been a few additions and bug fixes since then that @mortenpi has shepherded along. Doesn't appear to have many long-standing serious bugs, so in that regard it's doing alright. I'd want additional input from other core devs before we considered upstreaming anything.

@tpapp
Copy link
Contributor

tpapp commented Jun 30, 2020

I am not a core dev, but I am not sure that the issue linked above is a sufficiently strong reason to even consider moving something into Base. Technically, there is no reason VS Code just couldn't support DocStringExtensions as a package.

It is my impression that the tendency is to move things out of Base, and be very conservative with standard libraries. A couple of reasons for this are

  1. extra maintenance burden; a serious bug in Base or the std libs could block a release,
  2. less flexibility (bugfixes and new features have to wait for a release),
  3. a more conservative approach to new features and refactoring, as the API is technically frozen for breaking changes until next major release

@dgleich
Copy link

dgleich commented Jun 2, 2021

As a note on the discussion above. It goes both ways too. I might use these extensions if they were in Base, but perhaps wouldn't bother if they aren't in Base.

Also, Base does seem to have some of this functionality available untemplated through their docview anyway. (e.g. in Base will output the following documentation automatically now...)

julia> mutable struct MyTypeSD
         x::Int 
       end

help?> MyTypeSD
search: MyTypeSD

  No documentation found.

  Summary
  ≡≡≡≡≡≡≡≡≡

  mutable struct MyTypeSD <: Any

  Fields
  ≡≡≡≡≡≡≡≡

  x :: Int64

If you want a poor quick imitation for your own documentation without any dependencies, the following suffices... (based on https://github.com/JuliaLang/julia/blob/master/stdlib/REPL/src/docview.jl where much of the automated code above lives...)

function _field_print(T::Type)
  pad = maximum(length(string(f)) for f in fieldnames(T))
  return string("# Fields\n",
  "```\n",
    map( (Z) -> string(rpad(Z[1], pad), " :: ", Z[2]), zip(fieldnames(T), T.types) )..., 
  "```\n")
end


""" With documentation 

$(_field_print(MyTypeSD))
"""
mutable struct MyTypeSD
  x::Int 
end


help?> MyTypeSD
search: MyTypeSD

  With documentation 

  Fields
  ≡≡≡≡≡≡≡≡

  x :: Int64

(Note, it's possible there's additional discussion relevant somewhere out there in the Juila ecosystem, will update if I find anything.)

@MichaelHatherly
Copy link
Member

Thanks for your input @dgleich. The points made earlier in favour of this remaining out of Base still, as far as I'm concerned, outweigh any possible benefits of inclusion as a stdlib.

I might use these extensions if they were in Base, but perhaps wouldn't bother if they aren't in Base.

Just to make this clear, the dependencies of the package are about as light as you can get. It only depends on stdlibs and there has been, to my knowledge, zero effort spent on improving it's latency so load times could likely be improve with some amount of careful effort if someone felt the need. (If that's what the concern is with regard to including this as a dep?)

Also, Base does seem to have some of this functionality available untemplated through their docview anyway.

Yeah, those were added when we initially implemented docstrings as a kind of "lite" version of some of the features here. They aren't quite as useful though.

Personally, I don't feel there are enough benefits to pushing this into the stdlib, and I'm not going to be pursuing it myself. The vscode issue that kicked this one off is, I feel, a bit of a red herring. Regardless of whether this is in Base or not, you still have to evaluate code that can do arbitrary things to render the docstrings.

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

No branches or pull requests

4 participants