From 5595d7aa5db5cd926318ffcaa3cd590255282a3b Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Thu, 20 Apr 2017 09:22:38 +0100 Subject: [PATCH 01/14] 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. --- src/doc-builders.js | 7 ++ src/doc-debug.js | 7 ++ src/doc-printer.js | 89 ++++++++++++++++++- src/doc-utils.js | 4 +- src/printer.js | 46 ++++++---- .../__snapshots__/jsfmt.spec.js.snap | 3 +- .../__snapshots__/jsfmt.spec.js.snap | 34 +++---- .../__snapshots__/jsfmt.spec.js.snap | 4 +- .../__snapshots__/jsfmt.spec.js.snap | 3 +- .../__snapshots__/jsfmt.spec.js.snap | 71 +++++++++++++++ tests/jsx-text-wrap/jsfmt.spec.js | 1 + tests/jsx-text-wrap/test.js | 25 ++++++ 12 files changed, 247 insertions(+), 47 deletions(-) create mode 100644 tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/jsx-text-wrap/jsfmt.spec.js create mode 100644 tests/jsx-text-wrap/test.js diff --git a/src/doc-builders.js b/src/doc-builders.js index c54e41beeb80..837e0d6d3a9e 100644 --- a/src/doc-builders.js +++ b/src/doc-builders.js @@ -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, diff --git a/src/doc-debug.js b/src/doc-debug.js index df3dd2794d8a..e0732d2a54d1 100644 --- a/src/doc-debug.js +++ b/src/doc-debug.js @@ -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) + ")"; } diff --git a/src/doc-printer.js b/src/doc-printer.js index d78101d42a54..a78587f8a0be 100644 --- a/src/doc-printer.js +++ b/src/doc-printer.js @@ -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,77 @@ 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 code 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 non-whitespace items fit on the same line without + // breaking -> output both items and the whitespace "flat". + // * Only the first item fits on the line without breaking -> output the + // first item "flat" and the whitespace with "break". + // * Neither item fits on the line without breaking -> output the first + // item and the whitespace with "break". + case "fill": { + let rem = width - pos; + + const parts = doc.parts; + if (parts.length === 0) { + break; + } + + const first = parts[0]; + const firstCmd = [ind, MODE_FLAT, first]; + if (parts.length === 1) { + if (fits(firstCmd, cmds, width - rem, true)) { + cmds.push(firstCmd); + } else { + cmds.push([ind, mode, first]); + } + break; + } + + const split = parts[1]; + if (parts.length === 2) { + if (fits(firstCmd, cmds, width - rem, true)) { + cmds.push([ind, MODE_FLAT, split]); + cmds.push(firstCmd); + } else { + cmds.push([ind, mode, split]); + cmds.push([ind, mode, first]); + } + break; + } + + const second = parts[2]; + const remaining = parts.slice(2); + const remainingCmd = [ind, MODE_BREAK, fill(remaining)]; + + const firstAndSecondCmd = [ind, MODE_FLAT, concat([first, split, second])]; + + if (fits(firstAndSecondCmd, cmds, rem, true)) { + cmds.push(remainingCmd) + cmds.push([ind, MODE_FLAT, split]); + cmds.push(firstCmd); + } else if (fits(firstCmd, cmds, width - rem, true)) { + cmds.push(remainingCmd) + cmds.push([ind, MODE_BREAK, split]); + cmds.push(firstCmd); + } else { + cmds.push(remainingCmd); + cmds.push([ind, MODE_BREAK, split]); + cmds.push([ind, MODE_BREAK, first]); + } + break; + } case "if-break": if (mode === MODE_BREAK) { if (doc.breakContents) { diff --git a/src/doc-utils.js b/src/doc-utils.js index 94b88ce9778f..656db6fab0ca 100644 --- a/src/doc-utils.js +++ b/src/doc-utils.js @@ -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)) }); diff --git a/src/printer.js b/src/printer.js index 522282643cd0..a6d38708fdca 100644 --- a/src/printer.js +++ b/src/printer.js @@ -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; @@ -3436,8 +3437,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); @@ -3448,27 +3449,30 @@ function printJSXChildren(path, options, print, jsxWhitespace) { return; } - const beginSpace = /^\s+/.test(line); + const beginSpace = /^\s+/.test(textLine); if (beginSpace) { children.push(jsxWhitespace); - 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); } }); - - if (!isLineNext(util.getLast(children))) { - children.push(softline); - } } else if (/\n/.test(value)) { children.push(hardline); @@ -3481,15 +3485,19 @@ 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); + 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 JSX elements without + // any text or whitespace between them. let next = n.children[i + 1]; - if (!(next && /^\s*\n/.test(next.value))) { + const followedByJSXElement = next && next.type === 'JSXElement'; + if (followedByJSXElement) { children.push(softline); } } @@ -3547,8 +3555,8 @@ function printJSXElement(path, options, print) { let forcedBreak = willBreak(openingLines); const jsxWhitespace = options.singleQuote - ? ifBreak("{' '}", " ") - : ifBreak('{" "}', " "); + ? ifBreak(concat(["{' '}", softline]), " ") + : ifBreak(concat(['{" "}', softline]), " "); const children = printJSXChildren(path, options, print, jsxWhitespace); // Trim trailing lines, recording if there was a hardline @@ -3619,7 +3627,7 @@ function printJSXElement(path, options, print) { groups.map( contents => (Array.isArray(contents) - ? conditionalGroup([concat(contents)]) + ? conditionalGroup([fill(contents)]) : contents) ) ) @@ -3639,7 +3647,7 @@ function printJSXElement(path, options, print) { } return conditionalGroup([ - group(concat([openingLines, concat(children), closingLines])), + group(concat([openingLines, fill(children), closingLines])), multiLineElem ]); } diff --git a/tests/flow/more_react/__snapshots__/jsfmt.spec.js.snap b/tests/flow/more_react/__snapshots__/jsfmt.spec.js.snap index 6afb885f1877..dc7116992e32 100644 --- a/tests/flow/more_react/__snapshots__/jsfmt.spec.js.snap +++ b/tests/flow/more_react/__snapshots__/jsfmt.spec.js.snap @@ -164,7 +164,8 @@ var App = require("App.react"); var app = ( - {" "}// error, y: number but foo expects string in App.react + {" "} + // error, y: number but foo expects string in App.react Some text. ); diff --git a/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap index e1bb3b167212..7199c88099b7 100644 --- a/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap @@ -90,8 +90,9 @@ before = ( before_break1 = ( - - {" "} + {" "} foo ); @@ -100,16 +101,14 @@ before_break2 = ( - {" "} + />{" "} foo ); after_break = ( - foo - {" "} + foo{" "} ); @@ -146,13 +145,15 @@ nest_plz = ( regression_not_transformed_1 = ( - {" "} + {" "} + ); regression_not_transformed_2 = ( {" "} + ); @@ -165,8 +166,8 @@ similar_1 = ( similar_2 = ( - - {" "} + {" "} + ); @@ -178,22 +179,15 @@ similar_3 = ( not_broken_end = (
- long text long text long text long text long text long text long text long text - {" "} - url - {" "} - long text long text + long text long text long text long text long text long text long text long + text url long text long text
); not_broken_begin = (
-
- {" "} - long text long text long text long text long text long text long text long text - url - {" "} - long text long text +
long text long text long text long text long text long text long text + long texturl long text long text
); diff --git a/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap index b672ecc84e8a..168dc233a19f 100644 --- a/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap @@ -122,8 +122,7 @@ long_open_long_children = ( colour="blue" size="large" submitLabel="Sign in with Google" - /> - d + />d {" "} + ); diff --git a/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap index dd9d578781af..bd165a7e2bee 100644 --- a/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap @@ -130,8 +130,7 @@ const render6 = ({ styles }) => ( attr4 > ddd d dd d d dddd dddd hello - - {" "} + {" "} hello diff --git a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000000..ff636bb529eb --- /dev/null +++ b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,71 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`test.js 1`] = ` +// Wrapping text +x =
+ Some text that would need to wrap on to a new line in order to display correctly and nicely +
+ +// Wrapping tags +x =
+ f f f f f f +
+ +// Wrapping tags +x =
+ ffffff +
+ +// Wrapping tags +x = + +// Wrapping tags +x =
+ f +
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// Wrapping text +x = ( +
+ Some text that would need to wrap on to a new line in order to display + correctly and nicely +
+); + +// Wrapping tags +x = ( +
+ f f f f{" "} + f f +
+); + +// Wrapping tags +x = ( +
+ ffff + ff +
+); + +// Wrapping tags +x = ( + +); + +// Wrapping tags +x = ( +
+ {" "} + f +
+); + +`; diff --git a/tests/jsx-text-wrap/jsfmt.spec.js b/tests/jsx-text-wrap/jsfmt.spec.js new file mode 100644 index 000000000000..7580dfab0b75 --- /dev/null +++ b/tests/jsx-text-wrap/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(__dirname, null, ["typescript"]); diff --git a/tests/jsx-text-wrap/test.js b/tests/jsx-text-wrap/test.js new file mode 100644 index 000000000000..ad519949aa1a --- /dev/null +++ b/tests/jsx-text-wrap/test.js @@ -0,0 +1,25 @@ +// Wrapping text +x =
+ Some text that would need to wrap on to a new line in order to display correctly and nicely +
+ +// Wrapping tags +x =
+ f f f f f f +
+ +// Wrapping tags +x =
+ ffffff +
+ +// Wrapping tags +x = + +// Wrapping tags +x =
+ f +
From 8f12bbd85b6ac0665f013dd2cb9e0de11916810f Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Fri, 21 Apr 2017 22:53:19 +0100 Subject: [PATCH 02/14] Quick fix for jsx whitespace regressions --- src/printer.js | 31 +++++++++++++++++-- .../__snapshots__/jsfmt.spec.js.snap | 19 ++++++------ .../__snapshots__/jsfmt.spec.js.snap | 1 - .../__snapshots__/jsfmt.spec.js.snap | 3 +- .../__snapshots__/jsfmt.spec.js.snap | 9 ++++-- 5 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/printer.js b/src/printer.js index a6d38708fdca..0f1638adfc7b 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3555,8 +3555,21 @@ function printJSXElement(path, options, print) { let forcedBreak = willBreak(openingLines); const jsxWhitespace = options.singleQuote + ? ifBreak(concat([softline, "{' '}", softline]), " ") + : ifBreak(concat([softline, '{" "}', softline]), " "); + + const leadingJsxWhitespace = options.singleQuote ? ifBreak(concat(["{' '}", softline]), " ") : ifBreak(concat(['{" "}', softline]), " "); + + const trailingJsxWhitespace = options.singleQuote + ? ifBreak(concat([softline, "{' '}"]), " ") + : ifBreak(concat([softline, '{" "}']), " "); + + const solitaryJsxWhitespace = options.singleQuote + ? ifBreak(concat(["{' '}"]), " ") + : ifBreak(concat(['{" "}']), " "); + const children = printJSXChildren(path, options, print, jsxWhitespace); // Trim trailing lines, recording if there was a hardline @@ -3593,10 +3606,14 @@ function printJSXElement(path, options, print) { // leading and trailing JSX whitespace don't go into a group if (child === jsxWhitespace) { if (i === 0) { - groups.unshift(child); + if (children.length === 1) { + groups.push(solitaryJsxWhitespace); + return; + } + groups.unshift(leadingJsxWhitespace); return; } else if (i === children.length - 1) { - groups.push(child); + groups.push(trailingJsxWhitespace); return; } } @@ -3618,6 +3635,16 @@ function printJSXElement(path, options, print) { } }); + if (children[0] === jsxWhitespace) { + children[0] = leadingJsxWhitespace; + if (children.length === 1) { + children[0] = solitaryJsxWhitespace; + } + } + if (children[children.length - 1] === jsxWhitespace) { + children[children.length - 1] = trailingJsxWhitespace; + } + const childrenGroupedByLine = [ hardline, // Conditional groups suppress break propagation; we want to output diff --git a/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap index 7199c88099b7..b207e51f1f15 100644 --- a/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-significant-space/__snapshots__/jsfmt.spec.js.snap @@ -90,9 +90,8 @@ before = ( before_break1 = ( - {" "} + + {" "} foo ); @@ -101,14 +100,16 @@ before_break2 = ( {" "} + /> + {" "} foo ); after_break = ( - foo{" "} + foo + {" "} ); @@ -152,8 +153,8 @@ regression_not_transformed_1 = ( regression_not_transformed_2 = ( - {" "} - + + {" "} ); @@ -166,8 +167,8 @@ similar_1 = ( similar_2 = ( - {" "} - + + {" "} ); diff --git a/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap index 168dc233a19f..92ae55ea0813 100644 --- a/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap @@ -178,7 +178,6 @@ leave_opening = ( submitLabel="Sign in with Google" > {" "} - ); diff --git a/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap index bd165a7e2bee..dd9d578781af 100644 --- a/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap @@ -130,7 +130,8 @@ const render6 = ({ styles }) => ( attr4 > ddd d dd d d dddd dddd hello - {" "} + + {" "} hello diff --git a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap index ff636bb529eb..ae532dfc6538 100644 --- a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap @@ -38,7 +38,8 @@ x = ( // Wrapping tags x = (
- f f f f{" "} + f f f f + {" "} f f
); @@ -55,7 +56,8 @@ x = ( x = ( ); @@ -63,7 +65,8 @@ x = ( // Wrapping tags x = (
- {" "} + + {" "} f
); From db0a639413bccec85721751bf6746835fbf19dc9 Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sun, 23 Apr 2017 20:59:24 +0100 Subject: [PATCH 03/14] Fix a couple more bugs --- src/printer.js | 14 +- .../__snapshots__/jsfmt.spec.js.snap | 3 +- .../__snapshots__/jsfmt.spec.js.snap | 144 ++++++++++++++++-- tests/jsx-text-wrap/test.js | 78 ++++++++-- 4 files changed, 203 insertions(+), 36 deletions(-) diff --git a/src/printer.js b/src/printer.js index 0f1638adfc7b..93a88d9b961e 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3449,9 +3449,15 @@ function printJSXChildren(path, options, print, jsxWhitespace) { return; } + if (textLine.length === 0) { + return; + } + const beginSpace = /^\s+/.test(textLine); if (beginSpace) { children.push(jsxWhitespace); + } else { + children.push(softline); } const stripped = textLine.replace(/^\s+|\s+$/g, ""); @@ -3471,6 +3477,8 @@ function printJSXChildren(path, options, print, jsxWhitespace) { const endSpace = /\s+$/.test(textLine); if (endSpace) { children.push(jsxWhitespace); + } else { + children.push(softline); } }); } else if (/\n/.test(value)) { @@ -3493,10 +3501,10 @@ function printJSXChildren(path, options, print, jsxWhitespace) { } else { children.push(print(childPath)); - // add a softline where we have two adjacent JSX elements without - // any text or whitespace between them. + // add a softline where we have two adjacent elements that are not + // literals let next = n.children[i + 1]; - const followedByJSXElement = next && next.type === 'JSXElement'; + const followedByJSXElement = next && !namedTypes.Literal.check(next); if (followedByJSXElement) { children.push(softline); } diff --git a/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap index 92ae55ea0813..b672ecc84e8a 100644 --- a/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-split-attrs/__snapshots__/jsfmt.spec.js.snap @@ -122,7 +122,8 @@ long_open_long_children = ( colour="blue" size="large" submitLabel="Sign in with Google" - />d + /> + d - Some text that would need to wrap on to a new line in order to display correctly and nicely - +x = +
+ Some text that would need to wrap on to a new line in order to display correctly and nicely +
// Wrapping tags -x =
- f f f f f f -
+x = +
+ f f f f f f +
// Wrapping tags -x =
- ffffff -
+x = +
+ ffffff +
// Wrapping tags -x = +x = + // Wrapping tags -x =
- f -
+x = +
+ f +
+ +x = +
+ before
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur at mollis lorem.
after +
+ +x = +
+ before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}{stuff}{stuff}after{stuff}after +
+ +x = +
+ before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {stuff} {stuff} {stuff} after {stuff} after +
+ +x = +
+ Please state your name and occupation for the board of school directors. +
+ +function DiffOverview(props) { + const { source, target, since } = props; + return ( +
+
+

+ This diff overview is computed against the current list of records in + this collection and the list it contained on {humanDate(since)}. +

+

+ Note: last_modified and schema record metadata + are omitted for easier review. +

+
+ +
+ ); +} + +x = Starting at minute {graphActivity.startTime}, running for {graphActivity.length} to minute {graphActivity.startTime + graphActivity.length} ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Wrapping text x = ( @@ -71,4 +117,70 @@ x = ( ); +x = ( +
+ before +
+ Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur at + mollis lorem. +
+ after +
+); + +x = ( +
+ before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff} + {stuff}{stuff}after{stuff}after +
+); + +x = ( +
+ before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after + {" "} + {stuff} {stuff} {stuff} after {stuff} after +
+); + +x = ( +
+ Please state your name and occupation for the board of + {" "} + school directors. +
+); + +function DiffOverview(props) { + const { source, target, since } = props; + return ( +
+
+

+ This diff overview is computed against the current list of records in + this collection and the list it contained on {humanDate(since)}. +

+

+ Note: last_modified and schema record + metadata + are omitted for easier review. +

+
+ +
+ ); +} + +x = ( + + + Starting at minute {graphActivity.startTime}, running for + {" "} + {graphActivity.length} to minute + {" "} + {graphActivity.startTime + graphActivity.length} + + +); + `; diff --git a/tests/jsx-text-wrap/test.js b/tests/jsx-text-wrap/test.js index ad519949aa1a..b99b5e2d4443 100644 --- a/tests/jsx-text-wrap/test.js +++ b/tests/jsx-text-wrap/test.js @@ -1,25 +1,71 @@ // Wrapping text -x =
- Some text that would need to wrap on to a new line in order to display correctly and nicely -
+x = +
+ Some text that would need to wrap on to a new line in order to display correctly and nicely +
// Wrapping tags -x =
- f f f f f f -
+x = +
+ f f f f f f +
// Wrapping tags -x =
- ffffff -
+x = +
+ ffffff +
// Wrapping tags -x = +x = + // Wrapping tags -x =
- f -
+x = +
+ f +
+ +x = +
+ before
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur at mollis lorem.
after +
+ +x = +
+ before{stuff}after{stuff}after{stuff}after{stuff}after{stuff}after{stuff}{stuff}{stuff}after{stuff}after +
+ +x = +
+ before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {stuff} {stuff} {stuff} after {stuff} after +
+ +x = +
+ Please state your name and occupation for the board of school directors. +
+ +function DiffOverview(props) { + const { source, target, since } = props; + return ( +
+
+

+ This diff overview is computed against the current list of records in + this collection and the list it contained on {humanDate(since)}. +

+

+ Note: last_modified and schema record metadata + are omitted for easier review. +

+
+ +
+ ); +} + +x = Starting at minute {graphActivity.startTime}, running for {graphActivity.length} to minute {graphActivity.startTime + graphActivity.length} From fe548118d6487fc797569bee6d332b42dfae5b7e Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sun, 23 Apr 2017 21:33:54 +0100 Subject: [PATCH 04/14] 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!). --- src/doc-printer.js | 67 ++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/src/doc-printer.js b/src/doc-printer.js index a78587f8a0be..a3864db385aa 100644 --- a/src/doc-printer.js +++ b/src/doc-printer.js @@ -240,7 +240,7 @@ function printDocToString(doc, options) { // 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 code and + // Expects doc.parts to be an array of alternating content and // whitespace. The whitespace contains the linebreaks. // // For example: @@ -249,12 +249,14 @@ function printDocToString(doc, options) { // [{ type: group, ... }, softline, { type: group, ... }] // // It uses this parts structure to handle three main layout cases: - // * The first two non-whitespace items fit on the same line without - // breaking -> output both items and the whitespace "flat". - // * Only the first item fits on the line without breaking -> output the - // first item "flat" and the whitespace with "break". - // * Neither item fits on the line without breaking -> output the first - // item and the whitespace with "break". + // * 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; @@ -263,47 +265,54 @@ function printDocToString(doc, options) { break; } - const first = parts[0]; - const firstCmd = [ind, MODE_FLAT, first]; + const content = parts[0]; + const contentFlatCmd = [ind, MODE_FLAT, content]; + const contentBreakCmd = [ind, MODE_BREAK, content]; + const contentFits = fits(contentFlatCmd, cmds, width - rem, true) + if (parts.length === 1) { - if (fits(firstCmd, cmds, width - rem, true)) { - cmds.push(firstCmd); + if (contentFits) { + cmds.push(contentFlatCmd); } else { - cmds.push([ind, mode, first]); + cmds.push(contentBreakCmd); } break; } - const split = parts[1]; + const whitespace = parts[1]; + const whitespaceFlatCmd = [ind, MODE_FLAT, whitespace] + const whitespaceBreakCmd = [ind, MODE_BREAK, whitespace] + if (parts.length === 2) { - if (fits(firstCmd, cmds, width - rem, true)) { - cmds.push([ind, MODE_FLAT, split]); - cmds.push(firstCmd); + if (contentFits) { + cmds.push(whitespaceFlatCmd); + cmds.push(contentFlatCmd); } else { - cmds.push([ind, mode, split]); - cmds.push([ind, mode, first]); + cmds.push(whitespaceBreakCmd); + cmds.push(contentBreakCmd); } break; } - const second = parts[2]; const remaining = parts.slice(2); - const remainingCmd = [ind, MODE_BREAK, fill(remaining)]; + const remainingCmd = [ind, mode, fill(remaining)]; - const firstAndSecondCmd = [ind, MODE_FLAT, concat([first, split, second])]; + const secondContent = parts[2]; + const firstAndSecondContentFlatCmd = [ind, MODE_FLAT, concat([content, whitespace, secondContent])]; + const firstAndSecondContentFits = fits(firstAndSecondContentFlatCmd, cmds, rem, true); - if (fits(firstAndSecondCmd, cmds, rem, true)) { + if (firstAndSecondContentFits) { cmds.push(remainingCmd) - cmds.push([ind, MODE_FLAT, split]); - cmds.push(firstCmd); - } else if (fits(firstCmd, cmds, width - rem, true)) { + cmds.push(whitespaceFlatCmd); + cmds.push(contentFlatCmd); + } else if (contentFits) { cmds.push(remainingCmd) - cmds.push([ind, MODE_BREAK, split]); - cmds.push(firstCmd); + cmds.push(whitespaceBreakCmd); + cmds.push(contentFlatCmd); } else { cmds.push(remainingCmd); - cmds.push([ind, MODE_BREAK, split]); - cmds.push([ind, MODE_BREAK, first]); + cmds.push(whitespaceBreakCmd); + cmds.push(contentBreakCmd); } break; } From 12e171666a6983d06cfaa5a96cdf795ffb3413bc Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sun, 23 Apr 2017 21:43:22 +0100 Subject: [PATCH 05/14] Small tidy up of JSX whitespace declarations --- src/printer.js | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/src/printer.js b/src/printer.js index 93a88d9b961e..3560782dd6ab 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3423,7 +3423,7 @@ function isEmptyJSXElement(node) { // // For another, leading, trailing, and lone whitespace all need to // turn themselves into the rather ugly `{' '}` when breaking. -function printJSXChildren(path, options, print, jsxWhitespace) { +function printJSXChildren(path, options, print, innerJsxWhitespace) { const n = path.getValue(); const children = []; @@ -3455,7 +3455,7 @@ function printJSXChildren(path, options, print, jsxWhitespace) { const beginSpace = /^\s+/.test(textLine); if (beginSpace) { - children.push(jsxWhitespace); + children.push(innerJsxWhitespace); } else { children.push(softline); } @@ -3476,7 +3476,7 @@ function printJSXChildren(path, options, print, jsxWhitespace) { const endSpace = /\s+$/.test(textLine); if (endSpace) { - children.push(jsxWhitespace); + children.push(innerJsxWhitespace); } else { children.push(softline); } @@ -3492,7 +3492,7 @@ function printJSXChildren(path, options, print, jsxWhitespace) { // whitespace(s)-only without newlines, // eg; one or more spaces separating two elements for (let i = 0; i < value.length; ++i) { - children.push(jsxWhitespace); + children.push(innerJsxWhitespace); if (i + 1 < value.length) { children.push(''); } @@ -3562,23 +3562,13 @@ 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(concat([softline, "{' '}", softline]), " ") - : ifBreak(concat([softline, '{" "}', softline]), " "); + const jsxWhitespace = options.singleQuote ? "{' '}" : '{" "}'; + const innerJsxWhitespace = ifBreak(concat([softline, jsxWhitespace, softline]), " ") + const leadingJsxWhitespace = ifBreak(concat([jsxWhitespace, softline]), " "); + const trailingJsxWhitespace = ifBreak(concat([softline, jsxWhitespace]), " "); + const solitaryJsxWhitespace = ifBreak(jsxWhitespace, " "); - const leadingJsxWhitespace = options.singleQuote - ? ifBreak(concat(["{' '}", softline]), " ") - : ifBreak(concat(['{" "}', softline]), " "); - - const trailingJsxWhitespace = options.singleQuote - ? ifBreak(concat([softline, "{' '}"]), " ") - : ifBreak(concat([softline, '{" "}']), " "); - - const solitaryJsxWhitespace = options.singleQuote - ? ifBreak(concat(["{' '}"]), " ") - : ifBreak(concat(['{" "}']), " "); - - const children = printJSXChildren(path, options, print, jsxWhitespace); + const children = printJSXChildren(path, options, print, innerJsxWhitespace); // Trim trailing lines, recording if there was a hardline let numTrailingHard = 0; @@ -3612,7 +3602,7 @@ function printJSXElement(path, options, print) { let groups = [[]]; // Initialize the first line's group children.forEach((child, i) => { // leading and trailing JSX whitespace don't go into a group - if (child === jsxWhitespace) { + if (child === innerJsxWhitespace) { if (i === 0) { if (children.length === 1) { groups.push(solitaryJsxWhitespace); @@ -3643,13 +3633,13 @@ function printJSXElement(path, options, print) { } }); - if (children[0] === jsxWhitespace) { + if (children[0] === innerJsxWhitespace) { children[0] = leadingJsxWhitespace; if (children.length === 1) { children[0] = solitaryJsxWhitespace; } } - if (children[children.length - 1] === jsxWhitespace) { + if (children[children.length - 1] === innerJsxWhitespace) { children[children.length - 1] = trailingJsxWhitespace; } From dba1a7200a3d10b4cd1d0d9c62a643aa497392fb Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Mon, 24 Apr 2017 21:36:11 +0100 Subject: [PATCH 06/14] 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. --- src/printer.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/printer.js b/src/printer.js index 3560782dd6ab..92633a535713 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3633,16 +3633,6 @@ function printJSXElement(path, options, print) { } }); - if (children[0] === innerJsxWhitespace) { - children[0] = leadingJsxWhitespace; - if (children.length === 1) { - children[0] = solitaryJsxWhitespace; - } - } - if (children[children.length - 1] === innerJsxWhitespace) { - children[children.length - 1] = trailingJsxWhitespace; - } - const childrenGroupedByLine = [ hardline, // Conditional groups suppress break propagation; we want to output From 5496bdeadf4540f2a7bd04108fb3ae3198bb61de Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Mon, 24 Apr 2017 21:37:54 +0100 Subject: [PATCH 07/14] A little more tidy up based on PR feedback --- src/printer.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/printer.js b/src/printer.js index 92633a535713..198ea225357c 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3603,11 +3603,11 @@ function printJSXElement(path, options, print) { children.forEach((child, i) => { // leading and trailing JSX whitespace don't go into a group if (child === innerJsxWhitespace) { - if (i === 0) { - if (children.length === 1) { - groups.push(solitaryJsxWhitespace); - return; - } + if (children.length === 1) { + groups.unshift(solitaryJsxWhitespace); + groups.pop(); // remove unnecessary empty group + return; + } else if (i === 0) { groups.unshift(leadingJsxWhitespace); return; } else if (i === children.length - 1) { From 4fd4444833c4c9da1c732e684b2abe9559224519 Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sat, 6 May 2017 21:17:24 +0100 Subject: [PATCH 08/14] Fix up tests after rebasing --- tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap index 378a12bea314..84a2da6853de 100644 --- a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap @@ -139,7 +139,7 @@ x = (
before {stuff} after {stuff} after {stuff} after {stuff} after {stuff} after {" "} - {stuff} {stuff} {stuff} after {stuff} after + {stuff} {stuff} {stuff} after {stuff} after
); From 1858d5e0812f0da3a5b898b22ac57cd4da085945 Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sat, 6 May 2017 21:19:33 +0100 Subject: [PATCH 09/14] Make it explicit that we keep multiple consecutive spaces --- tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap | 4 ++++ tests/jsx_escape/nbsp.js | 1 + 2 files changed, 5 insertions(+) diff --git a/tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap b/tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap index 36657874bf0d..368e10f1ed26 100644 --- a/tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx_escape/__snapshots__/jsfmt.spec.js.snap @@ -18,6 +18,7 @@ exports[`nbsp.js 1`] = ` many_nbsp =
   
single_nbsp =
 
many_raw_nbsp =
   
+many_raw_spaces =
amp = foo & bar @@ -26,6 +27,7 @@ raw_amp = foo & bar many_nbsp =
   
; single_nbsp =
 
; many_raw_nbsp =
; +many_raw_spaces =
; amp = foo & bar; raw_amp = foo & bar; @@ -36,6 +38,7 @@ exports[`nbsp.js 2`] = ` many_nbsp =
   
single_nbsp =
 
many_raw_nbsp =
   
+many_raw_spaces =
amp = foo & bar @@ -44,6 +47,7 @@ raw_amp = foo & bar many_nbsp =
   
; single_nbsp =
 
; many_raw_nbsp =
; +many_raw_spaces =
; amp = foo & bar; raw_amp = foo & bar; diff --git a/tests/jsx_escape/nbsp.js b/tests/jsx_escape/nbsp.js index 87f6989a1cb2..fe3afc604734 100644 --- a/tests/jsx_escape/nbsp.js +++ b/tests/jsx_escape/nbsp.js @@ -1,6 +1,7 @@ many_nbsp =
   
single_nbsp =
 
many_raw_nbsp =
   
+many_raw_spaces =
amp = foo & bar From 9884b3a54503504f15f1630f3b0dfbb641bea11c Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sat, 6 May 2017 21:25:29 +0100 Subject: [PATCH 10/14] Add an explanatory comment --- src/printer.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/printer.js b/src/printer.js index 198ea225357c..cdeaeb3ebec8 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3493,6 +3493,9 @@ function printJSXChildren(path, options, print, innerJsxWhitespace) { // eg; one or more spaces separating two elements for (let i = 0; i < value.length; ++i) { children.push(innerJsxWhitespace); + // 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(''); } From 50908f6b9ad0e58239f05fc58e4578da143c3988 Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sun, 7 May 2017 19:24:33 +0100 Subject: [PATCH 11/14] Fix broken snapshot in master --- .../expressions/functionCalls/__snapshots__/jsfmt.spec.js.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/typescript/conformance/expressions/functionCalls/__snapshots__/jsfmt.spec.js.snap b/tests/typescript/conformance/expressions/functionCalls/__snapshots__/jsfmt.spec.js.snap index 16502cb83212..a71ea878ac0e 100644 --- a/tests/typescript/conformance/expressions/functionCalls/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript/conformance/expressions/functionCalls/__snapshots__/jsfmt.spec.js.snap @@ -56,7 +56,7 @@ class D extends C { // @target: ES6 interface X { - foo(x: number, y: number, ...z: string[]) + foo(x: number, y: number, ...z: string[]); } function foo(x: number, y: number, ...z: string[]) {} From 4b4e365c7a10e7ae7b34bc300d0ab7ede9581b9b Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sun, 7 May 2017 20:02:23 +0100 Subject: [PATCH 12/14] Ignore existing commands when deciding whether content will fit when using fill --- src/doc-printer.js | 6 +-- .../__snapshots__/jsfmt.spec.js.snap | 38 +++++++++++++++++++ tests/jsx-text-wrap/test.js | 17 +++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/doc-printer.js b/src/doc-printer.js index a3864db385aa..d248d5fa246e 100644 --- a/src/doc-printer.js +++ b/src/doc-printer.js @@ -268,7 +268,7 @@ function printDocToString(doc, options) { const content = parts[0]; const contentFlatCmd = [ind, MODE_FLAT, content]; const contentBreakCmd = [ind, MODE_BREAK, content]; - const contentFits = fits(contentFlatCmd, cmds, width - rem, true) + const contentFits = fits(contentFlatCmd, [], width - rem, true) if (parts.length === 1) { if (contentFits) { @@ -299,7 +299,7 @@ function printDocToString(doc, options) { const secondContent = parts[2]; const firstAndSecondContentFlatCmd = [ind, MODE_FLAT, concat([content, whitespace, secondContent])]; - const firstAndSecondContentFits = fits(firstAndSecondContentFlatCmd, cmds, rem, true); + const firstAndSecondContentFits = fits(firstAndSecondContentFlatCmd, [], rem, true); if (firstAndSecondContentFits) { cmds.push(remainingCmd) @@ -384,7 +384,7 @@ function printDocToString(doc, options) { out[out.length - 1] = out[out.length - 1].replace( /[^\S\n]*$/, "" - ); + ); } } diff --git a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap index 84a2da6853de..86e0c2c0b8c6 100644 --- a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap @@ -72,6 +72,23 @@ function DiffOverview(props) { } x = Starting at minute {graphActivity.startTime}, running for {graphActivity.length} to minute {graphActivity.startTime + graphActivity.length} + +x = +
+ First second third +
Something
+
+ +x = +
+
+ First +
+ Second +
+ Third +
+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Wrapping text x = ( @@ -183,4 +200,25 @@ x = ( ); +x = ( +
+ First second third +
+ Something +
+
+); + +x = ( +
+
+ First +
+ Second +
+ Third +
+
+); + `; diff --git a/tests/jsx-text-wrap/test.js b/tests/jsx-text-wrap/test.js index b99b5e2d4443..336f3f661dd6 100644 --- a/tests/jsx-text-wrap/test.js +++ b/tests/jsx-text-wrap/test.js @@ -69,3 +69,20 @@ function DiffOverview(props) { } x = Starting at minute {graphActivity.startTime}, running for {graphActivity.length} to minute {graphActivity.startTime + graphActivity.length} + +x = +
+ First second third +
Something
+
+ +x = +
+
+ First +
+ Second +
+ Third +
+
From 83397ec2199cdd4772544928bd8c74112583728c Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sun, 7 May 2017 20:43:57 +0100 Subject: [PATCH 13/14] Fix a bug where children would get incorrectly filled onto a line --- src/printer.js | 49 +++++-------------- .../__snapshots__/jsfmt.spec.js.snap | 22 ++++++++- tests/jsx-text-wrap/test.js | 7 +++ 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/printer.js b/src/printer.js index cdeaeb3ebec8..52a4d6efac43 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3601,60 +3601,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 === innerJsxWhitespace) { if (children.length === 1) { - groups.unshift(solitaryJsxWhitespace); - groups.pop(); // remove unnecessary empty group + multilineChildren.push(jsxWhitespace); return; } else if (i === 0) { - groups.unshift(leadingJsxWhitespace); + multilineChildren.push(concat([jsxWhitespace, hardline])); return; } else if (i === children.length - 1) { - groups.push(trailingJsxWhitespace); + multilineChildren.push(concat([hardline, jsxWhitespace])); return; } } - let prev = children[i - 1]; - if (prev && willBreak(prev)) { - 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); - } + multilineChildren.push(child); - // Ensure we record hardline of last element. - if (!forcedBreak && i === children.length - 1) { - if (willBreak(child)) forcedBreak = true; + 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([fill(contents)]) - : contents) - ) - ) - ]; - const multiLineElem = group( concat([ openingLines, - indent(group(concat(childrenGroupedByLine), { shouldBreak: true })), + indent(concat([hardline, fill(multilineChildren)])), hardline, closingLines ]) diff --git a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap index 86e0c2c0b8c6..b1aaaa14798e 100644 --- a/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap @@ -89,6 +89,13 @@ x = Third + +x = +
+ First
+ Second +
Third +
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Wrapping text x = ( @@ -175,7 +182,8 @@ function DiffOverview(props) {

This diff overview is computed against the current list of records in - this collection and the list it contained on {humanDate(since)}. + this collection and the list it contained on {humanDate(since)} + .

Note: last_modified and schema record @@ -221,4 +229,16 @@ x = (

); +x = ( +
+ First + {" "} +
+ Second +
+ {" "} + Third +
+); + `; diff --git a/tests/jsx-text-wrap/test.js b/tests/jsx-text-wrap/test.js index 336f3f661dd6..5a526643d48c 100644 --- a/tests/jsx-text-wrap/test.js +++ b/tests/jsx-text-wrap/test.js @@ -86,3 +86,10 @@ x = Third + +x = +
+ First
+ Second +
Third +
From b84438e930927bef3cf5c2c3f7c34af60e9811c5 Mon Sep 17 00:00:00 2001 From: Karl O'Keeffe Date: Sun, 7 May 2017 20:53:58 +0100 Subject: [PATCH 14/14] Tidy up JSX whitespace names --- src/printer.js | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/printer.js b/src/printer.js index 52a4d6efac43..96c72579c63b 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3423,7 +3423,7 @@ function isEmptyJSXElement(node) { // // For another, leading, trailing, and lone whitespace all need to // turn themselves into the rather ugly `{' '}` when breaking. -function printJSXChildren(path, options, print, innerJsxWhitespace) { +function printJSXChildren(path, options, print, jsxWhitespace) { const n = path.getValue(); const children = []; @@ -3455,7 +3455,7 @@ function printJSXChildren(path, options, print, innerJsxWhitespace) { const beginSpace = /^\s+/.test(textLine); if (beginSpace) { - children.push(innerJsxWhitespace); + children.push(jsxWhitespace); } else { children.push(softline); } @@ -3476,7 +3476,7 @@ function printJSXChildren(path, options, print, innerJsxWhitespace) { const endSpace = /\s+$/.test(textLine); if (endSpace) { - children.push(innerJsxWhitespace); + children.push(jsxWhitespace); } else { children.push(softline); } @@ -3492,7 +3492,7 @@ function printJSXChildren(path, options, print, innerJsxWhitespace) { // whitespace(s)-only without newlines, // eg; one or more spaces separating two elements for (let i = 0; i < value.length; ++i) { - children.push(innerJsxWhitespace); + children.push(jsxWhitespace); // Because fill expects alternating content and whitespace parts // we need to include an empty content part between each JSX // whitespace. @@ -3565,13 +3565,10 @@ 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 ? "{' '}" : '{" "}'; - const innerJsxWhitespace = ifBreak(concat([softline, jsxWhitespace, softline]), " ") - const leadingJsxWhitespace = ifBreak(concat([jsxWhitespace, softline]), " "); - const trailingJsxWhitespace = ifBreak(concat([softline, jsxWhitespace]), " "); - const solitaryJsxWhitespace = ifBreak(jsxWhitespace, " "); + const rawJsxWhitespace = options.singleQuote ? "{' '}" : '{" "}'; + const jsxWhitespace = ifBreak(concat([softline, rawJsxWhitespace, softline]), " ") - const children = printJSXChildren(path, options, print, innerJsxWhitespace); + const children = printJSXChildren(path, options, print, jsxWhitespace); // Trim trailing lines, recording if there was a hardline let numTrailingHard = 0; @@ -3608,15 +3605,15 @@ function printJSXElement(path, options, print) { // Ensure that we display leading, trailing, and solitary whitespace as // `{" "}` when outputting this element over multiple lines. - if (child === innerJsxWhitespace) { + if (child === jsxWhitespace) { if (children.length === 1) { - multilineChildren.push(jsxWhitespace); + multilineChildren.push(rawJsxWhitespace); return; } else if (i === 0) { - multilineChildren.push(concat([jsxWhitespace, hardline])); + multilineChildren.push(concat([rawJsxWhitespace, hardline])); return; } else if (i === children.length - 1) { - multilineChildren.push(concat([hardline, jsxWhitespace])); + multilineChildren.push(concat([hardline, rawJsxWhitespace])); return; } }