-
Notifications
You must be signed in to change notification settings - Fork 609
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
[api-documenter] markdown: fixes newline in table code blocks #1234
Merged
octogonz
merged 4 commits into
microsoft:master
from
rudolf:fix-newline-in-table-code-block
Apr 17, 2019
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
2249300
[api-documenter] mardown: fixes newline in table code blocks
skaapgif 52e7bf3
Update changelog entry
skaapgif d2fcac1
Merge branch 'master' into fix-newline-in-table-code-block
rudolf 7315b90
Merge branch 'master' into fix-newline-in-table-code-block
octogonz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
15 changes: 15 additions & 0 deletions
15
.../api-documenter-test/etc/markdown/api-documenter-test.idocinterface4.context.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<!-- Do not edit this file. It is automatically generated by API Documenter. --> | ||
|
||
[Home](./index) > [api-documenter-test](./api-documenter-test.md) > [IDocInterface4](./api-documenter-test.idocinterface4.md) > [Context](./api-documenter-test.idocinterface4.context.md) | ||
|
||
## IDocInterface4.Context property | ||
|
||
Test newline rendering when code blocks are used in tables | ||
|
||
<b>Signature:</b> | ||
|
||
```typescript | ||
Context: ({ children }: { | ||
children: string; | ||
}) => boolean; | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 11 additions & 0 deletions
11
...n/changes/@microsoft/api-documenter/fix-newline-in-table-code-block_2019-04-15-10-59.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"changes": [ | ||
{ | ||
"packageName": "@microsoft/api-documenter", | ||
"comment": "Fixes newline rendering when code blocks are used in tables", | ||
rudolf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"type": "patch" | ||
} | ||
], | ||
"packageName": "@microsoft/api-documenter", | ||
"email": "[email protected]" | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 agree that you've found the best possible solution, but I'll point out that it has the downside that each line of code will have a separate styling rectangle. It looks okay on GitHub, but results may vary with other Markdown renders.
I've never liked the pipes-and-dashes table construct. It has many limitations, and the syntax isn't particularly easy to write. If I remember right, CommonMark chose not to standardize the pipes-and-dashes table after a very long discussion about it. My general philosophy is that while Markdown saves a lot of typing for basic formatting problems, we should defer to using HTML for any complex nesting structures. (This is the philosophy that I'm advocating for TSDoc.)
Unfortunately, as soon as we introduce an HTML table tag, it turns off Markdown entirely:
gets rendered as:
Long term, I'd like to add an
api-documenter html
command line that generates (style-neutral) HTML instead of Markdown. HTML is much easier to generate, whereas Markdown was intended for an interactive human editing experience. Markdown is very difficult for a machine to generate correctly as we see in this PR. The main reason we chose Markdown initially was to support a custom documentation engine that doesn't accept HTML input. This is a fairly common situation, though, so api-documenter should certainly continue to support Markdown forever, alongside any other output types we may add.Another idea I've considered is finding an existing documentation generator with very professional templates, and writing an adapter to drive it using .api.json files. I wouldn't want to bake too much fancy rendering into api-documenter, because its goal is to be complete but simple and readable, so people can use its source code as a guide when writing their own custom solution.
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'm glad to hear that Markdown will be supported going forward. There are still good reasons to support it fully:
If tables turn out to be a continuing source of trouble, maybe there's a way to generate reasonable output without tables. In any case, while I agree that html is better specified and would make it easier to generate, I'd vote for keeping markdown for its interoperability.
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.
Thanks @acchou, useful feedback!
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.
Btw do you have an example of docusaurus output? Maybe we should mention this option in the docs.
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'm working on my site right now, I will post a link here when it's published.
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'm also curious whether you will try to implement a navigation tree for your docs. Markdown by itself doesn't provide any facility for this, but we could easily write the tree into a JSON file similar to what we do for DocFX. In that case you might ask to disable the breadcrumb entirely.)
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.
A navigation tree might be nice, but I don't consider it a priority until it's clear the existing breadcrumbs are insufficient. Plus I plan on adding search to the site so that should alleviate some of the need.
Another way to mitigate the demand for navigation would be to reorganize the output so there aren't so many files generated. It would be nice if every class or module was a single file. There are many pages that just have a short description and little else.
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.
BTW I added your link as an example for the Generating API docs article
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.
Cool, thanks!
Also, to drive home the "Home" link issue, note that that node-core-library documentation's breadcrumb for "Home" is a 404.
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.
@acchou thanks for reporting this!
The missing extension works okay on GitHub pages but I didn't realize that it was broken in the GitHub markdown viewer. Here's a PR to fix it: #1251