From db5b1b387c204da6f8b1e06222163c0bbf2fa821 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 23 Feb 2015 14:30:25 -0800 Subject: [PATCH 1/2] use character instead of column when formatting multiline comments with tabs --- src/harness/fourslash.ts | 23 ++++++++++++ src/services/formatting/formatting.ts | 16 ++++----- src/services/formatting/smartIndenter.ts | 13 +++++-- .../formattingMultilineCommentsWithTabs1.ts | 29 +++++++++++++++ .../formattingMultilineCommentsWithTabs2.ts | 36 +++++++++++++++++++ tests/cases/fourslash/fourslash.ts | 27 ++++++++++++++ 6 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/formattingMultilineCommentsWithTabs1.ts create mode 100644 tests/cases/fourslash/formattingMultilineCommentsWithTabs2.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 59fc2c23f5c79..c022d4a95b92d 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1480,6 +1480,29 @@ module FourSlash { return runningOffset; } + public copyFormatOptions(): ts.FormatCodeOptions { + return { + ConvertTabsToSpaces: this.formatCodeOptions.ConvertTabsToSpaces, + IndentSize: this.formatCodeOptions.IndentSize, + InsertSpaceAfterCommaDelimiter: this.formatCodeOptions.InsertSpaceAfterCommaDelimiter, + InsertSpaceAfterFunctionKeywordForAnonymousFunctions: this.formatCodeOptions.InsertSpaceAfterFunctionKeywordForAnonymousFunctions, + InsertSpaceAfterKeywordsInControlFlowStatements: this.formatCodeOptions.InsertSpaceAfterKeywordsInControlFlowStatements, + InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: this.formatCodeOptions.InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis, + InsertSpaceAfterSemicolonInForStatements: this.formatCodeOptions.InsertSpaceAfterSemicolonInForStatements, + InsertSpaceBeforeAndAfterBinaryOperators: this.formatCodeOptions.InsertSpaceBeforeAndAfterBinaryOperators, + NewLineCharacter: this.formatCodeOptions.NewLineCharacter, + PlaceOpenBraceOnNewLineForControlBlocks: this.formatCodeOptions.PlaceOpenBraceOnNewLineForControlBlocks, + PlaceOpenBraceOnNewLineForFunctions: this.formatCodeOptions.PlaceOpenBraceOnNewLineForFunctions, + TabSize: this.formatCodeOptions.TabSize + } + } + + public setFormatOptions(formatCodeOptions: ts.FormatCodeOptions): ts.FormatCodeOptions { + var oldFormatCodeOptions = this.formatCodeOptions; + this.formatCodeOptions = formatCodeOptions; + return oldFormatCodeOptions; + } + public formatDocument() { this.scenarioActions.push(''); diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 180ff0c9e9528..dd12b452b57f8 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -842,9 +842,9 @@ module ts.formatting { var startLinePos = getStartPositionOfLine(startLine, sourceFile); var nonWhitespaceColumnInFirstPart = - SmartIndenter.findFirstNonWhitespaceColumn(startLinePos, parts[0].pos, sourceFile, options); + SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(startLinePos, parts[0].pos, sourceFile, options); - if (indentation === nonWhitespaceColumnInFirstPart) { + if (indentation === nonWhitespaceColumnInFirstPart.column) { return; } @@ -855,21 +855,21 @@ module ts.formatting { } // shift all parts on the delta size - var delta = indentation - nonWhitespaceColumnInFirstPart; + var delta = indentation - nonWhitespaceColumnInFirstPart.column; for (var i = startIndex, len = parts.length; i < len; ++i, ++startLine) { var startLinePos = getStartPositionOfLine(startLine, sourceFile); - var nonWhitespaceColumn = + var nonWhitespaceCharacterAndColumn = i === 0 ? nonWhitespaceColumnInFirstPart - : SmartIndenter.findFirstNonWhitespaceColumn(parts[i].pos, parts[i].end, sourceFile, options); + : SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(parts[i].pos, parts[i].end, sourceFile, options); - var newIndentation = nonWhitespaceColumn + delta; + var newIndentation = nonWhitespaceCharacterAndColumn.column + delta; if (newIndentation > 0) { var indentationString = getIndentationString(newIndentation, options); - recordReplace(startLinePos, nonWhitespaceColumn, indentationString); + recordReplace(startLinePos, nonWhitespaceCharacterAndColumn.character, indentationString); } else { - recordDelete(startLinePos, nonWhitespaceColumn); + recordDelete(startLinePos, nonWhitespaceCharacterAndColumn.character); } } } diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 04849420e70ff..89bf9e674ee81 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -306,12 +306,13 @@ module ts.formatting { return findFirstNonWhitespaceColumn(lineStart, lineStart + lineAndCharacter.character, sourceFile, options); } - export function findFirstNonWhitespaceColumn(startPos: number, endPos: number, sourceFile: SourceFile, options: EditorOptions): number { + export function findFirstNonWhitespaceCharacterAndColumn(startPos: number, endPos: number, sourceFile: SourceFile, options: EditorOptions) { + var character = 0; var column = 0; for (var pos = startPos; pos < endPos; ++pos) { var ch = sourceFile.text.charCodeAt(pos); if (!isWhiteSpace(ch)) { - return column; + break; } if (ch === CharacterCodes.tab) { @@ -320,8 +321,14 @@ module ts.formatting { else { column++; } + + character++; } - return column; + return { column, character }; + } + + export function findFirstNonWhitespaceColumn(startPos: number, endPos: number, sourceFile: SourceFile, options: EditorOptions): number { + return findFirstNonWhitespaceCharacterAndColumn(startPos, endPos, sourceFile, options).column; } function nodeContentIsAlwaysIndented(kind: SyntaxKind): boolean { diff --git a/tests/cases/fourslash/formattingMultilineCommentsWithTabs1.ts b/tests/cases/fourslash/formattingMultilineCommentsWithTabs1.ts new file mode 100644 index 0000000000000..967a0f1ee0da9 --- /dev/null +++ b/tests/cases/fourslash/formattingMultilineCommentsWithTabs1.ts @@ -0,0 +1,29 @@ +/// + +////var f = function (j) { +//// +//// switch (j) { +//// case 1: +/////*1*/ /* when current checkbox has focus, Firefox has changed check state already +/////*2*/ on SPACE bar press only +/////*3*/ IE does not have issue, use the CSS class +/////*4*/ input:focus[type=checkbox] (z-index = 31290) +/////*5*/ to determine whether checkbox has focus or not +//// */ +//// break; +//// case 2: +//// break; +//// } +////} +format.document(); +goTo.marker("1"); +verify.currentLineContentIs(" /* when current checkbox has focus, Firefox has changed check state already"); +goTo.marker("2"); +verify.currentLineContentIs(" on SPACE bar press only"); +goTo.marker("3"); +verify.currentLineContentIs(" IE does not have issue, use the CSS class"); +goTo.marker("4"); +verify.currentLineContentIs(" input:focus[type=checkbox] (z-index = 31290)"); +goTo.marker("5"); +verify.currentLineContentIs(" to determine whether checkbox has focus or not"); + diff --git a/tests/cases/fourslash/formattingMultilineCommentsWithTabs2.ts b/tests/cases/fourslash/formattingMultilineCommentsWithTabs2.ts new file mode 100644 index 0000000000000..e66934c6cebc7 --- /dev/null +++ b/tests/cases/fourslash/formattingMultilineCommentsWithTabs2.ts @@ -0,0 +1,36 @@ +/// + +////var f = function (j) { +//// +//// switch (j) { +//// case 1: +/////*1*/ /* when current checkbox has focus, Firefox has changed check state already +/////*2*/ on SPACE bar press only +/////*3*/ IE does not have issue, use the CSS class +/////*4*/ input:focus[type=checkbox] (z-index = 31290) +/////*5*/ to determine whether checkbox has focus or not +//// */ +//// break; +//// case 2: +//// break; +//// } +////} +var options = format.copyFormatOptions(); +options.ConvertTabsToSpaces = false; +var oldOptions = format.setFormatOptions(options); +try { + format.document(); + goTo.marker("1"); + verify.currentLineContentIs(" /* when current checkbox has focus, Firefox has changed check state already"); + goTo.marker("2"); + verify.currentLineContentIs(" on SPACE bar press only"); + goTo.marker("3"); + verify.currentLineContentIs(" IE does not have issue, use the CSS class"); + goTo.marker("4"); + verify.currentLineContentIs(" input:focus[type=checkbox] (z-index = 31290)"); + goTo.marker("5"); + verify.currentLineContentIs(" to determine whether checkbox has focus or not"); +} +finally { + format.setFormatOptions(oldOptions); +} diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 4091815ce5973..087c43786f0a6 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -49,6 +49,25 @@ module FourSlashInterface { position: number; data?: any; } + + export interface EditorOptions { + IndentSize: number; + TabSize: number; + NewLineCharacter: string; + ConvertTabsToSpaces: boolean; + } + + export interface FormatCodeOptions extends EditorOptions { + InsertSpaceAfterCommaDelimiter: boolean; + InsertSpaceAfterSemicolonInForStatements: boolean; + InsertSpaceBeforeAndAfterBinaryOperators: boolean; + InsertSpaceAfterKeywordsInControlFlowStatements: boolean; + InsertSpaceAfterFunctionKeywordForAnonymousFunctions: boolean; + InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: boolean; + PlaceOpenBraceOnNewLineForFunctions: boolean; + PlaceOpenBraceOnNewLineForControlBlocks: boolean; + [s: string]: boolean | number| string; + } export interface Range { fileName: string; @@ -541,6 +560,14 @@ module FourSlashInterface { FourSlash.currentTestState.formatDocument(); } + public copyFormatOptions(): FormatCodeOptions { + return FourSlash.currentTestState.copyFormatOptions(); + } + + public setFormatOptions(options: FormatCodeOptions) { + return FourSlash.currentTestState.setFormatOptions(options); + } + public selection(startMarker: string, endMarker: string) { FourSlash.currentTestState.formatSelection(FourSlash.currentTestState.getMarkerByName(startMarker).position, FourSlash.currentTestState.getMarkerByName(endMarker).position); } From 3119839d55d1d4569d001e67f4e4185b3478f73e Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 23 Feb 2015 22:21:03 -0800 Subject: [PATCH 2/2] addressed CR feedback --- src/harness/fourslash.ts | 15 +-------------- src/services/formatting/smartIndenter.ts | 7 +++++++ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index c022d4a95b92d..d8b7c358a5548 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1481,20 +1481,7 @@ module FourSlash { } public copyFormatOptions(): ts.FormatCodeOptions { - return { - ConvertTabsToSpaces: this.formatCodeOptions.ConvertTabsToSpaces, - IndentSize: this.formatCodeOptions.IndentSize, - InsertSpaceAfterCommaDelimiter: this.formatCodeOptions.InsertSpaceAfterCommaDelimiter, - InsertSpaceAfterFunctionKeywordForAnonymousFunctions: this.formatCodeOptions.InsertSpaceAfterFunctionKeywordForAnonymousFunctions, - InsertSpaceAfterKeywordsInControlFlowStatements: this.formatCodeOptions.InsertSpaceAfterKeywordsInControlFlowStatements, - InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: this.formatCodeOptions.InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis, - InsertSpaceAfterSemicolonInForStatements: this.formatCodeOptions.InsertSpaceAfterSemicolonInForStatements, - InsertSpaceBeforeAndAfterBinaryOperators: this.formatCodeOptions.InsertSpaceBeforeAndAfterBinaryOperators, - NewLineCharacter: this.formatCodeOptions.NewLineCharacter, - PlaceOpenBraceOnNewLineForControlBlocks: this.formatCodeOptions.PlaceOpenBraceOnNewLineForControlBlocks, - PlaceOpenBraceOnNewLineForFunctions: this.formatCodeOptions.PlaceOpenBraceOnNewLineForFunctions, - TabSize: this.formatCodeOptions.TabSize - } + return ts.clone(this.formatCodeOptions); } public setFormatOptions(formatCodeOptions: ts.FormatCodeOptions): ts.FormatCodeOptions { diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 89bf9e674ee81..593e6163e297b 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -306,6 +306,13 @@ module ts.formatting { return findFirstNonWhitespaceColumn(lineStart, lineStart + lineAndCharacter.character, sourceFile, options); } + /* + Character is the actual index of the character since the beginning of the line. + Column - position of the character after expanding tabs to spaces + "0\t2$" + value of 'character' for '$' is 3 + value of 'column' for '$' is 6 (assuming that tab size is 4) + */ export function findFirstNonWhitespaceCharacterAndColumn(startPos: number, endPos: number, sourceFile: SourceFile, options: EditorOptions) { var character = 0; var column = 0;