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

Discussion(doc,tools): Include file source text into Markdown files #32916

Closed
DerekNonGeneric opened this issue Apr 18, 2020 · 16 comments
Closed
Labels
discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.

Comments

@DerekNonGeneric
Copy link
Contributor

DerekNonGeneric commented Apr 18, 2020

Is your feature request related to a problem? Please describe.

Problem space: There does not appear to be a way to embed the source text of a local file into the Markdown files of the API docs.

Context: I'm trying to include a JS code sample in API docs Markdown that would exist in a different file. It would be preferable if it could pass a few validation & lint-style checks prior to being embedded and disseminated. I mentioned the desire for this in a previous comment, which follows.

Ideally these samples would live in their own files and be subject to typechecking and linting prior to being embedded into the docs via a templating engine otherwise you inevitably end up providing bad samples to users, which isn't uncommon.

After further thought and a new PR, re-implementation of this test fixture (presently inert & obsoleted), re-purposing it, and re-integrating it as a code sample supporting the prose of a new loader example seems most logical. However, as @devsnek pointed out, code samples intended for docs shouldn't exist in test/ (with doc/ potentially being more appropriate as an idea fielded on IRC).

Describe the solution you'd like

After a cursory look, @Trott and myself are under the impression that the documentation generator (tools/doc/) is not presently capable of performing this feature and it was suggested that there may be value in opening an issue for discussion.

Therefore, (pointed out to me by @Trott) this is solvable in a number of different ways as is apparent in the following StackOverflow question and may benefit from discussion.

https://stackoverflow.com/questions/4779582/markdown-and-including-multiple-files


Any thoughts/suggestions are welcome and would be highly appreciated! :)

/cc @rubys and @vsemozhetbyt as have been relevant prior contributors to tools/doc/
/cc @wooorm as has been helpful with previous @unifiedjs tooling workflows
/cc @GeoffreyBooth as is likely interested in the discussion and outcome
/cc @nodejs/documentation as would likely be the relevant team

@wooorm
Copy link

wooorm commented Apr 19, 2020

Is there currently a way to add extensions to Node docs?
Generally, there are several ways to extend markdown, such as:

  • a comment: <!--import example.md-->,
  • a code block:
    ```import example.md
    ```
    or:
    ```import
    example.md
    ```
  • MDX/HTML/XML components: <Import src="example.md" />

@Trott
Copy link
Member

Trott commented Apr 19, 2020

Is there currently a way to add extensions to Node docs?

The markdown docs are processed by the code in tools/doc/ via tasks in Makefile. I'm not sure if there is a hook setup, but certainly the code there could be modified to adopt one of these paradigms for extensions.

@Trott
Copy link
Member

Trott commented Apr 19, 2020

One downside to this approach is that it further degrades the experience of browsing the markdown files on GitHub. Not necessarily a deal-breaker, but the upside should outweigh the downside.

@Trott
Copy link
Member

Trott commented Apr 19, 2020

It would be preferable if it could pass a few validation & lint-style checks prior to being embedded and disseminated.

@DerekNonGeneric The code samples in the Markdown files are linted already. For example, in esm.md, if you add a code sample with a missing semi-colon and then run make lint-js, you will get something like this:

/Users/trott/io.js/doc/api/esm.md
  13:16  error  Missing semicolon  semi

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Is there a particular problem/rule that needs to be added to the linting?

You can see the modifications to our base .eslintrc.js in doc/.eslintrc.yaml.

@Trott
Copy link
Member

Trott commented Apr 19, 2020

Just left a comment at #32646 (comment) but probably worth repeating it here in case there is a need for more rigorous checking than linting provides:

For addons.md, we have code that extracts the code samples and makes sure they all compile. Perhaps we could do something similar for esm.md?

@DerekNonGeneric
Copy link
Contributor Author

DerekNonGeneric commented Apr 19, 2020

Wow, we have some good talking points so far. Thanks for looking into this!

/re @Trott's comments

  1. I'm not sure if there is a hook setup, but certainly the code there could be modified to adopt one of these paradigms for extensions.

    I would be +1 to this depending on the answer(s) to my following question.

  2. One obvious downside to this approach is that it further degrades the experience of browsing the markdown files on GitHub.

    Are there other downsides to this? Weighing pros against cons seems helpful.

  3. The code samples in the Markdown files are linted already.

    Yes, I was previously aware of this (it's handled by the eslint-plugin-markdown package).

  4. Is there a particular problem/rule that needs to be added to the linting?

    There are actually several. Ensuring that they compile silently (no warnings or errors) being one of them.

  5. [...] in case there is a need for more rigorous checking than linting provides.

    That may be the case. Ensuring that executable samples (those in the examples section) are doing what they're supposed to sounds appealing to me.

  6. For addons.md, we have code that extracts the code samples and makes sure they all compile. Perhaps we could do something similar for esm.md?

    This is an interesting alternative that I was not aware of—going to look into it now.


Quick recap of options that may solve for my particular use case:

 A. extend Markdown — #32916 (comment)
 B. extend ESLint — #32916 (comment)
 C. use extraction code — #32916 (comment)

Note: option A would be representative of a feature request if this really were one.

@GeoffreyBooth
Copy link
Member

I maintain the CoffeeScript docs, and in those I took the “extend Markdown” approach: a code block that starts with codeFor( is evaluated at build time when converting the markdown to HTML. For example:

One obvious downside to this approach is that it further degrades the experience of browsing the markdown files on GitHub.

To be honest this hadn’t occurred to me for my implementation, but I see why it would be a concern. One thing I’ve noticed is that you can have relative links in Markdown, so a potential solution is to include a relative link to the code file in the Markdown itself, and possibly hide it in the HTML version (or exclude it from the output). Something like:

```import
./sources/esm/import-statements.js
```
[Raw code](./sources/esm/import-statements.js)

Where ./sources/esm/import-statements.js is a valid path relative to the Markdown file that this is written in. The Raw code here would render as a link in GitHub, and clicking it would go to the code file. It’s not as ideal as seeing the code inline, sure, but if they want that they should just go to the Node site 😄

@DerekNonGeneric
Copy link
Contributor Author

There appears to be no simple answer and it may be entirely possible that all three of these options would need to be used in conjunction to accomplish the ideal:

  • inline in the Markdown
  • linted
  • tested
  • verified to compile silently

Some of it is handled via ESLint plugin, so I think for our own sake, we should start there while leaving the rest of the options on the table.

@DerekNonGeneric DerekNonGeneric changed the title Discussion(doc): Include file source text into Markdown files doc,tool,discuss: Include file source text into Markdown files Apr 20, 2020
@DerekNonGeneric

This comment has been minimized.

@DerekNonGeneric DerekNonGeneric changed the title doc,tool,discuss: Include file source text into Markdown files Discussion(doc,tools): Include file source text into Markdown files Apr 20, 2020
@Trott Trott added discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Apr 20, 2020
@Trott
Copy link
Member

Trott commented Apr 20, 2020

@nodejs/linting

@Trott
Copy link
Member

Trott commented Apr 20, 2020

@Trott, would it be acceptable to at least begin by ensuring custom rules are followed when linting code samples in doc/api/esm.md?

I'm not familiar with the jsdoc plugin rules, so I honestly don't know, but I suppose you could try enabling that in doc/.eslintrc.yaml and see how extensive the changes would need to be throughout the docs? And if too extensive, perhaps the relevant rules can be enabled via comments in esm.md so as to only affect that file?

@Trott
Copy link
Member

Trott commented Apr 20, 2020

Also, relevant comment in #32913 (comment). Apparently, the N-API team would find it useful to maintain a particular markdown table outside of the N-API markdown doc itself. (The idea is for that to make it easier to keep the table up to date across release lines, which I would guess has to do with ease of cherry-picking and not having to manually backport stuff due to conflicts, but I'm not actually 100% sure.)

@DerekNonGeneric
Copy link
Contributor Author

DerekNonGeneric commented Apr 20, 2020

Apparently, the N-API team would find it useful to maintain a particular markdown table outside of the N-API markdown doc itself.

I saw that and didn't want to derail this discussion with anything regarding talking point no. 4. I'm planning to open something more focused on that one. ;)

Glad to hear more feedback may be on the way! Git's precedent may help justify their case—for example:

Included file source text: https://github.com/git/git/blob/master/Documentation/trace2-target-values.txt
Markdown (or whatevs) file:
https://github.com/git/git/blob/efe3874640e2e58209ddbb3b072f48f6b7094f34/Documentation/technical/api-trace2.txt#L143
Output: https://git-scm.com/docs/api-trace2#_enabling_a_target

@DerekNonGeneric
Copy link
Contributor Author

DerekNonGeneric commented Apr 24, 2020

This may have been overlooked and clarity on whether the Markdown files in the /doc/api directory of this repo are indeed GitHub Flavored Markdown (GFM), as opposed to CommonMark, seems fundamental to understanding what "extend Markdown" actually means in this context. Here's what I would expect, but can't say for certain whether or not this is correct:

/node/
├── /.github/
│   ├── /workflows/
│   ├── /ISSUE_TEMPLATE/
│   │   ├── 1-bug-report.md                  # GFM
│   │   ├── 2-feature-request.md             # GFM
│   │   └── config.yml
│   └── PULL_REQUEST_TEMPLATE.md             # GFM
├── /doc/api/*.md                            # CommonMark

Specs:
https://github.github.com/gfm/
https://spec.commonmark.org/0.29/

@GeoffreyBooth
Copy link
Member

Here's what I would expect, but can't say for certain whether or not this is correct:

I think what you're describing is as it should be, certainly. If there are places where it's not, that's probably more because someone forgot (and we're not linting carefully enough).

@DerekNonGeneric
Copy link
Contributor Author

Hi Folks,

Thanks for your participation!

We've reached a conclusion, so I'm closing this thread as an indication of that. If further discussion would've been desirable (or anything was left unsaid), feel free to continue the conversation here or check out doc/guides/contributing/issues.md for guidance on opening new issues for discussion. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

No branches or pull requests

4 participants