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: remove extra space in console.log examples #7885

Closed
wants to merge 5 commits into from

Conversation

joeyespo
Copy link
Contributor

@joeyespo joeyespo commented Jul 27, 2016

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

This fixes the console.log example in the docs. Before this change, an extra space would be printed, which wouldn't match the example output. Now it's consistent.

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. console Issues and PRs related to the console subsystem. labels Jul 27, 2016
@mscdex
Copy link
Contributor

mscdex commented Jul 27, 2016

LGTM

@ChALkeR
Copy link
Member

ChALkeR commented Jul 27, 2016

Could you fix the exact same issue in other doc files, please?
E.g. https://github.com/nodejs/node/blob/master/doc/api/vm.md#vmruninthiscontextcode-options

@joeyespo
Copy link
Contributor Author

@ChALkeR Done! I amended the commit so there's still only one.

@joeyespo joeyespo changed the title doc: remove extra space in console.log example doc: remove extra space in console.log examples Jul 27, 2016
@addaleax
Copy link
Member

LGTM

@ChALkeR
Copy link
Member

ChALkeR commented Jul 27, 2016

@joeyespo Cool, thanks! That was an example, though. Full list:

doc/api/console.md:console.log('count: ', count);
doc/api/https.md:  console.log('statusCode: ', res.statusCode);
doc/api/https.md:  console.log('headers: ', res.headers);
doc/api/https.md:  console.log('statusCode: ', res.statusCode);
doc/api/https.md:  console.log('headers: ', res.headers);
doc/api/process.md:    console.log("Unhandled Rejection at: Promise ", p, " reason: ", reason);
doc/api/vm.md:console.log('vmResult: ', vmResult);
doc/api/vm.md:console.log('localVar: ', localVar);
doc/api/vm.md:console.log('evalResult: ', evalResult);
doc/api/vm.md:console.log('localVar: ', localVar);

Ah, and LGTM either way.

@joeyespo joeyespo force-pushed the fix-console-docs branch 2 times, most recently from 93df4ac to 7cdfa2e Compare July 28, 2016 04:08
@joeyespo
Copy link
Contributor Author

joeyespo commented Jul 28, 2016

@ChALkeR Ah, sorry, I misunderstood. Done! I amended the original commit with those. Rebased too.

I also found a few more whitespace tweaks while making sure I got everything, along with the inverse of this fix (console.log(process.pid + 'is alive') won't print a space in between and it probably should), and a double-to-single quotes tweak for consistency.

I can squash them into one commit or split them out to a new PR if that's preferred. Let me know!

@@ -12,7 +12,7 @@ The contents of `foo.js`:

```js
const circle = require('./circle.js');
console.log( `The area of a circle of radius 4 is ${circle.area(4)}`);
console.log(`The area of a circle of radius 4 is ${circle.area(4)}`);
Copy link
Member

Choose a reason for hiding this comment

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

There is another whitespace at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry there is nothing. It's the rendering on my phone that is wrong.

@targos
Copy link
Member

targos commented Jul 28, 2016

All changes LGTM.

@@ -223,7 +223,7 @@ For example:

```js
process.on('unhandledRejection', (reason, p) => {
console.log("Unhandled Rejection at: Promise ", p, " reason: ", reason);
console.log('Unhandled Rejection at: Promise', p, 'reason:', reason);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also fix the indention to 2 spaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amended.

@joeyespo joeyespo force-pushed the fix-console-docs branch 2 times, most recently from 03c9347 to 1fb2055 Compare July 28, 2016 22:02
@joeyespo
Copy link
Contributor Author

Ok. All comments are addressed.

@jasnell
Copy link
Member

jasnell commented Jul 29, 2016

LGTM

jasnell pushed a commit that referenced this pull request Jul 29, 2016
PR-URL: #7885
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jul 29, 2016

Landed in a single squashed commit... 6ea8c66
Thank you!

@jasnell jasnell closed this Jul 29, 2016
@joeyespo joeyespo deleted the fix-console-docs branch July 29, 2016 16:57
@cjihrig cjihrig mentioned this pull request Aug 8, 2016
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
PR-URL: #7885
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
PR-URL: #7885
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
PR-URL: #7885
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
PR-URL: #7885
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants