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

Move docs for Base.Pkg into source file. #11912

Closed

Conversation

MichaelHatherly
Copy link
Member

A first pass at moving the docs for Pkg into the source file.

I've hit two bugs when trying this:

For Pkg.rm and Pkg.edit I've been getting

ErrorException("error in method definition: function Base.edit must be explicitly imported to be extended")))

when placing the docstrings above the methods, hence the use of function rm end.

I believe there's already an issue open for that, but can't find it at the moment (edit: probably #11798).

Second one:

help?> Pkg.dir
ERROR: KeyError: dir(path...) at pkg.jl:37 not found
 in getindex at dict.jl:695
 in doc at ./docs/Docs.jl:105

but I have confirmed that all the docs are in Base.Pkg.META. Not sure what the trouble is there.

I've just copy-pasted what was in helpdb.jl straight into the docstrings, so formatting could still use some work.

@tkelman tkelman added domain:docs This change adds or pertains to documentation domain:packages Package management and loading labels Jun 28, 2015
@MichaelHatherly
Copy link
Member Author

Segfault on travis seems unrelated. Tests are passing locally.

@MikeInnes MikeInnes self-assigned this Jun 28, 2015
@StefanKarpinski StefanKarpinski added docsystem The documentation building system and removed domain:docs This change adds or pertains to documentation labels Jun 28, 2015
@MichaelHatherly
Copy link
Member Author

Formatting should match up with the previous rst source pretty well now. I've wrapped the docstrings at 92 chars and written the code syntax with double backticks, if that's not the style people would like I'm open to suggestions.

@MichaelHatherly MichaelHatherly changed the title WIP: Move docs for Base.Pkg into source file. Move docs for Base.Pkg into source file. Jun 28, 2015
@MikeInnes
Copy link
Member

This is great to see! About syntax, the docstrings are going to be parsed as Markdown and we (hopefully only temporarily) have an RST output backend, so yeah, we'll want to have single quotes for backticks.

I'll try and track down the errors you've found.

@MichaelHatherly
Copy link
Member Author

we'll want to have single quotes for backticks

Fine by me. Switched out for single quotes, though the CommonMark Spec does allow for any number of consecutive backticks if I'm reading correctly.

@MichaelHatherly MichaelHatherly mentioned this pull request Jun 28, 2015
@MichaelHatherly
Copy link
Member Author

I'll try and track down the errors you've found.

@one-more-minute, it seems to be a dict rehashing problem and needs ObjectIdDictss instead:

$ git diff
diff --git a/base/docs/Docs.jl b/base/docs/Docs.jl
index ce76e31..6c8ffb5 100644
--- a/base/docs/Docs.jl
+++ b/base/docs/Docs.jl
@@ -75,11 +75,11 @@ end
 type FuncDoc
     main
     order::Vector{Method}
-    meta::Dict{Method, Any}
-    source::Dict{Method, Any}
+    meta::ObjectIdDict
+    source::ObjectIdDict
 end

-FuncDoc() = FuncDoc(nothing, [], Dict(), Dict())
+FuncDoc() = FuncDoc(nothing, [], ObjectIdDict(), ObjectIdDict())

 function doc!(f::Function, data)
     fd = get!(meta(), f, FuncDoc())
@@ -127,7 +127,7 @@ catdoc(xs...) = vcat(xs...)
 isdoc(x) = isexpr(x, :string, AbstractString) ||
     (isexpr(x, :macrocall) && endswith(string(x.args[1]), "_str"))

-dict_expr(d) = :(Dict($([:($(Expr(:quote, f)) => $d) for (f, d) in d]...)))
+dict_expr(d) = :(ObjectIdDict($([:($(Expr(:quote, f)) => $d) for (f, d) in d]...)))

 function field_meta (def)
     meta = Dict()
@@ -145,12 +145,12 @@ end

 type TypeDoc
     main
-    fields::Dict{Symbol, Any}
+    fields::ObjectIdDict
     order::Vector{Method}
-    meta::Dict{Method, Any}
+    meta::ObjectIdDict
 end

-TypeDoc() = TypeDoc(nothing, Dict(), [], Dict())
+TypeDoc() = TypeDoc(nothing, ObjectIdDict(), [], ObjectIdDict())

 function doc!(t::DataType, data, fields)
     td = get!(meta(), t, TypeDoc())

Fixes the KeyError:

help?> Pkg.dir
  dir() -> AbstractString
...

@MikeInnes
Copy link
Member

Great! Can you PR that? I think the fields::Dict{Symbol, Any} part can safely stay as is.

@MichaelHatherly
Copy link
Member Author

Will do, yeah I went a bit overboard with it, .fields can remain as is.

@MichaelHatherly
Copy link
Member Author

With #11926 merged I'll close this and revisit later on once those problems have be solved.

@MikeInnes
Copy link
Member

Yeah sorry about that, thanks a lot for the effort though.

@MichaelHatherly
Copy link
Member Author

That's fine, it managed to shake out a bug so not a wasted effort.

@MichaelHatherly MichaelHatherly deleted the mh/move-pkg-docs branch June 29, 2015 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docsystem The documentation building system domain:packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants