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

Refactor docsystem internals. #15266

Merged

Conversation

MichaelHatherly
Copy link
Member

Refactor docsystem internals

The first commit consists of the following changes:

Structure of a module's #meta dict now has the following layout in all cases (with the exception of keywords):

  • each module's #meta ObjectIdDict has keys of type Binding and values of type MultiDoc.
  • each MultiDoc stores a collection of DocStr objects representing a single docstring each. These are ordered based on their order of definition rather than the current type_morespecific approach.
  • DocStrs store the raw text of a docstring and lazily parse this to a Markdown.MD object when requested. By not parsing every docstring we also make some space savings in the sys.* files.

By keying #meta by Binding in every case the rest of the logic in doc! and doc becomes a lot more straightforward. at-doc expression building has also been simplified.

Additionally, the "summaries" displayed when no documentation can be found have been refactored and simplified.

Rewrite genstdlib.jl

The second commit takes advantage of the simplifications in the previous commit to reduce the complexity in this script. Outwardly the only difference should be the formatting of the summary that is displayed and speed (where timings drop from ~18 to ~9 seconds.)

@MichaelHatherly MichaelHatherly added the docsystem The documentation building system label Feb 27, 2016
println(io, "---\n")
println(io, readstring(readme))
else
println(io, "Additionally, no `README.md` found for module `", m, "`.\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this in addition to?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was in addition to the default no docs message that appears, but it's not really necessary so I've remove the "Additionally".

@tkelman
Copy link
Contributor

tkelman commented Feb 28, 2016

Aside from little nitpicks, this does look like a really nice simplification. The fewer things we have to implicitly import into base to make the docsystem work, the better, and 💯 for lazy parsing of the docstrings.

@MichaelHatherly MichaelHatherly force-pushed the mh/more-docs-refactoring branch from 942a6be to 285a166 Compare February 28, 2016 20:16
@MichaelHatherly
Copy link
Member Author

Thanks for reviewing @tkelman, I've pushed fixes for those comments.

@MichaelHatherly MichaelHatherly force-pushed the mh/more-docs-refactoring branch from 285a166 to 292a34d Compare February 29, 2016 21:15
@MichaelHatherly
Copy link
Member Author

Found and fixed an issue in parsedoc where the generated markdown did not include the same metadata as it used to, namely a :path and :module.

Also added a small third commit that returns some extra metadata from @doc that will be useful when generating external docs. I'm happy to make a separate PR later for this if that would be preferred.


# Modules
function summarise(binding::Binding, sig)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure the Americans will appreciate this name ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, probably not :( Will fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Spellings changed in latest push.

@tkelman
Copy link
Contributor

tkelman commented Mar 1, 2016

CI failures were #15294, restarted

From worker 4:       * numbers              Error During Test
    From worker 4:    Test threw an exception of type UndefVarError
    From worker 4:    Expression: round(T,true // false) === convert(T,Inf)
    From worker 4:    UndefVarError: no_op_err not defined
    From worker 4:     in gcd(::Bool, ::Bool) at ./intfuncs.jl:11
    From worker 4:     in Rational{Bool}(::Bool, ::Bool) at ./rational.jl:9
    From worker 4:     [inlined code] from /tmp/julia/share/julia/test/numbers.jl:2849
    From worker 4:     in anonymous at ./no file:4294967295
    From worker 4:     [inlined code] from ./essentials.jl:79
    From worker 4:     in include_string(::UTF8String, ::ASCIIString) at ./loading.jl:371
    From worker 4:     in include_from_node1(::ASCIIString) at ./loading.jl:420
    From worker 4:     [inlined code] from ./util.jl:179
    From worker 4:     in runtests(::ASCIIString) at /tmp/julia/share/julia/test/testdefs.jl:7
    From worker 4:     in (::Base.Serializer.__deserialized_types__.##9137)(::ASCIIString) at /tmp/julia/share/julia/test/runtests.jl:36
    From worker 4:     in run_work_thunk(::Base.##254#256{Base.CallMsg{:call_fetch}}, ::Bool) at ./multi.jl:714
    From worker 4:     [inlined code] from ./multi.jl:1010
    From worker 4:     in (::Base.##253#255{Base.CallMsg{:call_fetch},TCPSocket})() at ./task.jl:59

Structure of a module's `#meta` dict now has the following layout in
all cases (with the exception of keywords):

- each module's `#meta` `ObjectIdDict` has keys of type `Binding` and
  values of type `MultiDoc`.
- each `MultiDoc` stores a collection of `DocStr` objects representing a
  single docstring each. These are ordered based on their order of
  definition rather than the current `type_morespecific` approach.
- `DocStr`s store the raw text of a docstring and lazily parse this to a
  `Markdown.MD` object when requested. By not parsing every docstring we
  also make some space savings in the `sys.*` files.

By keying `#meta` by `Binding` in every case the rest of the logic in
`doc!` and `doc` becomes a lot more straightforward. `at-doc` expression
building has also been simplified.

Additionally, the "summaries" displayed when no documentation can be
found have been refactored and simplified.
This takes advantage of the simplifications in the previous commit to reduce
the complexity in this script. Outwardly the only difference should be
the formatting of the summary that is displayed and speed (where timings drop
from ~18 to ~9 seconds.)
This commit saves some of the values computed by `doc` and
`summarise` into the generated `MD` object that is returned.
These valus are the `Binding` and signature searched for, as
well as the vector of `DocStr` objects that match the `Binding`
and signature.
@MichaelHatherly MichaelHatherly force-pushed the mh/more-docs-refactoring branch from 292a34d to 1d851b2 Compare March 1, 2016 07:23
@MichaelHatherly
Copy link
Member Author

Barring further comments, I'll merge this later on today.

MichaelHatherly added a commit that referenced this pull request Mar 2, 2016
@MichaelHatherly MichaelHatherly merged commit 6f98b71 into JuliaLang:master Mar 2, 2016
@MichaelHatherly MichaelHatherly deleted the mh/more-docs-refactoring branch March 2, 2016 14:21
MichaelHatherly added a commit to MichaelHatherly/julia that referenced this pull request Mar 9, 2016
Custom docstring support was lost in JuliaLang#15266. This remedies the
regressions introduced there as well as fixing `Text` and `HTML`
display signatures that were also broken, likely when `Function`
became an abstract type, and had gone unnoticed.

Also adds a test case based on PyPlot to hopefully avoid future regressions.

Fixes JuliaLang#15424.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docsystem The documentation building system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants