Skip to content
This repository has been archived by the owner on May 2, 2020. It is now read-only.

Change to using typed docstrings. #35

Merged
merged 3 commits into from
Sep 19, 2014

Conversation

MichaelHatherly
Copy link
Owner

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.61%) when pulling b1ee8c1 on mh/typed-docs-and-interpolation into dea11d3 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.53%) when pulling e15ef8d on mh/typed-docs-and-interpolation into dea11d3 on master.

MichaelHatherly added a commit that referenced this pull request Sep 19, 2014
…lation

Change to using typed docstrings.
@MichaelHatherly MichaelHatherly merged commit e259c3f into master Sep 19, 2014
@MichaelHatherly MichaelHatherly deleted the mh/typed-docs-and-interpolation branch September 19, 2014 19:07
@@ -1,18 +1,29 @@
@docref () -> REF_ENTRY
type Entry{category} # category::Symbol
docs::String
docs::Docstring

Choose a reason for hiding this comment

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

Why not docs::Any?

Why does the documentation have to be a string at all, for that matter?

It seems unnecessarily limiting to have this be restricted to subtypes of a particular type.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, initially I figured that having an untyped field would be a bad thing, but I guess that Docstring is just about the same except with that added restriction.

What kind of documentation are you thinking won't be a string?

Choose a reason for hiding this comment

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

An image? An audio note? An MS word file? ;-) Of course, everything in Julia has a text/plain representation, but that's not the same as saying it is a string.

More importantly, using Docstring forces any documentation to descend from a single abstract class, and forces it to depend on Docile. For example, it prevents you from using an ASCIIString for documentation, which seems odd! Also, it inverts the dependency graph: it means that a Markdown package would have to depend on Docile, rather than the other way around, in order to define a Markdown type that is usable in documentation.

Copy link

Choose a reason for hiding this comment

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

An URL, filename (or other identifier) might also be a possible option. If someone wants to create a IDE where you can have rich WYSIWYG editor with images/graphs/audio in a method documentation, it would be nice to have it accessible from Julia help() as well. An external identifier will drastically reduce the size of the source file. A base64 encoded image flows very badly if you have line wrapping in your text editor.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes! I realised this was what you were meaning this afternoon. I guess that what was causing me trouble was the string in Docstring since these really don't need to be typical docstrings at all.

Using Anywould then allow for formats that haven't even been thought of yet (20 years from now, or something). I wholeheartedly agree with this choice.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@ivarne there's the :file keyword in the metadata section that would be suitable for this I would think.

@doc [ :file => "external_file.ext" ] ->
f(x) = x

Something like writemime but for reading from external sources, perhaps readmime, as a general mechanism to load file contents as the correct Julia type would be nice. I seem to remember an issue related to this in the Julia repo, but can't seem to locate it now.

@MichaelHatherly
Copy link
Owner Author

I picked MarkdownDocstring rather than Markdown since it avoided conflicting with Markdown from the package Markdown.jl. Putting it in it's own package would solve that though. Would it still just be a wrapper for a String or the full parsed AST?

@stevengj
Copy link

I don't want to conflict with Markdown.jl. I want to use Markdown.jl. Why should we have two Markdown implementations? I'm sure @one-more-minute would be willing to make any reasonable modifications to Markdown.jl that would be needed for Docile.

I think it should just wrap a string, and only compute the parsed AST lazily, when (if) it is needed (although it should probably cache the AST once it is computed).

@MichaelHatherly
Copy link
Owner Author

Using Markdown.jl was what I was wanting, though it's not really what came across in my last comment I see. I've not noticed anything that would need modification on that side on Markdown's side of things.

Would this lazy wrapper live in Docile or Markdown? Caching would be ideal.

What would be nice is loading a package (Markdown.jl in this case) lazily when it's actually needed to display things. I think that's probably possible right now, but I haven't had a look. Something like

function writemime(io::IO, ::MIME"text/plain", md::Markdown)
    isdefined(Docile, :Markdown) || include("markdown_loading_code.jl")
    # ...
end

I have a feeling that JuMP.jl might do some lazy loading of solvers.

@stevengj
Copy link

My thought is that this would live in Markdown.jl, which would have something like:

type Markdown{T<:String} <: String
     s::T
     ast::AST
     Markdown(s::T) = new(s)
end

so that md"foo *bar* would produce a Markdown object where the ast field is undefined. Any function that actually needed the AST (e.g. HTML output) would check isdefined(m::Markdown, :ast), and if false would call the parsing function and store the result in m.ast.

@MichaelHatherly
Copy link
Owner Author

That's looks good to me. Since Markdown.jl's module is called Markdown is it actually possible to use the same name for a type? I've never managed to get that working.

@MikeInnes
Copy link

Modules hold a reference to themselves, i.e. Markdown.Markdown, so it's not possible to use that name for the type, unfortunately.

That said, I'm not sure whether the lazy-loading thing really gains us much. To support things like interpolation we'd have to parse the AST regardless of whether the frontend supports markdown directly, so it's not like we can really save work there.

I think a much better solution would be to cache the parsed Markdown AST persistently. Metadata like file and line number will be stored as part of code caching anyway, so it won't be a huge stretch to cache documentation data along with the code.

@MichaelHatherly
Copy link
Owner Author

Having tried out both ways of storing the docs, I'm not really bothered about which we settle on.

Storing strings has the advantage of being a bit quicker when loading a package, but that shouldn't matter once precompiling happens. It's also simpler to do searches for text in the docstrings when it's just strings, but these aren't terribly compelling reasons to add complexity.

@MichaelHatherly
Copy link
Owner Author

One point in favour of keeping what @doc does to a minimum is that any code that's used by @doc can't then be documented using it, or at least I've not found a way to do that yet.

@stevengj
Copy link

Oh, right, probably not. Probably has to be MarkdownString after all. Not a big deal since most people will use md"...".

@MichaelHatherly
Copy link
Owner Author

Using the constructor directly should, as you point out, be used very rarely so a longer more meaningful name won't impact users that much. Currently it's MarkdownDocstring, but it seems like it may be useful for things other than documentation. I'll change it to MarkdownString.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants