Skip to content

Commit 61af315

Browse files
author
Arthur Ozga
committed
respond to comments
1 parent 485927b commit 61af315

6 files changed

Lines changed: 52 additions & 44 deletions

File tree

‎src/services/formatting/formatting.ts‎

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -97,23 +97,21 @@ namespace ts.formatting {
9797
}
9898

9999
export function formatOnSemicolon(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] {
100-
const outermostParent = findOutermostParentWithinListLevelFromPosition(position, SyntaxKind.SemicolonToken, sourceFile);
101-
return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon);
100+
const semicolon = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.SemicolonToken, sourceFile);
101+
return formatOutermostNodeWithinListLevel(semicolon, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon);
102102
}
103103

104104
export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] {
105-
const openingCurly = findPrecedingTokenOfKind(position, SyntaxKind.FirstPunctuation, sourceFile);
106-
const block = openingCurly && openingCurly.parent;
107-
if (!(block && isBlock(block))) {
108-
return [];
109-
}
110-
const outermostParent = findOutermostParentWithinListLevel(block);
111-
return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace);
105+
const openingCurly = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.OpenBraceToken, sourceFile);
106+
const curlyBraceRange = openingCurly && openingCurly.parent;
107+
return curlyBraceRange ?
108+
formatOutermostNodeWithinListLevel(curlyBraceRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace) :
109+
[];
112110
}
113111

114112
export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] {
115-
const outermostParent = findOutermostParentWithinListLevelFromPosition(position, SyntaxKind.CloseBraceToken, sourceFile);
116-
return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace);
113+
const precedingToken = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.CloseBraceToken, sourceFile);
114+
return formatOutermostNodeWithinListLevel(precedingToken, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace);
117115
}
118116

