FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: allow <code> in header elements by Trott · Pull Request #31086 · nodejs/node · GitHub

/ node Public

doc: allow <code> in header elements - #31086

Merged
Trott merged 44 commits into
nodejs:masterfrom
Trott:header-code
Dec 27, 2019
ConversationCommits (44)ChecksFiles changed
Merged

doc: allow <code> in header elements#31086
Trott merged 44 commits into
nodejs:masterfrom
Trott:header-code

Conversation

Trott commented Dec 25, 2019

Copy link
Copy Markdown
Member

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

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Dec 25, 2019

Trott commented Dec 25, 2019

Copy link
Copy Markdown
Member Author

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 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 25, 2019
marswong reviewed Dec 25, 2019
View reviewed changes

marswong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Comment thread doc/api/zlib.md Outdated

marswong Dec 25, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Class: Options might be more harmonious than Class: Options

Comment thread doc/api/v8.md Outdated
Comment thread doc/api/globals.md Outdated
Trott force-pushed the header-code branch 2 times, most recently from 0bf5c62 to 8e3fd7c Compare December 25, 2019 05:39

Trott commented Dec 25, 2019

Copy link
Copy Markdown
Member Author

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.

nschonni approved these changes Dec 26, 2019
View reviewed changes

nschonni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

BridgeAR approved these changes Dec 27, 2019
View reviewed changes

BridgeAR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM

Trott added 19 commits December 26, 2019 21:31
doc: allow <code> in header elements
fdec6bd 
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: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,dns: use code markup/markdown in headers
5998523 
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: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc: use code markup/markdown in headers in addons documentation
d0f9585 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,assert: use code markup/markdown in headers
b5834a4 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,async_hooks: use code markup/markdown in headers
a5eb139 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,buffer: use code markup/markdown in headers
5847a4d 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,child_process: use code markup/markdown in headers
19419a4 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,cluster: use code markup/markdown in headers
4459988 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,console: use code markup/markdown in headers
adc9fa2 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,crypto: use code markup/markdown in headers
f6b6701 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc: use code markup/markdown in headers in deprecations documentation
8e7d61e 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,dgram: use code markup/markdown in headers
0c8ff7c 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,domain: use code markup/markdown in headers
75ab328 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,errors: use code markup/markdown in headers
57f7537 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,esm: use code markup/markdown in headers
b3ff048 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,events: use code markup/markdown in headers
36edbde 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,fs: use code markup/markdown in headers
b407503 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc: use code markup/markdown in headers in globals documentation
6f5e765 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
doc,http: use code markup/markdown in headers
f8795db 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott deleted the header-code branch December 27, 2019 05:40
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc: allow <code> in header elements
0054636 
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: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,dns: use code markup/markdown in headers
424266a 
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: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc: use code markup/markdown in headers in addons documentation
ef4ecf4 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,assert: use code markup/markdown in headers
a933028 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,async_hooks: use code markup/markdown in headers
a2bea80 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,buffer: use code markup/markdown in headers
b4934ef 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,child_process: use code markup/markdown in headers
3e07682 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,cluster: use code markup/markdown in headers
700e28d 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,console: use code markup/markdown in headers
d92200a 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,crypto: use code markup/markdown in headers
af66dba 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc: use code markup/markdown in headers in deprecations documentation
e00e4d2 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,dgram: use code markup/markdown in headers
17efccd 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,domain: use code markup/markdown in headers
4eb5e60 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,errors: use code markup/markdown in headers
e74fa1e 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,esm: use code markup/markdown in headers
c763c70 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,events: use code markup/markdown in headers
b1f2dd6 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott added a commit to Trott/io.js that referenced this pull request Dec 27, 2019
doc,fs: use code markup/markdown in headers
e91846c 
PR-URL: nodejs#31086
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott mentioned this pull request Dec 27, 2019
Closed
3 tasks

Trott commented Dec 27, 2019

Copy link
Copy Markdown
Member Author

12.x backport in #31108

Trott mentioned this pull request Dec 27, 2019
Closed
3 tasks

Trott commented Dec 27, 2019

Copy link
Copy Markdown
Member Author

10.x backport in #31109

This was referenced Jan 1, 2020
Closed
Merged
BethGriggs mentioned this pull request Jan 7, 2020
Merged

Me1000 commented Jan 9, 2020 •
edited
Loading

Copy link
Copy Markdown

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

Me1000 mentioned this pull request Jan 9, 2020
Closed
targos mentioned this pull request Jan 15, 2020
Closed
richardlau mentioned this pull request Jan 24, 2020
Closed
4 tasks
MylesBorins mentioned this pull request Feb 8, 2020
Merged
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes
BridgeAR BridgeAR approved these changes
+2 more reviewers
marswong marswong left review comments
nschonni nschonni approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL