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

Update HTML docs #217

Merged
merged 3 commits into from
Aug 21, 2016
Merged

Update HTML docs #217

merged 3 commits into from
Aug 21, 2016

Conversation

mortenpi
Copy link
Member

Fix some mistakes and add a few details to the "Output formats" section. The style and code block changes are required so that the updated docs would render correctly.

The rendered version, including the style of <h1> in admonitions vs a top-level <h2>:

screenshot from 2016-08-20 09-58-31

@codecov-io
Copy link

codecov-io commented Aug 20, 2016

Current coverage is 82.49% (diff: 100%)

Merging #217 into master will decrease coverage by 2.23%

@@             master       #217   diff @@
==========================================
  Files            20         20          
  Lines          1474       1474          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           1249       1216    -33   
- Misses          225        258    +33   
  Partials          0          0          

Powered by Codecov. Last update 2126ea3...896afb5

@@ -667,6 +669,7 @@ if isdefined(Base.Markdown, :Admonition)
div[".admonition-text"](mdconvert(a.content, a))
)
end
mdconvert(c::Markdown.Code, parent::Markdown.Admonition) = mdconvert_codeblock(c, parent)
end
Copy link
Member

Choose a reason for hiding this comment

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

The other toplevel blocks should also have this change, not just Admonitions and MDs, correct? BlockQuote, Footnote, and List elements as well?

@MichaelHatherly
Copy link
Member

CSS changes look good to me.

@MichaelHatherly
Copy link
Member

Should be backported to 0.3.1?

@mortenpi
Copy link
Member Author

The other toplevel blocks should also have this change, not just Admonitions and MDs, correct? BlockQuote, Footnote, and List elements as well?

Yes, indeed, forgot those. Footnote is a bit tricky though.. I am wrapping it in a <span>, i.e. I am assuming that footnotes should only have inline content. Perhaps we should make that change in a separate PR.

Also, LaTeX has a context-dependent dispatch for the same thing as well. I decided to go with a Union type to dispatch on, instead of writing a separate signature for each block node.

I did notice that parsing lists can run into issues if the whitespace is inconsistent, i.e. in this

- A list

   ```julia
   associated
   ```

  ```math
  \sin(x)
  ```

 - ```math
   \sin(x)
   ```

some of it gets wrapped in paragraphs, the math block doesn't get parsed etc. Not sure if they're bugs though actually -- with consistent whitespace everything seems to work.

Should be backported to 0.3.1?

Probably, I suppose. I guess we should do bugfixes for the HTML part on 0.3 branch as well, since (hopefully) some people will switch their docs over.

@MichaelHatherly
Copy link
Member

I am assuming that footnotes should only have inline content

Footnote should be able to contain any nested block content, just like Admonitions.

Not sure if they're bugs though actually -- with consistent whitespace everything seems to work.

Technically the commonmark "spec" is quite lenient with indentation, but I've gone with requiring consistent indentation so that all docs will be just a little bit more uniform (which I think is a good thing).

Probably, I suppose. I guess we should do bugfixes for the HTML part on 0.3 branch as well, since (hopefully) some people will switch their docs over.

Yes, wherever we can backport non-invasive things we probably should. It can wait 'til this is merged to master though.

@mortenpi
Copy link
Member Author

I am assuming that footnotes should only have inline content

Footnote should be able to contain any nested block content, just like Admonitions.

Yup, I agree that we should change that. I'd do that in a separate PR though, since it also requires changes to the rendering code.

Shall we merge?

Markdown.Code and Markdown.LaTeX nodes should also be rendered as
blocks in other contexts besides Markdown.MD too. Specifically, instead
of only checking for Markdown.MD, we'll dispatch on a Union of all the
block-context nodes when we want to determine block vs. inline based on
the parent node.
In docstrings and admonitions the headers should be smaller than in
the top level context.
@mortenpi mortenpi merged commit 793b68c into master Aug 21, 2016
@MichaelHatherly MichaelHatherly deleted the mp/fix-html-docs branch August 21, 2016 07:55
mortenpi added a commit that referenced this pull request Aug 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants