Skip to content

Commit 545e8f4

Browse files
vladimavladima
committed
Merge pull request #2117 from Microsoft/formattingTabsInMultilineComments
use character instead of column when formatting multiline comments with ...
2 parents 55dafb5 + 3119839 commit 545e8f4

File tree

6 files changed

+127
-11
lines changed

6 files changed

+127
-11
lines changed

‎src/harness/fourslash.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1480,6 +1480,16 @@ module FourSlash {
14801480
return runningOffset;
14811481
}
14821482

1483+
public copyFormatOptions(): ts.FormatCodeOptions {
1484+
return ts.clone(this.formatCodeOptions);
1485+
}
1486+
1487+
public setFormatOptions(formatCodeOptions: ts.FormatCodeOptions): ts.FormatCodeOptions {
1488+
var oldFormatCodeOptions = this.formatCodeOptions;
1489+
this.formatCodeOptions = formatCodeOptions;
1490+
return oldFormatCodeOptions;
1491+
}
1492+
14831493
public formatDocument() {
14841494
this.scenarioActions.push('<FormatDocument />');
14851495

‎src/services/formatting/formatting.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -842,9 +842,9 @@ module ts.formatting {
842842
var startLinePos = getStartPositionOfLine(startLine, sourceFile);
843843

844844
var nonWhitespaceColumnInFirstPart =
845-
SmartIndenter.findFirstNonWhitespaceColumn(startLinePos, parts[0].pos, sourceFile, options);
845+
SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(startLinePos, parts[0].pos, sourceFile, options);
846846

847-
if (indentation === nonWhitespaceColumnInFirstPart) {
847+
if (indentation === nonWhitespaceColumnInFirstPart.column) {
848848
return;
849849
}
850850

@@ -855,21 +855,21 @@ module ts.formatting {
855855
}
856856

857857
// shift all parts on the delta size
858-
var delta = indentation - nonWhitespaceColumnInFirstPart;
858+
var delta = indentation - nonWhitespaceColumnInFirstPart.column;
859859
for (var i = startIndex, len = parts.length; i < len; ++i, ++startLine) {
860860
var startLinePos = getStartPositionOfLine(startLine, sourceFile);
861-
var nonWhitespaceColumn =
861+
var nonWhitespaceCharacterAndColumn =
862862
i === 0
863863
? nonWhitespaceColumnInFirstPart
864-
: SmartIndenter.findFirstNonWhitespaceColumn(parts[i].pos, parts[i].end, sourceFile, options);
864+
: SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(parts[i].pos, parts[i].end, sourceFile, options);
865865

866-
var newIndentation = nonWhitespaceColumn + delta;
866+
var newIndentation = nonWhitespaceCharacterAndColumn.column + delta;
867867
if (newIndentation > 0) {
868868
var indentationString = getIndentationString(newIndentation, options);
869-
recordReplace(startLinePos, nonWhitespaceColumn, indentationString);
869+
recordReplace(startLinePos, nonWhitespaceCharacterAndColumn.character, indentationString);
870870
}
871871
else {
872-
recordDelete(startLinePos, nonWhitespaceColumn);
872+
recordDelete(startLinePos, nonWhitespaceCharacterAndColumn.character);
873873
}
874874
}
875875
}

‎src/services/formatting/smartIndenter.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,12 +306,20 @@ module ts.formatting {
306306
return findFirstNonWhitespaceColumn(lineStart, lineStart + lineAndCharacter.character, sourceFile, options);
307307
}
308308

309-
export function findFirstNonWhitespaceColumn(startPos: number, endPos: number, sourceFile: SourceFile, options: EditorOptions): number {
309+
/*
310+
Character is the actual index of the character since the beginning of the line.
311+
Column - position of the character after expanding tabs to spaces
312+
"0\t2$"
313+
value of 'character' for '$' is 3
314+
value of 'column' for '$' is 6 (assuming that tab size is 4)
315+
*/
316+
export function findFirstNonWhitespaceCharacterAndColumn(startPos: number, endPos: number, sourceFile: SourceFile, options: EditorOptions) {
317+
var character = 0;
310318
var column = 0;
311319
for (var pos = startPos; pos < endPos; ++pos) {
312320
var ch = sourceFile.text.charCodeAt(pos);
313321
if (!isWhiteSpace(ch)) {
314-
return column;
322+
break;
315323
}
316324

317325
if (ch === CharacterCodes.tab) {
@@ -320,8 +328,14 @@ module ts.formatting {
320328
else {
321329
column++;
322330
}
331+
332+
character++;
323333
}
324-
return column;
334+
return { column, character };
335+
}
336+
337+
export function findFirstNonWhitespaceColumn(startPos: number, endPos: number, sourceFile: SourceFile, options: EditorOptions): number {
338+
return findFirstNonWhitespaceCharacterAndColumn(startPos, endPos, sourceFile, options).column;
325339
}
326340

327341
function nodeContentIsAlwaysIndented(kind: SyntaxKind): boolean {

‎tests/cases/fourslash/formattingMultilineCommentsWithTabs1.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////var f = function (j) {
4+
////
5+
//// switch (j) {
6+
//// case 1:
7+
/////*1*/ /* when current checkbox has focus, Firefox has changed check state already
8+
/////*2*/ on SPACE bar press only
9+
/////*3*/ IE does not have issue, use the CSS class
10+
/////*4*/ input:focus[type=checkbox] (z-index = 31290)
11+
/////*5*/ to determine whether checkbox has focus or not
12+
//// */
13+
//// break;
14+
//// case 2:
15+
//// break;
16+
//// }
17+
////}
18+
format.document();
19+
goTo.marker("1");
20+
verify.currentLineContentIs(" /* when current checkbox has focus, Firefox has changed check state already");
21+
goTo.marker("2");
22+
verify.currentLineContentIs(" on SPACE bar press only");
23+
goTo.marker("3");
24+
verify.currentLineContentIs(" IE does not have issue, use the CSS class");
25+
goTo.marker("4");
26+
verify.currentLineContentIs(" input:focus[type=checkbox] (z-index = 31290)");
27+
goTo.marker("5");
28+
verify.currentLineContentIs(" to determine whether checkbox has focus or not");
29+

‎tests/cases/fourslash/formattingMultilineCommentsWithTabs2.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////var f = function (j) {
4+
////
5+
//// switch (j) {
6+
//// case 1:
7+
/////*1*/ /* when current checkbox has focus, Firefox has changed check state already
8+
/////*2*/ on SPACE bar press only
9+
/////*3*/ IE does not have issue, use the CSS class
10+
/////*4*/ input:focus[type=checkbox] (z-index = 31290)
11+
/////*5*/ to determine whether checkbox has focus or not
12+
//// */
13+
//// break;
14+
//// case 2:
15+
//// break;
16+
//// }
17+
////}
18+
var options = format.copyFormatOptions();
19+
options.ConvertTabsToSpaces = false;
20+
var oldOptions = format.setFormatOptions(options);
21+
try {
22+
format.document();
23+
goTo.marker("1");
24+
verify.currentLineContentIs(" /* when current checkbox has focus, Firefox has changed check state already");
25+
goTo.marker("2");
26+
verify.currentLineContentIs(" on SPACE bar press only");
27+
goTo.marker("3");
28+
verify.currentLineContentIs(" IE does not have issue, use the CSS class");
29+
goTo.marker("4");
30+
verify.currentLineContentIs(" input:focus[type=checkbox] (z-index = 31290)");
31+
goTo.marker("5");
32+
verify.currentLineContentIs(" to determine whether checkbox has focus or not");
33+
}
34+
finally {
35+
format.setFormatOptions(oldOptions);
36+
}

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,25 @@ module FourSlashInterface {
4949
position: number;
5050
data?: any;
5151
}
52+
53+
export interface EditorOptions {
54+
IndentSize: number;
55+
TabSize: number;
56+
NewLineCharacter: string;
57+
ConvertTabsToSpaces: boolean;
58+
}
59+
60+
export interface FormatCodeOptions extends EditorOptions {
61+
InsertSpaceAfterCommaDelimiter: boolean;
62+
InsertSpaceAfterSemicolonInForStatements: boolean;
63+
InsertSpaceBeforeAndAfterBinaryOperators: boolean;
64+
InsertSpaceAfterKeywordsInControlFlowStatements: boolean;
65+
InsertSpaceAfterFunctionKeywordForAnonymousFunctions: boolean;
66+
InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: boolean;
67+
PlaceOpenBraceOnNewLineForFunctions: boolean;
68+
PlaceOpenBraceOnNewLineForControlBlocks: boolean;
69+
[s: string]: boolean | number| string;
70+
}
5271

5372
export interface Range {
5473
fileName: string;
@@ -541,6 +560,14 @@ module FourSlashInterface {
541560
FourSlash.currentTestState.formatDocument();
542561
}
543562

563+
public copyFormatOptions(): FormatCodeOptions {
564+
return FourSlash.currentTestState.copyFormatOptions();
565+
}
566+
567+
public setFormatOptions(options: FormatCodeOptions) {
568+
return FourSlash.currentTestState.setFormatOptions(options);
569+
}
570+
544571
public selection(startMarker: string, endMarker: string) {
545572
FourSlash.currentTestState.formatSelection(FourSlash.currentTestState.getMarkerByName(startMarker).position, FourSlash.currentTestState.getMarkerByName(endMarker).position);
546573
}

0 commit comments

Comments
 (0)