From 0632d0c38c286e44d0324f3117f92a102ca73401 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 29 Aug 2014 13:08:48 -0700 Subject: [PATCH] Addressed CR feedback, no longer highlighting elseifs with comments between. --- src/services/services.ts | 50 ++++++++++--------- tests/cases/fourslash/getOccurrencesIfElse.ts | 4 +- .../cases/fourslash/getOccurrencesIfElse2.ts | 2 +- .../cases/fourslash/getOccurrencesIfElse3.ts | 2 +- .../cases/fourslash/getOccurrencesIfElse4.ts | 2 +- .../getOccurrencesIfElseNegatives.ts | 8 +-- 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index ccc4924a9cc..ca59140ffb8 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2210,7 +2210,15 @@ module ts { // Now traverse back down through the else branches, aggregating if/else keywords of if-statements. while (ifStatement) { - pushIfAndElseKeywords(); + var children = ifStatement.getChildren(); + pushKeywordIf(keywords, children[0], SyntaxKind.IfKeyword); + + // Generally the 'else' keyword is second-to-last, so we traverse backwards. + for (var i = children.length - 1; i >= 0; i--) { + if (pushKeywordIf(keywords, children[i], SyntaxKind.ElseKeyword)) { + break; + } + } if (!hasKind(ifStatement.elseStatement, SyntaxKind.IfStatement)) { break @@ -2221,42 +2229,36 @@ module ts { var result: ReferenceEntry[] = []; - // Here we do a little extra. - // We'd like to make else/ifs on the same line to be highlighted together + // We'd like to highlight else/ifs together if they are only separated by spaces/tabs + // (i.e. the keywords are separated by no comments, no newlines). for (var i = 0; i < keywords.length; i++) { if (keywords[i].kind === SyntaxKind.ElseKeyword && i < keywords.length - 1) { var elseKeyword = keywords[i]; var ifKeyword = keywords[i + 1]; // this *should* always be an 'if' keyword. - // Ensure that the keywords are only separated by trivia. - if (elseKeyword.end === ifKeyword.getFullStart()) { - var elseLine = sourceFile.getLineAndCharacterFromPosition(elseKeyword.end); - var ifLine = sourceFile.getLineAndCharacterFromPosition(ifKeyword.getStart()); + var shouldHighlightNextKeyword = true; - if (elseLine.line === ifLine.line) { - result.push(new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(elseKeyword.getStart(), ifKeyword.end), /* isWriteAccess */ false)); - i++; // skip the next keyword - continue; + // Avoid recalculating getStart() by iterating backwards. + for (var j = ifKeyword.getStart() - 1; j >= elseKeyword.end; j--) { + var c = sourceFile.text.charCodeAt(j); + if (c !== CharacterCodes.space && c !== CharacterCodes.tab) { + shouldHighlightNextKeyword = false; + break; } } + + if (shouldHighlightNextKeyword) { + result.push(new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(elseKeyword.getStart(), ifKeyword.end), /* isWriteAccess */ false)); + i++; // skip the next keyword + continue; + } } + // Ordinary case: just highlight the keyword. result.push(keywordToReferenceEntry(keywords[i])); } return result; - - function pushIfAndElseKeywords() { - var children = ifStatement.getChildren(); - pushKeywordIf(keywords, children[0], SyntaxKind.IfKeyword); - - // Generally the 'else' keyword is second-to-last, so we traverse backwards. - for (var i = children.length - 1; i >= 0; i--) { - if (pushKeywordIf(keywords, children[i], SyntaxKind.ElseKeyword)) { - break; - } - } - } } function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { @@ -2350,7 +2352,7 @@ module ts { } function pushKeywordIf(keywordList: Node[], token: Node, ...expected: SyntaxKind[]): boolean { - if (token && contains(expected, token.kind)) { + if (token && contains(expected, token.kind)) { keywordList.push(token); return true; } diff --git a/tests/cases/fourslash/getOccurrencesIfElse.ts b/tests/cases/fourslash/getOccurrencesIfElse.ts index ec9533bf5be..96c70d404ab 100644 --- a/tests/cases/fourslash/getOccurrencesIfElse.ts +++ b/tests/cases/fourslash/getOccurrencesIfElse.ts @@ -13,9 +13,9 @@ //// var x = undefined; //// } ////} -////[|else i/**/f|] (null) { +////[|else i/**/f|] (null) { ////} -////[|else /* whar garbl */ if|] (undefined) { +////[|else|] /* whar garbl */ [|if|] (undefined) { ////} ////[|else|] ////[|if|] (false) { diff --git a/tests/cases/fourslash/getOccurrencesIfElse2.ts b/tests/cases/fourslash/getOccurrencesIfElse2.ts index 76df97179c4..012e7a96efd 100644 --- a/tests/cases/fourslash/getOccurrencesIfElse2.ts +++ b/tests/cases/fourslash/getOccurrencesIfElse2.ts @@ -13,7 +13,7 @@ //// var x = undefined; //// } ////} -////else if (null) { +////else if (null) { ////} ////else /* whar garbl */ if (undefined) { ////} diff --git a/tests/cases/fourslash/getOccurrencesIfElse3.ts b/tests/cases/fourslash/getOccurrencesIfElse3.ts index 92a31800e92..b0ca3615648 100644 --- a/tests/cases/fourslash/getOccurrencesIfElse3.ts +++ b/tests/cases/fourslash/getOccurrencesIfElse3.ts @@ -13,7 +13,7 @@ //// var x = undefined; //// } ////} -////else if (null) { +////else if (null) { ////} ////else /* whar garbl */ if (undefined) { ////} diff --git a/tests/cases/fourslash/getOccurrencesIfElse4.ts b/tests/cases/fourslash/getOccurrencesIfElse4.ts index c110521b93a..66746252741 100644 --- a/tests/cases/fourslash/getOccurrencesIfElse4.ts +++ b/tests/cases/fourslash/getOccurrencesIfElse4.ts @@ -13,7 +13,7 @@ //// var x = undefined; //// } ////} -////else if (null) { +////else if (null) { ////} ////else /* whar garbl */ if (undefined) { ////} diff --git a/tests/cases/fourslash/getOccurrencesIfElseNegatives.ts b/tests/cases/fourslash/getOccurrencesIfElseNegatives.ts index 68cbf5f1875..601eba63c64 100644 --- a/tests/cases/fourslash/getOccurrencesIfElseNegatives.ts +++ b/tests/cases/fourslash/getOccurrencesIfElseNegatives.ts @@ -13,7 +13,7 @@ //// var x = undefined; //// } ////} -////else/*8*/ if (null) { +////else/*8*/ if (null) { ////} ////else/*9*/ /* whar garbl */ if/*10*/ (undefined) { ////} @@ -23,7 +23,7 @@ ////else/*13*/ { } -for (var i = 1; i <= test.markers().length; i++) { - goTo.marker("" + i); +test.markers().forEach(m => { + goTo.position(m.position, m.fileName) verify.occurrencesAtPositionCount(0); -} +});