Skip to content

test: introduce tag functions to handle long strings in code #13016

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.

Sign up for GitHub

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

Closed
vsemozhetbyt wants to merge 2 commits into from
Closed

test: introduce tag functions to handle long strings in code #13016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
243dfb9
test: add some base tag functions to common
vsemozhetbyt May 11, 2017
306f228
test: apply tag functions
vsemozhetbyt May 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions test/common/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,134 @@ Platform normalizes the `pwd` command.

Synchronous version of `spawnPwd`.

Copy link
Contributor

@refack refack May 14, 2017

Choose a reason for hiding this comment

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

Add ## tag function of template literals

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt May 14, 2017

Choose a reason for hiding this comment

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

OK.

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt May 14, 2017

Choose a reason for hiding this comment

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

However, this will break TOC structure and API reference alphabet principle. So I would abstain.

Copy link
Contributor

@refack refack May 14, 2017

Choose a reason for hiding this comment

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

Ok. Let's see what others think

_**Note:** tag functions for tagged template literals (begin with `tag*`)
usually take a multiline template literal with leading and trailing line break:
this allows dividing code noise (like backticks) and pure data.
These leading and trailing line breaks are stripped in the result.
All these functions leave interpolated variables untouched,
only literal parts are processed._

### tagGlue()
* return [<String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)

Tag function for tagged template literals:
removes all literal line breaks and leading spaces.
Handy to code big text blobs not intended for human reading.

<!-- eslint-disable strict -->
```js
const assert = require('assert');
const common = require('../common');

const unTagged =
'0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';

const tagged = common.tagGlue`
0123456789
ABCDEFGHIJKLMNOPQRSTUVWXYZ
abcdefghijklmnopqrstuvwxyz
`;

assert.strictEqual(unTagged, tagged);
```

### tagUnwrap()
Copy link
Contributor

@refack refack May 14, 2017

Choose a reason for hiding this comment

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

[suggestion] rename to mergeTextLines / compact. IMHO Unwrap is not clear`.

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt May 14, 2017

Choose a reason for hiding this comment

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

It seems pretty usual term in text editors (also used in our style guide) for visual formatting. Your variants seem to suppose more aggressive editing.

* return [&lt;String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)

Tag function for tagged template literals:
replaces all literal line breaks and leading spaces with a space,
then trims one leading and one trailing space.
Handy to code long readable text one-liners split for either readability
or to respect the 80 char rule.

<!-- eslint-disable strict -->
```js
const assert = require('assert');
const common = require('../common');

const unTagged =
'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod...';

const tagged = common.tagUnwrap`
Lorem ipsum dolor sit amet,
consectetur adipiscing elit,
sed do eiusmod...
`;

assert.strictEqual(unTagged, tagged);
```

### tagLFy()
* return [&lt;String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)

Tag function for tagged template literals:
removes one framing line break from the start and the end
as well as auxiliary indents (based on the indent of the first text line).
Handy to code multiline readable text, JavaScript metacode, HTML markup.

<!-- eslint-disable strict -->
```js
const assert = require('assert');
const common = require('../common');

const unTagged =
`<!doctype html>
<html>
<head>
<meta charset="UTF-8">
<title></title>
</head>
<body>
${process.versions}
</body>
</html>`;

const tagged = common.tagLFy`
<!doctype html>
<html>
<head>
<meta charset="UTF-8">
<title></title>
</head>
<body>
${process.versions}
</body>
</html>
`;

assert.strictEqual(unTagged, tagged);
```

### tagCRLFy()
* return [&lt;String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)

Tag function for tagged template literals:
same as `tagLFy()`, but replaces all literal `\n` by `\r\n`.
Handy to code raw HTTP requests/responses.

<!-- eslint-disable strict -->
```js
const assert = require('assert');
const common = require('../common');

const unTagged =
'HTTP/1.1 200 OK\r\n' +
'Content-Type: text/plain\r\n' +
'Content-Length: ' + `${process.versions}`.length + '\r\n' +
'\r\n' +
process.versions;

const tagged = common.tagCRLFy`
HTTP/1.1 200 OK
Content-Type: text/plain
Content-Length: ${`${process.versions}`.length}

${process.versions}
`;

