From 2ea1dbcb6ab53c68c4b7ab897eb6cd8430d4af59 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Tue, 28 Feb 2017 07:55:32 -0800 Subject: [PATCH] Fix indentation of a merged group (#828) Printing a merged group indented was actually not the right fix. The right fix was to print them in a single line. It used to have this behavior when I was mutating the first group but now that I don't anymore I need to reproduce this condition. Fixes #823 --- src/printer.js | 10 +-- .../__snapshots__/jsfmt.spec.js.snap | 6 +- .../__snapshots__/jsfmt.spec.js.snap | 61 ++++++++++++------- tests/method-chain/inline_merge.js | 8 +++ 4 files changed, 57 insertions(+), 28 deletions(-) create mode 100644 tests/method-chain/inline_merge.js diff --git a/src/printer.js b/src/printer.js index a87012bf6..3a9e6e86d 100644 --- a/src/printer.js +++ b/src/printer.js @@ -2381,10 +2381,10 @@ function printMemberChain(path, options, print) { return concat(printedGroup.map(tuple => tuple.printed)); } - function printIndentedGroup(groups, lineType) { + function printIndentedGroup(groups) { return indent( options.tabWidth, - group(concat([lineType, join(lineType, groups.map(printGroup))])) + group(concat([hardline, join(hardline, groups.map(printGroup))])) ); } @@ -2394,14 +2394,14 @@ function printMemberChain(path, options, print) { // If we only have a single `.`, we shouldn't do anything fancy and just // render everything concatenated together. - if (groups.length <= 2 && !hasComment) { + if (groups.length <= (shouldMerge ? 3 : 2) && !hasComment) { return group(oneLine); } const expanded = concat([ printGroup(groups[0]), - shouldMerge ? printIndentedGroup(groups.slice(1, 2), "") : "", - printIndentedGroup(groups.slice(shouldMerge ? 2 : 1), hardline) + shouldMerge ? concat(groups.slice(1, 2).map(printGroup)) : "", + printIndentedGroup(groups.slice(shouldMerge ? 2 : 1)) ]); // If there's a comment, we don't want to print in one line. 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 94186dbfd..21a7ac786 100644 --- a/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap +++ b/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap @@ -169,8 +169,10 @@ const render10 = props => ( ); const notJSX = (aaaaaaaaaaaaaaaaa, bbbbbbbbbbb) => - this.someLongCallWithParams(aaaaaa, bbbbbbb) - .anotherLongCallWithParams(cccccccccccc, dddddddddddddddddddddd); + this.someLongCallWithParams(aaaaaa, bbbbbbb).anotherLongCallWithParams( + cccccccccccc, + dddddddddddddddddddddd + ); React.render( Observable.webSocket({ - url: THE_URL, - more: stuff(), - evenMore: stuff({ - value1: true, - value2: false, - value3: false - }) + url: THE_URL, + more: stuff(), + evenMore: stuff({ + value1: true, + value2: false, + value3: false }) + }) .filter(data => theFilter(data)) .map(({ theType, ...data }) => theMap(theType, data)) .retryWhen(errors => errors)); @@ -187,24 +187,43 @@ export default function theFunction(action$, store) { function f() { return this._getWorker(workerOptions)({ - filePath, - hasteImplModulePath: this._options.hasteImplModulePath - }) - .then(metadata => { - // \`1\` for truthy values instead of \`true\` to save cache space. - fileMetadata[H.VISITED] = 1; - const metadataId = metadata.id; - const metadataModule = metadata.module; - if (metadataId && metadataModule) { - fileMetadata[H.ID] = metadataId; - setModule(metadataId, metadataModule); - } - fileMetadata[H.DEPENDENCIES] = metadata.dependencies || []; - }); + filePath, + hasteImplModulePath: this._options.hasteImplModulePath + }).then(metadata => { + // \`1\` for truthy values instead of \`true\` to save cache space. + fileMetadata[H.VISITED] = 1; + const metadataId = metadata.id; + const metadataModule = metadata.module; + if (metadataId && metadataModule) { + fileMetadata[H.ID] = metadataId; + setModule(metadataId, metadataModule); + } + fileMetadata[H.DEPENDENCIES] = metadata.dependencies || []; + }); } " `; +exports[`inline_merge.js 1`] = ` +"Object.keys( + availableLocales({ + test: true + }) +) +.forEach(locale => { + // ... +}); +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Object.keys( + availableLocales({ + test: true + }) +).forEach(locale => { + // ... +}); +" +`; + exports[`multiple-members.js 1`] = ` "if (testConfig.ENABLE_ONLINE_TESTS === \\"true\\") { describe(\\"POST /users/me/pet\\", function() { diff --git a/tests/method-chain/inline_merge.js b/tests/method-chain/inline_merge.js new file mode 100644 index 000000000..7a1623fb7 --- /dev/null +++ b/tests/method-chain/inline_merge.js @@ -0,0 +1,8 @@ +Object.keys( + availableLocales({ + test: true + }) +) +.forEach(locale => { + // ... +});