tools: update marked dependency #6396

firedfox · 2016-04-26T13:11:33Z

Checklist

tests and code linting passes
the commit message follows commit guidelines

Affected core subsystem(s)

tools

Description of change

Update module tools/doc/node_modules/marked. Customize renderer to remove id from heading.

Diff result for all.html http://106.187.89.52/tmp/diff-683f2a7-doc-html-all.html
Diff result for all.json http://106.187.89.52/tmp/diff-683f2a7-doc-json-all.html

The newly updated marked module fixed several problems in the doc.

Removed unnecessary `</p><p>`

In addons.md:

Please see the examples below for further information or
<https://github.com/arturadib/node-qt> for an example in production.

HTML result from:

<p>Please see the examples below for further information or
</p>
<p><a href="https://github.com/arturadib/node-qt">https://github.com/arturadib/node-qt</a> for an example in production.

</p>

to:

<p>Please see the examples below for further information or
<a href="https://github.com/arturadib/node-qt">https://github.com/arturadib/node-qt</a> for an example in production.</p>

Fixed unordered list

In util.md:

`util.inspect.styles` is a map assigning each style a color
from `util.inspect.colors`.
Highlighted styles and their default values are:
 * `number` (yellow)
 * `boolean` (yellow)
 * `string` (green)
 * `date` (magenta)
 * `regexp` (red)
 * `null` (bold)
 * `undefined` (grey)
 * `special` - only function at this time (cyan)
 * `name` (intentionally no styling)

HTML result from:

<p><code>util.inspect.styles</code> is a map assigning each style a color
from <code>util.inspect.colors</code>.
Highlighted styles and their default values are:
 <em> <code>number</code> (yellow)
 </em> <code>boolean</code> (yellow)
 <em> <code>string</code> (green)
 </em> <code>date</code> (magenta)
 <em> <code>regexp</code> (red)
 </em> <code>null</code> (bold)
 <em> <code>undefined</code> (grey)
 </em> <code>special</code> - only function at this time (cyan)
 * <code>name</code> (intentionally no styling)

</p>

to:

<p><code>util.inspect.styles</code> is a map assigning each style a color
from <code>util.inspect.colors</code>.
Highlighted styles and their default values are:</p>
<ul>
<li><code>number</code> (yellow)</li>
<li><code>boolean</code> (yellow)</li>
<li><code>string</code> (green)</li>
<li><code>date</code> (magenta)</li>
<li><code>regexp</code> (red)</li>
<li><code>null</code> (bold)</li>
<li><code>undefined</code> (grey)</li>
<li><code>special</code> - only function at this time (cyan)</li>
<li><code>name</code> (intentionally no styling)</li>
</ul>

Removed redundant new lines

In addons.md:

To test:

HTML result from:

<p>To test:

</p>

to:

<p>To test:</p>

code element class name changes

It changed the class name for code element from cpp / js / javascript to lang-cpp / lang-js / lang-javascript. It does not matter because there is no style defined for these class names.

silverwind · 2016-05-04T17:13:21Z

@nodejs/documentation

addaleax · 2016-05-04T17:25:40Z

tools/doc/html.js

@@ -8,6 +8,15 @@ var typeParser = require('./type-parser.js');

 module.exports = toHTML;

+// customized heading without id attribute
+var renderer = new marked.Renderer();


Wouldn't really fit the style of this file. I guess we shoul to es6ify the doctool later.

Two lines below this, template strings are used :P

Yes it's a problem. I think I'd better change the template string to string replace (edit: or just string add) in order to keep the style consistent.

fwiw, I also used template strings in #6495, I don’t really think that’s a problem. And sorry for the merge conflict, btw, but that should be limited to the package.json.

It doesn't matter. I rebased upstream and resolved the conflict.
I just changed the template to string add before I saw your latest comment :D
I think either is OK. At least the style is consistent now. We can es6ify it later.

silverwind · 2016-05-04T17:35:12Z

Let's see if the doctool tests pass:

https://ci.nodejs.org/job/node-test-pull-request/2500/

firedfox · 2016-05-06T01:35:27Z

There seems to be something wrong with the armv8-ubuntu1404 environment.

silverwind · 2016-05-06T07:15:07Z

Probably nothing to worry about, I bet it'll be green next run. Here's another run:https://ci.nodejs.org/job/node-test-pull-request/2523/

LGTM, by the way.

addaleax · 2016-05-06T17:52:44Z

CI is red, example failure: https://ci.nodejs.org/job/node-test-commit-arm/3159/nodes=armv7-wheezy/tapTestReport/test.tap-1068/

That’s just whitespace changes in the test results, though. It may be nice to do something there like we do for the HTML output tests, i.e. strip whitespaces in both expected and actual output and compare that.

Update module marked. Customize renderer to remove id from heading.

firedfox · 2016-05-07T05:11:59Z

@addaleax It's my bad. I modified the test case to make my local tests pass but forgot to commit it. It's updated now. I just removed extra \ns from the expected data. Those \ns are bugs of the old version marked module. I think they will not change in future. So we don't have to take more effort to strip them from json objects.

addaleax · 2016-05-07T05:21:54Z

LGTM pending CI: https://ci.nodejs.org/job/node-test-commit/3215/

addaleax · 2016-05-07T06:26:21Z

CI is green.

@firedfox the Author: field in your git commit is set to firedfox, but you are listed with your full name in the AUTHORS file. If you want, this can be fixed in when landing the commit?

