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

Markdown headings support #197

Open
vidartf opened this issue Nov 21, 2019 · 8 comments
Open

Markdown headings support #197

vidartf opened this issue Nov 21, 2019 · 8 comments
Labels
effort: medium Needs a somewhat experienced developer enhancement The issue is asking for a new feature or design change help wanted If you're looking to contribute, this issue is a good place to start! octogonz-parser-backlog Items for @octogonz to consider during the next iteration on the parser

Comments

@vidartf
Copy link

vidartf commented Nov 21, 2019

Support for markdown headings is listed as a goal in the README, but unsupported in #169. In practice (using the tsdoc playground) it seems to be unsupported. In #169, it is also recommended to open a new issue for further discussion on this point if needed.

To start the discussion, I will share this example that currently works well with TypeDoc:

/**
 * Reverse an array in-place.
 *
 * @param array - The mutable array-like object of interest.
 *
 * @param start - The index of the first element in the range to be
 *   reversed, inclusive. The default value is `0`. Negative values
 *   are taken as an offset from the end of the array.
 *
 * @param stop - The index of the last element in the range to be
 *   reversed, inclusive. The default value is `-1`. Negative values
 *   are taken as an offset from the end of the array.
 *
 * #### Complexity
 * Linear.
 *
 * #### Undefined Behavior
 * A `start` or  `stop` index which is non-integral.
 *
 * #### Example
 * ```typescript
 * import { ArrayExt } from '@lumino/algorithm';
 *
 * let data = [0, 1, 2, 3, 4];
 * ArrayExt.reverse(data, 1, 3);  // [0, 3, 2, 1, 4]
 * ArrayExt.reverse(data, 3);     // [0, 3, 2, 4, 1]
 * ArrayExt.reverse(data);        // [1, 4, 2, 3, 0]
 * ```
 */

In this comment, the #### Example heading can and should be replaced with @example. It is not so clear what to do with the #### Complexity and #### Undefined Behavior headings. A logical first step is to put both in a @remarks section, but it is unclear if there is any way to subdivide the remarks section further (without falling back to HTML), other than to allow markdown headers.

@octogonz
Copy link
Collaborator

We should implement support for Markdown headings.

But I think the TSDoc heading should start at level 1 (#) not level 4 (####), because doc comments shouldn't make assumptions about the page structure that will be rendered by the documentation tool.

@octogonz
Copy link
Collaborator

As far as the AST representation for headings, probably they should not be a nesting structure. They can be a regular section member similar to a DocParagraph.

@vidartf
Copy link
Author

vidartf commented Nov 29, 2019

I think the TSDoc heading should start at level 1 (#) not level 4 (####)

Sure, this was just an artifact from optimizing for typedoc's system.

@octogonz octogonz added effort: medium Needs a somewhat experienced developer enhancement The issue is asking for a new feature or design change help wanted If you're looking to contribute, this issue is a good place to start! octogonz-parser-backlog Items for @octogonz to consider during the next iteration on the parser labels Dec 9, 2019
@muuvmuuv
Copy link

Let's start with 2 not 1, since the first line of docs is the headline 1 and in HTML only one h1 is suggested/allowed. One # would be marked as wrong then.

@octogonz
Copy link
Collaborator

octogonz commented Aug 18, 2020

Let's start with 2 not 1, since the first line of docs is the headline 1 and in HTML only one h1 is suggested/allowed. One # would be marked as wrong then.

* https://www.w3.org/wiki/HTML/Usage/Headings/h1only

* https://www.w3.org/QA/Tips/Use_h1_for_Title

I did not know that! Maybe TSDoc should start with ## then, since the title is expected to be provided by the documentation system.

Does anybody else have an issue with this? We could even issue a validation warning for # (or ### without ##).

CC @rbuckton

@vidartf
Copy link
Author

vidartf commented Aug 20, 2020

I did not know that! Maybe TSDoc should start with ## then, since the title is expected to be provided by the documentation system.

and

As far as the AST representation for headings, probably they should not be a nesting structure. They can be a regular section member similar to a DocParagraph.

If there is no nesting, should tsdoc really try to enforce/warn about which levels are used? Can it just not pass through the headers that are given?

@octogonz
Copy link
Collaborator

If there is no nesting, should tsdoc really try to enforce/warn about which levels are used? Can it just not pass through the headers that are given?

There are different scenarios for different tools. The setup that I'm used to is like this:

  1. A developer writes a code comment using VS Code -- an ESLint rule will underline issues caught by the TSDoc plugin
  2. The developer makes a PR -- API Extractor will report analysis errors caught during the CI build.
  3. Some time after the PR is merged, the API documentation website will get regenerated (e.g. when a release is published) -- at this step API Documenter can report errors relating to documentation.

Typically the original developer may never see the final rendered output at all. Even if there is a nice staging server stood up somewhere, the person writing the code may not be interested in that, and would not bother to look at it.

Thus wherever reasonable, it is desirable to catch mistakes as early in the pipeline as possible. Making TSDoc very strict and predictable enables us to catch issues by looking at code comments, without needing to run an entire documentation pipeline to understand how it will get rendered. For people who don't have this requirement, these strict checks can easily be disabled.

@vidartf
Copy link
Author

vidartf commented Aug 21, 2020

Right, as long as it is a warning it is easily ignorable with a rule, good point 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: medium Needs a somewhat experienced developer enhancement The issue is asking for a new feature or design change help wanted If you're looking to contribute, this issue is a good place to start! octogonz-parser-backlog Items for @octogonz to consider during the next iteration on the parser
Projects
None yet
Development

No branches or pull requests

3 participants