From ae87db7b3e7031431b4fd62cc74877db2b5b5009 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 20 Sep 2017 16:26:46 -0700 Subject: [PATCH 1/3] getAdjustedStartPosition shouldn't skip to next line when on 1st line --- src/services/textChanges.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index f3ad7df1607..fdfac9e54cc 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -152,7 +152,7 @@ namespace ts.textChanges { return position === Position.Start ? start : fullStart; } // get start position of the line following the line that contains fullstart position - let adjustedStartPosition = getStartPositionOfLine(getLineOfLocalPosition(sourceFile, fullStartLine) + 1, sourceFile); + let adjustedStartPosition = getStartPositionOfLine(getLineOfLocalPosition(sourceFile, fullStartLine) + (fullStart > 0 ? 1 : 0), sourceFile); // skip whitespaces/newlines adjustedStartPosition = skipWhitespacesAndLineBreaks(sourceFile.text, adjustedStartPosition); return getStartPositionOfLine(getLineOfLocalPosition(sourceFile, adjustedStartPosition), sourceFile); From 410f84656dea7487e99983c13a593658fb55292a Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 20 Sep 2017 16:31:28 -0700 Subject: [PATCH 2/3] Update baselines temporarily The loss of comments is not good, but should be fixed when (1) trivia-handling issues are fixed or (2) the reafactorings themselves add a workaround. --- tests/cases/fourslash/extract-method-uniqueName.ts | 3 +-- .../cases/fourslash/server/convertFunctionToEs6Class-server.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/cases/fourslash/extract-method-uniqueName.ts b/tests/cases/fourslash/extract-method-uniqueName.ts index 44f026ae8a8..5c359b23164 100644 --- a/tests/cases/fourslash/extract-method-uniqueName.ts +++ b/tests/cases/fourslash/extract-method-uniqueName.ts @@ -9,8 +9,7 @@ edit.applyRefactor({ actionName: "scope_0", actionDescription: "Extract to function in global scope", newContent: -`// newFunction -/*RENAME*/newFunction_1(); +`/*RENAME*/newFunction_1(); function newFunction_1() { // newFunction diff --git a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts index 5782b4a1f41..83a05b0661a 100644 --- a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts +++ b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts @@ -13,8 +13,7 @@ verify.applicableRefactorAvailableAtMarker('1'); verify.fileAfterApplyingRefactorAtMarker('1', -`// Comment -class fn { +`class fn { constructor() { this.baz = 10; } From 3cc0aeb6be1652c08b3803a29fd0a6dedc6faf69 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 21 Sep 2017 09:44:51 -0700 Subject: [PATCH 3/3] PR comments I plan to fix the missing comment issue when I add the convert-jsdoc-types-to-typescript-types refactoring. Or at least work around it. --- src/services/textChanges.ts | 4 +++- tests/cases/fourslash/extract-method-uniqueName.ts | 2 ++ .../fourslash/server/convertFunctionToEs6Class-server.ts | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index fdfac9e54cc..5a01752f38e 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -152,7 +152,9 @@ namespace ts.textChanges { return position === Position.Start ? start : fullStart; } // get start position of the line following the line that contains fullstart position - let adjustedStartPosition = getStartPositionOfLine(getLineOfLocalPosition(sourceFile, fullStartLine) + (fullStart > 0 ? 1 : 0), sourceFile); + // (but only if the fullstart isn't the very beginning of the file) + const nextLineStart = fullStart > 0 ? 1 : 0; + let adjustedStartPosition = getStartPositionOfLine(getLineOfLocalPosition(sourceFile, fullStartLine) + nextLineStart, sourceFile); // skip whitespaces/newlines adjustedStartPosition = skipWhitespacesAndLineBreaks(sourceFile.text, adjustedStartPosition); return getStartPositionOfLine(getLineOfLocalPosition(sourceFile, adjustedStartPosition), sourceFile); diff --git a/tests/cases/fourslash/extract-method-uniqueName.ts b/tests/cases/fourslash/extract-method-uniqueName.ts index 5c359b23164..8f024ad6a47 100644 --- a/tests/cases/fourslash/extract-method-uniqueName.ts +++ b/tests/cases/fourslash/extract-method-uniqueName.ts @@ -3,6 +3,8 @@ ////// newFunction /////*start*/1 + 1/*end*/; +// NOTE: '// newFunction' should be included, but due to incorrect handling of trivia, +// it's omitted right now. goTo.select('start', 'end') edit.applyRefactor({ refactorName: "Extract Method", diff --git a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts index 83a05b0661a..437bf6dadf4 100644 --- a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts +++ b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts @@ -12,6 +12,8 @@ //// } verify.applicableRefactorAvailableAtMarker('1'); +// NOTE: '// Comment' should be included, but due to incorrect handling of trivia, +// it's omitted right now. verify.fileAfterApplyingRefactorAtMarker('1', `class fn { constructor() {