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

tools: replace string concatenation with template literals #16804

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
7 participants
@kevinwcyu
Copy link
Contributor

commented Nov 6, 2017

Replace string concatenation in tools/doc/preprocess.js with template literals.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)
  • tools
inc + `\n<!-- [end-include:${fname}] -->\n`;
input = input.split(include + '\n').join(includeData[fname] + '\n');
includeData[fname] = `<!-- [start-include:${fname}] -->
${inc}

This comment has been minimized.

Copy link
@gireeshpunathil

gireeshpunathil Nov 6, 2017

Member

lost indentation here?

includeData[fname] = `<!-- [start-include:${fname}] -->\n` +
inc + `\n<!-- [end-include:${fname}] -->\n`;
input = input.split(include + '\n').join(includeData[fname] + '\n');
includeData[fname] = `<!-- [start-include:${fname}] -->

This comment has been minimized.

Copy link
@apapirovski

apapirovski Nov 6, 2017

Member

Multi-line string concatenation is actually ok (the way it was before), but it would be nice if the 2nd line became this instead:

`${inc}\n<!-- [end-include:${fname}] -->\n`

That way we don't have to lose indentation level.

@kevinwcyu kevinwcyu force-pushed the kevinwcyu:master branch from bb6a40a to bef2e39 Nov 6, 2017

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Nov 6, 2017

@Trott

Trott approved these changes Nov 6, 2017

Trott added a commit to Trott/io.js that referenced this pull request Nov 6, 2017

tools: replace string concatenation with template literals
Replace string concatenation with template literals in
tools/doc/preprocess.js.

PR-URL: nodejs#16804
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@Trott

This comment has been minimized.

Copy link
Member

commented Nov 6, 2017

Landed in 13e983b

@Trott Trott closed this Nov 6, 2017

@Trott

This comment has been minimized.

Copy link
Member

commented Nov 6, 2017

Thanks for the contribution! 🎉

cjihrig added a commit to cjihrig/node-1 that referenced this pull request Nov 7, 2017

tools: replace string concatenation with template literals
Replace string concatenation with template literals in
tools/doc/preprocess.js.

PR-URL: nodejs#16804
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

@cjihrig cjihrig referenced this pull request Nov 7, 2017

Merged

v9.1.0 proposal #16851

MylesBorins added a commit that referenced this pull request Nov 16, 2017

tools: replace string concatenation with template literals
Replace string concatenation with template literals in
tools/doc/preprocess.js.

PR-URL: #16804
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

MylesBorins added a commit that referenced this pull request Nov 16, 2017

tools: replace string concatenation with template literals
Replace string concatenation with template literals in
tools/doc/preprocess.js.

PR-URL: #16804
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

@MylesBorins MylesBorins referenced this pull request Nov 21, 2017

Merged

v6.12.1 proposal #17180

@gibfahn gibfahn referenced this pull request Nov 21, 2017

Merged

v8.9.2 proposal #17204

MylesBorins added a commit that referenced this pull request Nov 21, 2017

tools: replace string concatenation with template literals
Replace string concatenation with template literals in
tools/doc/preprocess.js.

PR-URL: #16804
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

MylesBorins added a commit that referenced this pull request Nov 28, 2017

tools: replace string concatenation with template literals
Replace string concatenation with template literals in
tools/doc/preprocess.js.

PR-URL: #16804
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

tools: replace string concatenation with template literals
Replace string concatenation with template literals in
tools/doc/preprocess.js.

PR-URL: nodejs#16804
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.