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

doc: allow <code> in header elements #31086

Merged
merged 44 commits into from Dec 27, 2019
Merged

doc: allow <code> in header elements #31086

merged 44 commits into from Dec 27, 2019

Conversation

@Trott
Copy link
Member

Trott commented Dec 25, 2019

First commit:

doc: allow <code> in header elements

Allow use of <code> in header elements without styling side effects. We
can add styling later if desired. The goal here is to allow code to be
set off in markdown without needing to escape characters and do lint
exceptions for terms. This is probably a win in terms of accessibility
too although it would be moreso if we had some visual differentiation
for <code> inside of headers. As mentioned above, that can always be
added at a later time.

Second commit:

doc,dns: use code markup/markdown in headers

This will allow us to lint for use of `hostname` in prose without
flagging `hostname` in code within headers. This also allows us to
remove backslash escaping for `[` and `]` inside of header code, which
makes the bare markdown more readable.

Subsequent commits make the change for all the other subsystems.

@nodejs/documentation

@nodejs/website (note the CSS changes)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
@Trott Trott force-pushed the Trott:header-code branch from 9471111 to 2d3d124 Dec 25, 2019
@Trott

This comment has been minimized.

Copy link
Member Author

Trott commented Dec 25, 2019

Probably easiest to review by looking at the first commit in isolation, and then the second commit in isolation. All the other commits follow suit from the second commit.

@Trott Trott added the author ready label Dec 25, 2019
Copy link
Contributor

marswong left a comment

LGTM

@@ -382,7 +382,7 @@ These advanced options are available for controlling decompression:
* Boolean flag enabling “Large Window Brotli” mode (not compatible with the
Brotli format as standardized in [RFC 7932][]).

## Class: Options
## Class: `Options`

This comment has been minimized.

Copy link
@marswong

marswong Dec 25, 2019

Contributor

Class: Options might be more harmonious than Class: Options

doc/api/v8.md Outdated Show resolved Hide resolved
doc/api/globals.md Show resolved Hide resolved
@Trott Trott force-pushed the Trott:header-code branch 2 times, most recently from 0bf5c62 to 8e3fd7c Dec 25, 2019
@Trott

This comment has been minimized.

Copy link
Member Author

Trott commented Dec 25, 2019

Had to make a small change to tools/eslint-rules/documented-errors.js so that it would pass with the ` marks added to errors.md.

Copy link
Member

nschonni left a comment

LGTM, I've seen this over on the MS Docs that they use this approach as well because it is also an indicator that these are chunks that shouldn't be translated

Copy link
Member

BridgeAR left a comment

RSLGTM

Trott added 19 commits Dec 23, 2019
Allow use of <code> in header elements without styling side effects. We
can add styling later if desired. The goal here is to allow code to be
set off in markdown without needing to escape characters and do lint
exceptions for terms. This is probably a win in terms of accessibility
too although it would be moreso if we had some visual differentiation
for <code> inside of headers. As mentioned above, that can always be
added at a later time.

PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This will allow us to lint for use of `hostname` in prose without
flagging `hostname` in code within headers. This also allows us to
remove backslash escaping for `[` and `]` inside of header code, which
makes the bare markdown more readable.

PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Jan 8, 2020
Backport-PR-URL: #31108
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@Me1000

This comment has been minimized.

Copy link

Me1000 commented Jan 9, 2020

Hey there! This is a cool change, but unfortunately it introduced a bug with the JSON docs that get generated.

Specifically the methods field is no longer there:

In 13.5.0:

  source: 'node/doc/api/assert.md',
  modules: [
    {
      textRaw: 'Assert',
      name: 'assert',
      introduced_in: 'v0.1.21',
      stability: 2,
      stabilityText: 'Stable',
      desc: '<p>The <code>assert</code> module provides a set of assertion functions for verifying\n' +
        'invariants. The module provides a recommended <code>strict</code> mode and a more\n' +
        'lenient legacy mode.</p>\n',
      classes: [Array],
      modules: [Array],
      methods: [Array],
      type: 'module',
      displayName: 'Assert'
    }
  ]
}

In 13.6.0 ▶️:

  source: 'node/doc/api/assert.md',
  modules: [
    {
      textRaw: 'Assert',
      name: 'assert',
      introduced_in: 'v0.1.21',
      stability: 2,
      stabilityText: 'Stable',
      desc: '<p>The <code>assert</code> module provides a set of assertion functions for verifying\n' +
        'invariants. The module provides a recommended <code>strict</code> mode and a more\n' +
        'lenient legacy mode.</p>\n',
      classes: [Array],
      modules: [Array],
      type: 'module',
      displayName: 'Assert'
    }
  ]
}

I'm still investigating, but it looks like this regex no longer matched: https://github.com/nodejs/node/blob/master/tools/doc/json.js#L475

targos added a commit that referenced this pull request Jan 14, 2020
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos added a commit that referenced this pull request Jan 14, 2020
This also allows us to remove backslash escaping for `[` and `]`
inside of header code, which makes the bare markdown more readable.

Refs: #31086

PR-URL: #31149
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request Jan 14, 2020
PR-URL: #31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos added a commit that referenced this pull request Jan 14, 2020
This also allows us to remove backslash escaping for `[` and `]`
inside of header code, which makes the bare markdown more readable.

Refs: #31086

PR-URL: #31149
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.