assert.strictEqual(unTagged, tagged);
```

### tmpDir
* return [&lt;String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)

Expand Down
61 changes: 55 additions & 6 deletions test/common/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,11 @@ Object.defineProperty(exports, 'localhostIPv4', {
if (process.env.LOCALHOST) {
localhostIPv4 = process.env.LOCALHOST;
} else {
console.error('Looks like we\'re in a FreeBSD Jail. ' +
'Please provide your default interface address ' +
'as LOCALHOST or expect some tests to fail.');
console.error(exports.tagUnwrap`
Looks like we're in a FreeBSD Jail.
Please provide your default interface address as LOCALHOST
or expect some tests to fail.
`);
}
}

Expand Down Expand Up @@ -289,9 +291,10 @@ exports.childShouldThrowAndAbort = function() {
testCmd += `"${process.argv[1]}" child`;
const child = child_process.exec(testCmd);
child.on('exit', function onExit(exitCode, signal) {
const errMsg = 'Test should have aborted ' +
`but instead exited with exit code ${exitCode}` +
` and signal ${signal}`;
const errMsg = exports.tagUnwrap`
Test should have aborted
but instead exited with exit code ${exitCode} and signal ${signal}
`;
assert(exports.nodeProcessAborted(exitCode, signal), errMsg);
});
};
Expand Down Expand Up @@ -746,3 +749,49 @@ exports.getTTYfd = function getTTYfd() {
}
return tty_fd;
};

// exports.tag*: tag functions for tagged template literals

// Removes all literal line breaks and leading spaces
exports.tagGlue = function tagGlue(strings, ...expressions) {
const breaks = /\n */g;
Copy link
Contributor

@refack refack May 14, 2017

Choose a reason for hiding this comment

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

move all consts out of the functions. no need to redefine on each invocation.
could be:

const TAG_CONSTANTS = {
  tagGlueBreaks: /\n */g,
  tagUnwrapBreaks: /(\n *)+/g,
  tagUnwarpTrimmer: /^ | $/g,
...

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt May 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I would prefer to keep the functions self-sufficient for better modularity while they are not used in libs critical for performance.

Copy link
Contributor

@refack refack May 14, 2017

Choose a reason for hiding this comment

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

I won't block on this, but you might find out you can reuse some, or other coders might reuse, also gives them better names.

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt May 14, 2017

Choose a reason for hiding this comment

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

I also remember that common module is considered too bloated, so I am afraid to pollute it with more global variables.


return expressions.reduce(
(acc, expr, i) => `${acc}${expr}${strings[i + 1].replace(breaks, '')}`,
strings[0].replace(breaks, '')
);
};

// Replaces all literal line breaks and leading spaces with a space,
// then trims one leading and one trailing space
exports.tagUnwrap = function tagUnwrap(strings, ...expressions) {
const breaks = /(\n *)+/g;

return expressions.reduce(
(acc, expr, i) => `${acc}${expr}${strings[i + 1].replace(breaks, ' ')}`,
strings[0].replace(breaks, ' ')
).replace(/^ | $/g, '');
Copy link
Member

@joyeecheung joyeecheung May 14, 2017

Choose a reason for hiding this comment

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

.trim()?

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt May 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

On this stage, the replacing concerns the whole string, including interpolated variables, so I try to be extra careful here. i.e. to remove only two auxiliary spaces (which were framing line breaks before the reducing).

};

// Removes one framing line break from the start and the end
// as well as auxiliary indents (based on the indent of the first text line)
exports.tagLFy = function tagLFy(strings, ...expressions) {
const [, firstIndent = ''] = strings[0].match(/\n+( *)/) || [];
const indent = new RegExp(`\n {0,${firstIndent.length}}`, 'g');

return expressions.reduce(
(acc, expr, i) => `${acc}${expr}${strings[i + 1].replace(indent, '\n')}`,
strings[0].replace(indent, '\n')
).replace(/^\n|\n$/g, '');
};

// Same as tagLFy(), but replaces all literal \n by \r\n
exports.tagCRLFy = function tagCRLFy(strings, ...expressions) {
const [, firstIndent = ''] = strings[0].match(/\n+( *)/) || [];
const indent = new RegExp(`\n {0,${firstIndent.length}}`, 'g');

return expressions.reduce(
(acc, expr, i) => `${acc}${expr}${strings[i + 1].replace(indent, '\r\n')}`,
strings[0].replace(indent, '\r\n')
).replace(/^\r\n|\r\n$/g, '');
};
Loading