-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Docs improvements: content in Markdown, organization into subtemplates, fixed tests #4401
Conversation
…int in separate .css files and their separate HTTP requests
…ve-docs # Conflicts: # documentation/index.html
…quickly that there’s no need to pre-render them and save the intermediate .js files
…at we can add in logic to include different files for v1 versus v2
…apse the parts of `releaseHeader` into one compact function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this. Very much. Very well done!
</b> | ||
""" | ||
htmlFor = -> | ||
marked = require 'marked' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be added to package.json,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Are you okay with me merging this into develop whenever I'm done testing it? I'm working on getting the 2 docs built in their branch, based on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Master, I mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, go for it!
|
||
All together now: | ||
|
||
<table class="definitions"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use CommonMark, then we could possibly write the tables in a more readable way. http://spec.commonmark.org/0.27/#html-blocks
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that supported by the marked
module? Its docs say to use this syntax: https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet#tables
I just haven't gotten around to converting, the automated HTML-to-Markdown converters I tried didn't support tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I didn't realize you used an autmated converter.
- marked does not implement the CommonMark spec. Use CommonMark instead of loosely defined Markdown flavour markedjs/marked#563
- Yes, there are "unofficial" markdown extensions that allow you to type tables as ASCII tables and have that converted to HTML. That would be nice, too, but what I was talking about is that CommonMark supports having entire block of HTML without having to have all those blank lines and missing indentation: http://spec.commonmark.org/0.27/#example-115
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that essentially treating the table as one big passthrough? But we still want markdown within the cells to be evaluated.
I would think the github style would be more idiomatic markdown than any form of HTML tags. It'll just take manual effort to convert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There’s also the issue of rendering a headerless table in Markdown: markedjs/marked#484
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we only have two tables in total currently, so the way it is currently is not such a big deal. But if it could be made nicer in any way, that'd be great! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that can be a future improvement. You're welcome to try 😄
This reverts commit ec9e559.
d2b1ee7
to
16dda9c
Compare
16dda9c
to
4aa0130
Compare
…auto-generated and conflicting ids
Very, very nice! |
Then it will cost less time to translate into Chinese. Great. |
… docs can have different example code blocks/editors
59001f2
to
85b3be9
Compare
…keywords and triple backticks (docs output is unchanged)
More improvements to the documentation:
index.html
now requires sub-templates, to divide content into relevant sub-files. The build process reassembles everything. The sub-files are mostly inside adocumentation/v1
folder, giving the 2 docs source files a logical place to go.index.html
andtest.html
are now built via separatecake
tasks, to avoid a gigantic function.test.html
now run all the tests, includingerror_messages.coffee
, thanks to a fixed polyfill for@throw
.