Skip to content

Commit

Permalink
Fix indentation of a merged group (prettier#828)
Browse files Browse the repository at this point in the history
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 prettier#823
  • Loading branch information
vjeux authored and jlongster committed Feb 28, 2017
1 parent c17bcab commit 2ea1dbc
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 28 deletions.
10 changes: 5 additions & 5 deletions src/printer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))]))
);
}

Expand All @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<BaseForm
Expand Down
61 changes: 40 additions & 21 deletions tests/method-chain/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -172,39 +172,58 @@ function f() {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
export default function theFunction(action$, store) {
return action$.ofType(THE_ACTION).switchMap(action => 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));
}
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() {
Expand Down
8 changes: 8 additions & 0 deletions tests/method-chain/inline_merge.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Object.keys(
availableLocales({
test: true
})
)
.forEach(locale => {
// ...
});

0 comments on commit 2ea1dbc

Please sign in to comment.