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.

Sign up for GitHub

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

Jump to bottom

[api-documenter] markdown: fixes newline in table code blocks #1234

Merged
merged 4 commits into from
Apr 17, 2019
Merged

[api-documenter] markdown: fixes newline in table code blocks #1234

Show file tree
Hide file tree
Changes from all commits
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 Apr 15, 2019
52e7bf3
Update changelog entry
skaapgif Apr 15, 2019
d2fcac1
Merge branch 'master' into fix-newline-in-table-code-block
rudolf Apr 16, 2019
7315b90
Merge branch 'master' into fix-newline-in-table-code-block
octogonz Apr 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/api-documenter/src/markdown/MarkdownEmitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export class MarkdownEmitter {
if (context.insideTable) {
const code: string = this.getTableEscapedText(docCodeSpan.code);
const parts: string[] = code.split(/\r?\n/g);
writer.write(parts.join('`<p/>`'));
writer.write(parts.join('</code><br/><code>'));
Copy link
Collaborator

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:

<table><tr><td>
`code`
**bold**
</td></tr></table>

gets rendered as:

`code` **bold**

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.

Copy link
Contributor

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:

  • markdown allows documentation to be viewed in GitHub repositories naturally. You can link to documentation from a README and it will render nicely.
  • markdown is common as an input format for static site generators commonly used for documentation. For example, docusaurus can consume api-documenter output and render html with the same style as other pages it generates. This makes for a much nicer documentation site.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @acchou, useful feedback!

Copy link
Collaborator

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.

Copy link
Contributor

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.

Copy link
Collaborator

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.)

Copy link
Contributor

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.

Copy link
Collaborator

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

Copy link
Contributor

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.

Copy link
Collaborator

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

} else {
writer.write(docCodeSpan.code);
}
Expand Down
45 changes: 45 additions & 0 deletions build-tests/api-documenter-test/etc/api-documenter-test.api.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,51 @@
"releaseTag": "Public",
"name": "IDocInterface4",
"members": [
{
"kind": "PropertySignature",
"canonicalReference": "Context",
"docComment": "/**\n * Test newline rendering when code blocks are used in tables\n */\n",
"excerptTokens": [
{
"kind": "Reference",
"text": "Context"
},
{
"kind": "Content",
"text": ": "
},
{
"kind": "Content",
"text": "({ "
},
{
"kind": "Reference",
"text": "children"
},
{
"kind": "Content",
"text": " }: {\n "
},
{
"kind": "Reference",
"text": "children"
},
{
"kind": "Content",
"text": ": string;\n }) => boolean"
},
{
"kind": "Content",
"text": ";"
}
],
"releaseTag": "Public",
"name": "Context",
"propertyTypeTokenRange": {
"startIndex": 2,
"endIndex": 7
}
},
{
"kind": "PropertySignature",
"canonicalReference": "generic",
Expand Down
3 changes: 3 additions & 0 deletions build-tests/api-documenter-test/etc/api-documenter-test.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ export interface IDocInterface3 {

// @public
export interface IDocInterface4 {
Context: ({ children }: {
children: string;
}) => boolean;
generic: Generic<number>;
numberOrFunction: number | (() => number);
stringOrNumber: string | number;
Expand Down
15 changes: 15 additions & 0 deletions .../api-documenter-test/etc/markdown/api-documenter-test.idocinterface4.context.md
View file
Open in desktop
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) &gt; [api-documenter-test](./api-documenter-test.md) &gt; [IDocInterface4](./api-documenter-test.idocinterface4.md) &gt; [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;
```
1 change: 1 addition & 0 deletions build-tests/api-documenter-test/etc/markdown/api-documenter-test.idocinterface4.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface IDocInterface4

| Property | Type | Description |
| --- | --- | --- |
| [Context](./api-documenter-test.idocinterface4.context.md) | <code>({ children }: {</code><br/><code> children: string;</code><br/><code> }) =&gt; boolean</code> | Test newline rendering when code blocks are used in tables |
| [generic](./api-documenter-test.idocinterface4.generic.md) | <code>Generic&lt;number&gt;</code> | make sure html entities are escaped in tables. |
| [numberOrFunction](./api-documenter-test.idocinterface4.numberorfunction.md) | <code>number &#124; (() =&gt; number)</code> | a union type with a function |
| [stringOrNumber](./api-documenter-test.idocinterface4.stringornumber.md) | <code>string &#124; number</code> | a union type |
Expand Down
19 changes: 19 additions & 0 deletions build-tests/api-documenter-test/etc/yaml/api-documenter-test/idocinterface4.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,28 @@ items:
type: interface
package: api-documenter-test
children:
- api-documenter-test.IDocInterface4.Context
- api-documenter-test.IDocInterface4.generic
- api-documenter-test.IDocInterface4.numberOrFunction
- api-documenter-test.IDocInterface4.stringOrNumber
- uid: api-documenter-test.IDocInterface4.Context
summary: Test newline rendering when code blocks are used in tables
name: Context
fullName: Context
langs:
- typeScript
type: property
syntax:
content: |-
Context: ({ children }: {
children: string;
}) => boolean;
return:
type:
- |-
({ children }: {
children: string;
}) => boolean
- uid: api-documenter-test.IDocInterface4.generic
summary: make sure html entities are escaped in tables.
name: generic
Expand Down
4 changes: 4 additions & 0 deletions build-tests/api-documenter-test/src/DocClass1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ export interface IDocInterface4 {
* make sure html entities are escaped in tables.
*/
generic: Generic<number>;
/**
* Test newline rendering when code blocks are used in tables
*/
Context: ({ children }: { children: string }) => boolean;
}

/**
Expand Down
11 changes: 11 additions & 0 deletions ...n/changes/@microsoft/api-documenter/fix-newline-in-table-code-block_2019-04-15-10-59.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@microsoft/api-documenter",
"comment": "Fix newline rendering when code blocks are used in tables",
"type": "patch"
}
],
"packageName": "@microsoft/api-documenter",
"email": "[email protected]"
}