firedfox · 2016-05-07T07:23:54Z

Oh yes, please change the Author field to my full name. Thank you!

addaleax · 2016-05-07T07:30:21Z

Landed in 3f69ea5. Thanks!

You can set this via git config [--global] user.name '…', btw. :)

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

bnoordhuis · 2016-05-09T12:49:10Z

FYI, this PR looks to be breaking tools/doc/addon-verify.js; as a result, the embedded add-ons in the documentation are no longer built and tested.

I'm going to look into what's causing the breakage but if I can't figure it out or it's too much effort to fix, I'll be moving to revert it.

addaleax · 2016-05-09T13:17:42Z

@bnoordhuis addaleax/node@6e4342f seems to do the trick?

bnoordhuis · 2016-05-09T13:30:47Z

@addaleax Yes, seems to be working.

bnoordhuis · 2016-05-09T13:36:30Z

#6652

bnoordhuis · 2016-05-10T17:03:50Z

Release people: if this gets backported, then please also cherry-pick commit 4f925dd from #6652.

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Update module marked. Customize renderer to remove id from heading. PR-URL: nodejs#6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

mscdex added the tools Issues and PRs related to the tools directory. label Apr 26, 2016

addaleax added the doc Issues and PRs related to the documentations. label Apr 26, 2016

addaleax reviewed May 4, 2016
View reviewed changes

firedfox force-pushed the update-marked-dependency branch 2 times, most recently from 7343c52 to 062bdf2 Compare May 5, 2016 03:13

firedfox force-pushed the update-marked-dependency branch 2 times, most recently from fad0eac to d3ceaa8 Compare May 7, 2016 04:55

tools: update marked dependency

c3208bc

Update module marked. Customize renderer to remove id from heading.

firedfox force-pushed the update-marked-dependency branch from d3ceaa8 to c3208bc Compare May 7, 2016 04:59

addaleax closed this May 7, 2016

addaleax pushed a commit that referenced this pull request May 7, 2016

tools: update marked dependency

3f69ea5

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

bnoordhuis mentioned this pull request May 10, 2016

tools: fix tools/doc/addon-verify.js regression (and more) #6652

Merged

evanlucas pushed a commit that referenced this pull request May 17, 2016

tools: update marked dependency

341eaf2

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

evanlucas mentioned this pull request May 17, 2016

2016-05-17, Version 6.2.0 (Stable) #6810

Merged

MylesBorins added the dont-land-on-v4.x label May 18, 2016

addaleax mentioned this pull request Jul 12, 2016

Bring the doctool up to date on v4.x #7668

Merged

MylesBorins pushed a commit that referenced this pull request Jul 12, 2016

tools: update marked dependency

e7f700b

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins mentioned this pull request Jul 12, 2016

v4.5.0 proposal #7688

Merged

MylesBorins pushed a commit that referenced this pull request Jul 14, 2016

tools: update marked dependency

96922ec

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins pushed a commit that referenced this pull request Jul 14, 2016

tools: update marked dependency

96b5aa8

Update module marked. Customize renderer to remove id from heading. PR-URL: #6396 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: update marked dependency #6396

tools: update marked dependency #6396

firedfox commented Apr 26, 2016

silverwind commented May 4, 2016

addaleax May 4, 2016

silverwind May 4, 2016

thefourtheye May 4, 2016

firedfox May 5, 2016 •

edited

Loading

addaleax May 5, 2016

firedfox May 5, 2016

silverwind commented May 4, 2016

firedfox commented May 6, 2016

silverwind commented May 6, 2016 •

edited

Loading

addaleax commented May 6, 2016

firedfox commented May 7, 2016

addaleax commented May 7, 2016

addaleax commented May 7, 2016

firedfox commented May 7, 2016

addaleax commented May 7, 2016

bnoordhuis commented May 9, 2016

addaleax commented May 9, 2016

bnoordhuis commented May 9, 2016

bnoordhuis commented May 9, 2016

bnoordhuis commented May 10, 2016

tools: update marked dependency #6396

tools: update marked dependency #6396

Conversation

firedfox commented Apr 26, 2016

Checklist

Affected core subsystem(s)

Description of change

Removed unnecessary </p><p>

Fixed unordered list

Removed redundant new lines

code element class name changes

silverwind commented May 4, 2016

addaleax May 4, 2016

Choose a reason for hiding this comment

silverwind May 4, 2016

Choose a reason for hiding this comment

thefourtheye May 4, 2016

Choose a reason for hiding this comment

firedfox May 5, 2016 • edited Loading

Choose a reason for hiding this comment

addaleax May 5, 2016

Choose a reason for hiding this comment

firedfox May 5, 2016

Choose a reason for hiding this comment

silverwind commented May 4, 2016

firedfox commented May 6, 2016

silverwind commented May 6, 2016 • edited Loading

addaleax commented May 6, 2016

firedfox commented May 7, 2016

addaleax commented May 7, 2016

addaleax commented May 7, 2016

firedfox commented May 7, 2016

addaleax commented May 7, 2016

bnoordhuis commented May 9, 2016

addaleax commented May 9, 2016

bnoordhuis commented May 9, 2016

bnoordhuis commented May 9, 2016

bnoordhuis commented May 10, 2016

Removed unnecessary `</p><p>`

firedfox May 5, 2016 •

edited

Loading

silverwind commented May 6, 2016 •

edited

Loading