New primitive to fill a line with as many doc parts as possible (#1120)

* Add new fill primitive and use it to wrap text in JSX

This adds a new `fill` primitive that can be used to fill lines with as much code as possible before moving to a new line with the same indentation.

It is used here layout JSX children. This gives us nicer wrapping for JSX elements containings lots of text interspersed with tags.

* Quick fix for jsx whitespace regressions

* Fix a couple more bugs

* Tidy up the `fill` algorithm

Attempt to make the algorithm a little more regular, and improve the naming of variables to make it a little easier to understand (I hope!).

* Small tidy up of JSX whitespace declarations

* Remove unnecessary code

It turns out that `children` is only used in the case when the element is printed on a single line, in which case all the types of JSX whitespaces behave the same, so we don't need to special case leading/trailing/solitary whitespace.

* A little more tidy up based on PR feedback

* Fix up tests after rebasing

* Make it explicit that we keep multiple consecutive spaces

* Add an explanatory comment

* Fix broken snapshot in master

* Ignore existing commands when deciding whether content will fit when using fill

* Fix a bug where children would get incorrectly filled onto a line

* Tidy up JSX whitespace names
master
Karl O'Keeffe 2017-05-11 00:13:21 +01:00 committed by Christopher Chedeau
parent d4217f5508
commit 5cda955efc
12 changed files with 521 additions and 70 deletions

View File

@ -54,6 +54,12 @@ function conditionalGroup(states, opts) {
);
}
function fill(parts) {
parts.forEach(assertDoc);
return { type: "fill", parts };
}
function ifBreak(breakContents, flatContents) {
if (breakContents) {
assertDoc(breakContents);
@ -103,6 +109,7 @@ module.exports = {
literalline,
group,
conditionalGroup,
fill,
lineSuffix,
lineSuffixBoundary,
breakParent,

View File

@ -101,6 +101,13 @@ function printDoc(doc) {
);
}
if (doc.type === "fill") {
return ("fill") +
"(" +
doc.parts.map(printDoc).join(", ") +
")";
}
if (doc.type === "line-suffix") {
return "lineSuffix(" + printDoc(doc.contents) + ")";
}

View File

@ -1,5 +1,12 @@
"use strict";
const docBuilders = require("./doc-builders");
const concat = docBuilders.concat;
const line = docBuilders.line;
const softline = docBuilders.softline;
const fill = docBuilders.fill;
const ifBreak = docBuilders.ifBreak;
const MODE_BREAK = 1;
const MODE_FLAT = 2;
@ -40,7 +47,7 @@ function makeAlign(ind, n) {
};
}
function fits(next, restCommands, width) {
function fits(next, restCommands, width, mustBeFlat) {
let restIdx = restCommands.length;
const cmds = [next];
while (width >= 0) {
@ -80,8 +87,17 @@ function fits(next, restCommands, width) {
break;
case "group":
if (mustBeFlat && doc.break) {
return false;
}
cmds.push([ind, doc.break ? MODE_BREAK : mode, doc.contents]);
break;
case "fill":
for (var i = doc.parts.length - 1; i >= 0; i--) {
cmds.push([ind, mode, doc.parts[i]]);
}
break;
case "if-break":
if (mode === MODE_BREAK) {
@ -220,6 +236,86 @@ function printDocToString(doc, options) {
break;
}
break;
// Fills each line with as much code as possible before moving to a new
// line with the same indentation.
//
// Expects doc.parts to be an array of alternating content and
// whitespace. The whitespace contains the linebreaks.
//
// For example:
// ["I", line, "love", line, "monkeys"]
// or
// [{ type: group, ... }, softline, { type: group, ... }]
//
// It uses this parts structure to handle three main layout cases:
// * The first two content items fit on the same line without
// breaking
// -> output the first content item and the whitespace "flat".
// * Only the first content item fits on the line without breaking
// -> output the first content item "flat" and the whitespace with
// "break".
// * Neither content item fits on the line without breaking
// -> output the first content item and the whitespace with "break".
case "fill": {
let rem = width - pos;
const parts = doc.parts;
if (parts.length === 0) {
break;
}
const content = parts[0];
const contentFlatCmd = [ind, MODE_FLAT, content];
const contentBreakCmd = [ind, MODE_BREAK, content];
const contentFits = fits(contentFlatCmd, [], width - rem, true)
if (parts.length === 1) {
if (contentFits) {
cmds.push(contentFlatCmd);
} else {
cmds.push(contentBreakCmd);
}
break;
}
const whitespace = parts[1];
const whitespaceFlatCmd = [ind, MODE_FLAT, whitespace]
const whitespaceBreakCmd = [ind, MODE_BREAK, whitespace]
if (parts.length === 2) {
if (contentFits) {
cmds.push(whitespaceFlatCmd);
cmds.push(contentFlatCmd);
} else {
cmds.push(whitespaceBreakCmd);
cmds.push(contentBreakCmd);
}
break;
}
const remaining = parts.slice(2);
const remainingCmd = [ind, mode, fill(remaining)];
const secondContent = parts[2];
const firstAndSecondContentFlatCmd = [ind, MODE_FLAT, concat([content, whitespace, secondContent])];
const firstAndSecondContentFits = fits(firstAndSecondContentFlatCmd, [], rem, true);
if (firstAndSecondContentFits) {
cmds.push(remainingCmd)
cmds.push(whitespaceFlatCmd);
cmds.push(contentFlatCmd);
} else if (contentFits) {
cmds.push(remainingCmd)
cmds.push(whitespaceBreakCmd);
cmds.push(contentFlatCmd);
} else {
cmds.push(remainingCmd);
cmds.push(whitespaceBreakCmd);
cmds.push(contentBreakCmd);
}
break;
}
case "if-break":
if (mode === MODE_BREAK) {
if (doc.breakContents) {

View File

@ -10,7 +10,7 @@ function traverseDoc(doc, onEnter, onExit, shouldTraverseConditionalGroups) {
}
if (shouldRecurse) {
if (doc.type === "concat") {
if (doc.type === "concat" || doc.type === "fill") {
for (var i = 0; i < doc.parts.length; i++) {
traverseDocRec(doc.parts[i]);
}
@ -43,7 +43,7 @@ function traverseDoc(doc, onEnter, onExit, shouldTraverseConditionalGroups) {
function mapDoc(doc, func) {
doc = func(doc);
if (doc.type === "concat") {
if (doc.type === "concat" || doc.type === "fill") {
return Object.assign({}, doc, {
parts: doc.parts.map(d => mapDoc(d, func))
});

View File

@ -17,6 +17,7 @@ var group = docBuilders.group;
var indent = docBuilders.indent;
var align = docBuilders.align;
var conditionalGroup = docBuilders.conditionalGroup;
var fill = docBuilders.fill;
var ifBreak = docBuilders.ifBreak;
var breakParent = docBuilders.breakParent;
var lineSuffixBoundary = docBuilders.lineSuffixBoundary;
@ -3455,8 +3456,8 @@ function printJSXChildren(path, options, print, jsxWhitespace) {
if (/\S/.test(value)) {
// treat each line of text as its own entity
value.split(/(\r?\n\s*)/).forEach(line => {
const newlines = line.match(/\n/g);
value.split(/(\r?\n\s*)/).forEach(textLine => {
const newlines = textLine.match(/\n/g);
if (newlines) {
children.push(hardline);
@ -3467,27 +3468,38 @@ function printJSXChildren(path, options, print, jsxWhitespace) {
return;
}
const beginSpace = /^\s+/.test(line);
if (textLine.length === 0) {
return;
}
const beginSpace = /^\s+/.test(textLine);
if (beginSpace) {
children.push(jsxWhitespace);
} else {
children.push(softline);
}
const stripped = line.replace(/^\s+|\s+$/g, "");
const stripped = textLine.replace(/^\s+|\s+$/g, "");
if (stripped) {
children.push(stripped);
// Split text into words separated by "line"s.
stripped.split(/(\s+)/).forEach(word => {
const space = /\s+/.test(word);
if (space) {
children.push(line);
} else {
children.push(word);
}
});
}
const endSpace = /\s+$/.test(line);
const endSpace = /\s+$/.test(textLine);
if (endSpace) {
children.push(softline);
children.push(jsxWhitespace);
} else {
children.push(softline);
}
});
if (!isLineNext(util.getLast(children))) {
children.push(softline);
}
} else if (/\n/.test(value)) {
children.push(hardline);
@ -3500,15 +3512,22 @@ function printJSXChildren(path, options, print, jsxWhitespace) {
// eg; one or more spaces separating two elements
for (let i = 0; i < value.length; ++i) {
children.push(jsxWhitespace);
// Because fill expects alternating content and whitespace parts
// we need to include an empty content part between each JSX
// whitespace.
if (i + 1 < value.length) {
children.push('');
}
}
children.push(softline);
}
} else {
children.push(print(childPath));
// add a line unless it's followed by a JSX newline
// add a softline where we have two adjacent elements that are not
// literals
let next = n.children[i + 1];
if (!(next && /^\s*\n/.test(next.value))) {
const followedByJSXElement = next && !namedTypes.Literal.check(next);
if (followedByJSXElement) {
children.push(softline);
}
}
@ -3565,9 +3584,9 @@ function printJSXElement(path, options, print) {
// Record any breaks. Should never go from true to false, only false to true.
let forcedBreak = willBreak(openingLines);
const jsxWhitespace = options.singleQuote
? ifBreak("{' '}", " ")
: ifBreak('{" "}', " ");
const rawJsxWhitespace = options.singleQuote ? "{' '}" : '{" "}';
const jsxWhitespace = ifBreak(concat([softline, rawJsxWhitespace, softline]), " ")
const children = printJSXChildren(path, options, print, jsxWhitespace);
// Trim trailing lines, recording if there was a hardline
@ -3598,56 +3617,37 @@ function printJSXElement(path, options, print) {
children.unshift(hardline);
}
// Group by line, recording if there was a hardline.
let groups = [[]]; // Initialize the first line's group
// Tweak how we format children if outputting this element over multiple lines.
// Also detect whether we will force this element to output over multiple lines.
let multilineChildren = [];
children.forEach((child, i) => {
// leading and trailing JSX whitespace don't go into a group
// Ensure that we display leading, trailing, and solitary whitespace as
// `{" "}` when outputting this element over multiple lines.
if (child === jsxWhitespace) {
if (i === 0) {
groups.unshift(child);
if (children.length === 1) {
multilineChildren.push(rawJsxWhitespace);
return;
} else if (i === 0) {
multilineChildren.push(concat([rawJsxWhitespace, hardline]));
return;
} else if (i === children.length - 1) {
groups.push(child);
multilineChildren.push(concat([hardline, rawJsxWhitespace]));
return;
}
}
let prev = children[i - 1];
if (prev && willBreak(prev)) {
multilineChildren.push(child);
if (willBreak(child)) {
forcedBreak = true;
// On a new line, so create a new group and put this element in it.
groups.push([child]);
} else {
// Not on a newline, so add this element to the current group.
util.getLast(groups).push(child);
}
// Ensure we record hardline of last element.
if (!forcedBreak && i === children.length - 1) {
if (willBreak(child)) forcedBreak = true;
}
});
const childrenGroupedByLine = [
hardline,
// Conditional groups suppress break propagation; we want to output
// hard lines without breaking up the entire jsx element.
// Note that leading and trailing JSX Whitespace don't go into a group.
concat(
groups.map(
contents =>
(Array.isArray(contents)
? conditionalGroup([concat(contents)])
: contents)
)
)
];
const multiLineElem = group(
concat([
openingLines,
indent(group(concat(childrenGroupedByLine), { shouldBreak: true })),
indent(concat([hardline, fill(multilineChildren)])),
hardline,
closingLines
])
@ -3658,7 +3658,7 @@ function printJSXElement(path, options, print) {
}
return conditionalGroup([
group(concat([openingLines, concat(children), closingLines])),
group(concat([openingLines, fill(children), closingLines])),
multiLineElem
]);
}

View File

@ -164,7 +164,8 @@ var App = require("App.react");
var app = (
<App y={42}>
{" "}// error, y: number but foo expects string in App.react
{" "}
// error, y: number but foo expects string in App.react
Some text.
</App>
);

View File

@ -146,13 +146,15 @@ nest_plz = (
regression_not_transformed_1 = (
<span>
{" "}<Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
{" "}
<Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
</span>
);
regression_not_transformed_2 = (
<span>
<Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />{" "}
<Icon icon="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" />
{" "}
</span>
);
@ -178,22 +180,15 @@ similar_3 = (
not_broken_end = (
<div>
long text long text long text long text long text long text long text long text
{" "}
<link>url</link>
{" "}
long text long text
long text long text long text long text long text long text long text long
text <link>url</link> long text long text
</div>
);
not_broken_begin = (
<div>
<br />
{" "}
long text long text long text long text long text long text long text long text
<link>url</link>
{" "}
long text long text
<br /> long text long text long text long text long text long text long text
long text<link>url</link> long text long text
</div>
);

View File

@ -0,0 +1,244 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`test.js 1`] = `
// Wrapping text
x =
<div>
Some text that would need to wrap on to a new line in order to display correctly and nicely
</div>
// Wrapping tags
x =
<div>
<first>f</first> <first>f</first> <first>f</first> <first>f</first> <first>f</first> <first>f</first>
</div>
// Wrapping tags
x =
<div>
<first>f</first><first>f</first><first>f</first><first>f</first><first>f</first><first>f</first>
</div>
// Wrapping tags
x =
<div>
<a /><b /><c />
<first>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</first> <first>f</first>
</div>
// Wrapping tags
x =
<div>
<sadashdkjahsdkjhaskjdhaksjdhkashdkashdkasjhdkajshdkashdkashd /> <first>f</first>
</div>
x =
<div>
before<div>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur at mollis lorem.</div>after
</div>
x =
<div>
before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}{stuff}{stuff}after{stuff}after
</div>
x =
<div>
before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {stuff} {stuff} {stuff} after {stuff} after
</div>
x =
<div>
Please state your <b>name</b> and <b>occupation</b> for the board of <b>school</b> directors.
</div>
function DiffOverview(props) {
const { source, target, since } = props;
return (
<div>
<div className="alert alert-info">
<p>
This diff overview is computed against the current list of records in
this collection and the list it contained on <b>{humanDate(since)}</b>.
</p>
<p>
<b>Note:</b> <code>last_modified</code> and <code>schema</code> record metadata
are omitted for easier review.
</p>
</div>
<Diff source={source} target={target} />
</div>
);
}
x = <font size={-3}><i>Starting at minute {graphActivity.startTime}, running for {graphActivity.length} to minute {graphActivity.startTime + graphActivity.length}</i></font>
x =
<div>
First second third
<div attr="a very long string attribute that will overflow because it is very long">Something</div>
</div>
x =
<div>
<div>
First
</div>
Second
<div>
Third
</div>
</div>
x =
<div>
First <div>
Second
</div> Third
</div>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// Wrapping text
x = (
<div>
Some text that would need to wrap on to a new line in order to display
correctly and nicely
</div>
);
// Wrapping tags
x = (
<div>
<first>f</first> <first>f</first> <first>f</first> <first>f</first>
{" "}
<first>f</first> <first>f</first>
</div>
);
// Wrapping tags
x = (
<div>
<first>f</first><first>f</first><first>f</first><first>f</first>
<first>f</first><first>f</first>
</div>
);
// Wrapping tags
x = (
<div>
<a /><b /><c />
<first>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</first>
{" "}
<first>f</first>
</div>
);
// Wrapping tags
x = (
<div>
<sadashdkjahsdkjhaskjdhaksjdhkashdkashdkasjhdkajshdkashdkashd />
{" "}
<first>f</first>
</div>
);
x = (
<div>
before
<div>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur at
mollis lorem.
</div>
after
</div>
);
x = (
<div>
before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}
{stuff}{stuff}after{stuff}after
</div>
);
x = (
<div>
before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after
{" "}
{stuff} {stuff} {stuff} after {stuff} after
</div>
);
x = (
<div>
Please state your <b>name</b> and <b>occupation</b> for the board of
{" "}
<b>school</b> directors.
</div>
);
function DiffOverview(props) {
const { source, target, since } = props;
return (
<div>
<div className="alert alert-info">
<p>
This diff overview is computed against the current list of records in
this collection and the list it contained on <b>{humanDate(since)}</b>
.
</p>
<p>
<b>Note:</b> <code>last_modified</code> and <code>schema</code> record
metadata
are omitted for easier review.
</p>
</div>
<Diff source={source} target={target} />
</div>
);
}
x = (
<font size={-3}>
<i>
Starting at minute {graphActivity.startTime}, running for
{" "}
{graphActivity.length} to minute
{" "}
{graphActivity.startTime + graphActivity.length}
</i>
</font>
);
x = (
<div>
First second third
<div attr="a very long string attribute that will overflow because it is very long">
Something
</div>
</div>
);
x = (
<div>
<div>
First
</div>
Second
<div>
Third
</div>
</div>
);
x = (
<div>
First
{" "}
<div>
Second
</div>
{" "}
Third
</div>
);
`;

View File

@ -0,0 +1 @@
run_spec(__dirname, null, ["typescript"]);

View File

@ -0,0 +1,95 @@
// Wrapping text
x =
<div>
Some text that would need to wrap on to a new line in order to display correctly and nicely
</div>
// Wrapping tags
x =
<div>
<first>f</first> <first>f</first> <first>f</first> <first>f</first> <first>f</first> <first>f</first>
</div>
// Wrapping tags
x =
<div>
<first>f</first><first>f</first><first>f</first><first>f</first><first>f</first><first>f</first>
</div>
// Wrapping tags
x =
<div>
<a /><b /><c />
<first>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</first> <first>f</first>
</div>
// Wrapping tags
x =
<div>
<sadashdkjahsdkjhaskjdhaksjdhkashdkashdkasjhdkajshdkashdkashd /> <first>f</first>
</div>
x =
<div>
before<div>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur at mollis lorem.</div>after
</div>
x =
<div>
before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}{stuff}{stuff}after{stuff}after
</div>
x =
<div>
before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {stuff} {stuff} {stuff} after {stuff} after
</div>
x =
<div>
Please state your <b>name</b> and <b>occupation</b> for the board of <b>school</b> directors.
</div>
function DiffOverview(props) {
const { source, target, since } = props;
return (
<div>
<div className="alert alert-info">
<p>
This diff overview is computed against the current list of records in
this collection and the list it contained on <b>{humanDate(since)}</b>.
</p>
<p>
<b>Note:</b> <code>last_modified</code> and <code>schema</code> record metadata
are omitted for easier review.
</p>
</div>
<Diff source={source} target={target} />
</div>
);
}
x = <font size={-3}><i>Starting at minute {graphActivity.startTime}, running for {graphActivity.length} to minute {graphActivity.startTime + graphActivity.length}</i></font>
x =
<div>
First second third
<div attr="a very long string attribute that will overflow because it is very long">Something</div>
</div>
x =
<div>
<div>
First
</div>
Second
<div>
Third
</div>
</div>
x =
<div>
First <div>
Second
</div> Third
</div>

View File

@ -18,6 +18,7 @@ exports[`nbsp.js 1`] = `
many_nbsp = <div>&nbsp; &nbsp; </div>
single_nbsp = <div>&nbsp;</div>
many_raw_nbsp = <div>   </div>
many_raw_spaces = <div> </div>
amp = <span>foo &amp; bar</span>
@ -26,6 +27,7 @@ raw_amp = <span>foo & bar</span>
many_nbsp = <div>&nbsp; &nbsp; </div>;
single_nbsp = <div>&nbsp;</div>;
many_raw_nbsp = <div> </div>;
many_raw_spaces = <div> </div>;
amp = <span>foo &amp; bar</span>;
raw_amp = <span>foo & bar</span>;
@ -36,6 +38,7 @@ exports[`nbsp.js 2`] = `
many_nbsp = <div>&nbsp; &nbsp; </div>
single_nbsp = <div>&nbsp;</div>
many_raw_nbsp = <div>   </div>
many_raw_spaces = <div> </div>
amp = <span>foo &amp; bar</span>
@ -44,6 +47,7 @@ raw_amp = <span>foo & bar</span>
many_nbsp = <div>&nbsp; &nbsp; </div>;
single_nbsp = <div>&nbsp;</div>;
many_raw_nbsp = <div> </div>;
many_raw_spaces = <div> </div>;
amp = <span>foo &amp; bar</span>;
raw_amp = <span>foo & bar</span>;

View File

@ -1,6 +1,7 @@
many_nbsp = <div>&nbsp; &nbsp; </div>
single_nbsp = <div>&nbsp;</div>
many_raw_nbsp = <div>   </div>
many_raw_spaces = <div> </div>
amp = <span>foo &amp; bar</span>