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

Add at-doc and related code from Docile. #8581

Closed

Conversation

MichaelHatherly
Copy link
Member

This is some initial work to add @doc from Docile. I'm not sure if I've done the include and exports correctly -- it does work fine when calling using Base.Docstrings.

Only the module itself is exported from Base since I didn't feel adding additional exports was necessary.

I've retained the metadata section for each documented object (as per Docile), but I'm aware about the debate regarding whether to include metadata or not.

Hopefully this PR can further the discussion regarding package documentation.

TODO

  • tests
  • documentation

@MichaelHatherly
Copy link
Member Author

Ref: #8514, MichaelHatherly/Docile.jl#40

@ViralBShah ViralBShah added the docs This change adds or pertains to documentation label Oct 5, 2014
oset = :($before = isdefined($qn) ? Set(methods($n)) : Set{Method}())
nset = :(setdiff(Set(methods($n)), $before))

esc(:($oset; $obj; Docstrings.setmeta!(current_module(), $nset, :method, $source, $(data...))))
Copy link
Member

Choose a reason for hiding this comment

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

Rather than escaping the whole thing and then explicitly qualifying, it might be better just to escape specific parts.

@MikeInnes
Copy link
Member

Ok, I'm not sure about this Doc{:md} construct, it's overcomplicating things. The raw doc system shouldn't need to know anything about parsing, just store some object and return it.

It's much more flexible if the string macro used controls any necessary lazy evaluation. This is already implemented for the md"" macro in Markdown.jl.

I also suspect that you're storing the parsed string (with default escapes, interpolation etc.) instead of the raw string, which will also cause issues (no one wants to have to double escape everything). Again, having the doc system trying to control parsing is unnecessary and brittle.

@MikeInnes
Copy link
Member

It's worse than that, actually – having Docstrings in control of parsing actively blocks features of the Markdown parser. I need to be able to do things like adjusting image paths relative to the current file, registering the current module for lazy interpolation etc., which I cannot do as this PR stands. So yeah, simpler & more flexible without.

@MichaelHatherly
Copy link
Member Author

That sounds reasonable to me. It's worked out well in it's current form for Docile so as to not need to import Markdown just to document things. Adding both features to Base does change that though, and your approach makes sense.
I don't have much time for the next few days to do anything major, and I really don't want to be the one to hold this up any longer. Feel free to add the necessary bits to the markdown PR if you'd like.

@MikeInnes
Copy link
Member

Fair enough, thanks. I think the approach you used in Docile makes a lot of sense for the limitations of 0.3, but yeah, best to take a different tack for Base.

Anyway, I'll go ahead and add this tomorrow.

@MichaelHatherly
Copy link
Member Author

Your new Markdown.jl readme page has sold me over. This will be really awesome once it's finished!

@MichaelHatherly MichaelHatherly deleted the mh/docstrings branch October 5, 2014 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants