Skip to content

Indentation fix for multi-line call expression #3179

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.

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

Merged
merged 8 commits into from
Jun 24, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 44 additions & 1 deletion src/services/formatting/smartIndenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module ts.formatting {
precedingToken.kind === SyntaxKind.TemplateHead ||
precedingToken.kind === SyntaxKind.TemplateMiddle ||
precedingToken.kind === SyntaxKind.TemplateTail;
if (precedingTokenIsLiteral && precedingToken.getStart(sourceFile) <= position && precedingToken.end > position) {
if (precedingTokenIsLiteral && precedingToken.getStart(sourceFile) <= position && precedingToken.end > position) {
return 0;
}

Expand Down Expand Up @@ -66,6 +66,10 @@ module ts.formatting {
if (actualIndentation !== Value.Unknown) {
return actualIndentation;
}
actualIndentation = getLineIndentationWhenExpressionIsInMultiLine(current, sourceFile, options);
if (actualIndentation !== Value.Unknown) {
return actualIndentation + options.IndentSize;
}

previous = current;
current = current.parent;
Expand Down Expand Up @@ -122,6 +126,10 @@ module ts.formatting {
if (actualIndentation !== Value.Unknown) {
return actualIndentation + indentationDelta;
}
actualIndentation = getLineIndentationWhenExpressionIsInMultiLine(current, sourceFile, options);
if (actualIndentation !== Value.Unknown) {
return actualIndentation + indentationDelta;
}
}

// increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line
Expand Down Expand Up @@ -287,6 +295,41 @@ module ts.formatting {
}
}

function getLineIndentationWhenExpressionIsInMultiLine(node: Node, sourceFile: SourceFile, options: EditorOptions): number {
// actual indentation should not be used when:
// - node is close parenthesis - this is the end of the expression
// - node is property access expression
if (node.kind !== SyntaxKind.CloseParenToken &&
node.kind !== SyntaxKind.PropertyAccessExpression &&
node.parent && (
node.parent.kind === SyntaxKind.CallExpression ||
node.parent.kind === SyntaxKind.NewExpression)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

CallExpression case is not covered by tests


let parentExpression = (<CallExpression | NewExpression>node.parent).expression;
let startingExpression = getStartingExpression(<PropertyAccessExpression | CallExpression | ElementAccessExpression>parentExpression);

if (parentExpression === startingExpression) {
return Value.Unknown;
}

let parentExpressionEnd = sourceFile.getLineAndCharacterOfPosition(parentExpression.end);
let startingExpressionEnd = sourceFile.getLineAndCharacterOfPosition(startingExpression.end);

if (parentExpressionEnd.line === startingExpressionEnd.line) {
return Value.Unknown;
}

return findColumnForFirstNonWhitespaceCharacterInLine(parentExpressionEnd, sourceFile, options);
}
return Value.Unknown;

function getStartingExpression(expression: PropertyAccessExpression | CallExpression | ElementAccessExpression) {
while (expression.expression)
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap loop body in curlies and add a check that Node.kind is equal to PropertyAccess / ElementAccess / CallExpression

Copy link
Contributor

Choose a reason for hiding this comment

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

add tests for that will touch all codepaths

expression = <PropertyAccessExpression | CallExpression | ElementAccessExpression>expression.expression;
return expression;
}
}

function deriveActualIndentationFromList(list: Node[], index: number, sourceFile: SourceFile, options: EditorOptions): number {
Debug.assert(index >= 0 && index < list.length);
let node = list[index];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ goTo.marker("1");
edit.insert("\r\n");
goTo.marker("0");
// Won't-fixed: Smart indent during chained function calls
verify.indentationIs(4);
verify.indentationIs(8);
17 changes: 16 additions & 1 deletion tests/cases/fourslash/formattingOnChainedCallbacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,20 @@
//// })/*b*/
////}

////Promise
//// .then(
//// /*n1*/
//// )
//// /*n2*/
//// .then();


goTo.marker('1');
edit.insertLine('');
goTo.marker('2');
verify.currentLineContentIs(' ""');
edit.insertLine('');
verify.indentationIs(8);
goTo.marker('4');
edit.insertLine('');
goTo.marker('3');
Expand All @@ -34,4 +44,9 @@ edit.insert(';');
verify.currentLineContentIs(' "";');
goTo.marker('b');
edit.insert(';');
verify.currentLineContentIs(' });');
verify.currentLineContentIs(' });');

goTo.marker('n1');
verify.indentationIs(8);
goTo.marker('n2');
verify.indentationIs(4);