119117
export function formatDocument(sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] {
@@ -133,38 +131,21 @@ namespace ts.formatting {
133131
return formatSpan(span, sourceFile, options, rulesProvider, FormattingRequestKind.FormatSelection);
134132
}
135133

136-
function formatOutermostParent(parent: Node | undefined, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] {
137-
if (!parent) {
138-
return [];
139-
}
140-
141-
const span = {
142-
pos: getLineStartPositionForPosition(parent.getStart(sourceFile), sourceFile),
143-
end: parent.end
144-
};
145-
146-
return formatSpan(span, sourceFile, options, rulesProvider, requestKind);
147-
}
148-
149-
function findPrecedingTokenOfKind(position: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node | undefined {
150-
const precedingToken = findPrecedingToken(position, sourceFile);
151-
152-
return precedingToken && precedingToken.kind === expectedTokenKind && position === precedingToken.getEnd() ?
153-
precedingToken :
154-
undefined;
155-
}
156-
157134
/**
158135
* Validating `expectedLastToken` ensures the token was typed in the context we expect (eg: not a comment).
159136
* @param expectedLastToken The last token constituting the desired parent node.
160137
*/
161-
function findOutermostParentWithinListLevelFromPosition(position: number, expectedLastToken: SyntaxKind, sourceFile: SourceFile) {
162-
const precedingToken = findPrecedingTokenOfKind(position, expectedLastToken, sourceFile);
163-
return precedingToken && findOutermostParentWithinListLevel(precedingToken);
138+
function findImmediatelyPrecedingTokenOfKind(end: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node | undefined {
139+
const precedingToken = findPrecedingToken(end, sourceFile);
140+
141+
return precedingToken && precedingToken.kind === expectedTokenKind && end === precedingToken.getEnd() ?
142+
precedingToken :
143+
undefined;
164144
}
165145

166146
/**
167-
* Finds the outermost parent within the same list level as the token at position.
147+
* Finds and formats the highest node enclosing `position` whose end does not exceed the given position
148+
* and is at the same list level as the token at `position`.
168149
*
169150
* Consider typing the following
170151
* ```
@@ -175,18 +156,18 @@ namespace ts.formatting {
175156
* Upon typing the closing curly, we want to format the entire `while`-statement, but not the preceding
176157
* variable declaration.
177158
*/
178-
function findOutermostParentWithinListLevel(token: Node): Node {
159+
function formatOutermostNodeWithinListLevel(node: Node, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, formattingRequestKind: FormattingRequestKind) {
179160
// If we walk upwards searching for the parent that has the same end value, we'll end up with the whole source file.
180161
// `isListElement` allows to stop on the list element level.
181-
let current = token;
162+
let current = node;
182163
while (current &&
183164
current.parent &&
184-
current.parent.end === token.end &&
165+
current.parent.end === node.end &&
185166
!isListElement(current.parent, current)) {
186167
current = current.parent;
187168
}
188169

189-
return current;
170+
return formatNodeLines(current, sourceFile, options, rulesProvider, formattingRequestKind);
190171
}
191172

192173
// Returns true if node is a element in some list in parent
@@ -338,7 +319,7 @@ namespace ts.formatting {
338319
}
339320

340321
/* @internal */
341-
export function formatNode(node: Node, sourceFileLike: SourceFileLike, languageVariant: LanguageVariant, initialIndentation: number, delta: number, rulesProvider: RulesProvider): TextChange[] {
322+
export function formatNodeGivenIndentation(node: Node, sourceFileLike: SourceFileLike, languageVariant: LanguageVariant, initialIndentation: number, delta: number, rulesProvider: RulesProvider): TextChange[] {
342323
const range = { pos: 0, end: sourceFileLike.text.length };
343324
return formatSpanWorker(
344325
range,
@@ -353,6 +334,19 @@ namespace ts.formatting {
353334
sourceFileLike);
354335
}
355336

337+
function formatNodeLines(node: Node, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] {
338+
if (!node) {
339+
return [];
340+
}
341+
342+
const span = {
343+
pos: getLineStartPositionForPosition(node.getStart(sourceFile), sourceFile),
344+
end: node.end
345+
};
346+
347+
return formatSpan(span, sourceFile, options, rulesProvider, requestKind);
348+
}
349+
356350
function formatSpan(originalRange: TextRange,
357351
sourceFile: SourceFile,
358352
options: FormatCodeSettings,

‎src/services/formatting/formattingContext.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ namespace ts.formatting {
4747
return this.contextNodeAllOnSameLine;
4848
}
4949

50+
public NextNodeAllOnSameLine(): boolean {
51+
if (this.nextNodeAllOnSameLine === undefined) {
52+
this.nextNodeAllOnSameLine = this.NodeIsOnOneLine(this.nextTokenParent);
53+
}
54+
55+
return this.nextNodeAllOnSameLine;
56+
}
57+
5058
public TokensAreOnSameLine(): boolean {
5159
if (this.tokensAreOnSameLine === undefined) {
5260
const startLine = this.sourceFile.getLineAndCharacterOfPosition(this.currentTokenSpan.pos).line;

‎src/services/formatting/rules.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ namespace ts.formatting {
654654

655655
// This check is done before an open brace in a control construct, a function, or a typescript block declaration
656656
static IsBeforeMultilineBlockContext(context: FormattingContext): boolean {
657-
return Rules.IsBeforeBlockContext(context) && !(context.NextNodeBlockIsOnOneLine());
657+
return Rules.IsBeforeBlockContext(context) && !(context.NextNodeAllOnSameLine() || context.NextNodeBlockIsOnOneLine());
658658
}
659659

660660
static IsMultilineBlockContext(context: FormattingContext): boolean {

‎src/services/textChanges.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ namespace ts.textChanges {
512512
lineMap,
513513
getLineAndCharacterOfPosition: pos => computeLineAndCharacterOfPosition(lineMap, pos)
514514
};
515-
const changes = formatting.formatNode(nonFormattedText.node, file, sourceFile.languageVariant, initialIndentation, delta, rulesProvider);
515+
const changes = formatting.formatNodeGivenIndentation(nonFormattedText.node, file, sourceFile.languageVariant, initialIndentation, delta, rulesProvider);
516516
return applyChanges(nonFormattedText.text, changes);
517517
}
518518

‎tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@
77
format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", true);
88
goTo.marker("0");
99
edit.insertLine("");
10+
verify.currentFileContentIs(
11+
`if (true)
12+
{
13+
}
14+
if(false){
15+
}`);
1016
goTo.marker("1");
1117
edit.insertLine("");
1218
verify.currentFileContentIs(

‎tests/cases/fourslash/semicolonFormattingNestedStatements.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
goTo.marker("innermost");
1212
edit.insert(";");
1313

14-
// Adding smicolon should format the innermost statement
14+
// Adding semicolon should format the innermost statement
1515
verify.currentLineContentIs(' var x = 0;');
1616

1717
// Also should format any parent statement that is terminated by the semicolon

0 commit comments

Comments
 (0)