Skip to content

Commit 0632d0c

Browse files
Addressed CR feedback, no longer highlighting elseifs with comments between.
1 parent 24f6e41 commit 0632d0c

File tree

6 files changed

+35
-33
lines changed

6 files changed

+35
-33
lines changed

src/services/services.ts

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,7 +2210,15 @@ module ts {
22102210

22112211
// Now traverse back down through the else branches, aggregating if/else keywords of if-statements.
22122212
while (ifStatement) {
2213-
pushIfAndElseKeywords();
2213+
var children = ifStatement.getChildren();
2214+
pushKeywordIf(keywords, children[0], SyntaxKind.IfKeyword);
2215+
2216+
// Generally the 'else' keyword is second-to-last, so we traverse backwards.
2217+
for (var i = children.length - 1; i >= 0; i--) {
2218+
if (pushKeywordIf(keywords, children[i], SyntaxKind.ElseKeyword)) {
2219+
break;
2220+
}
2221+
}
22142222

22152223
if (!hasKind(ifStatement.elseStatement, SyntaxKind.IfStatement)) {
22162224
break
@@ -2221,42 +2229,36 @@ module ts {
22212229

22222230
var result: ReferenceEntry[] = [];
22232231

2224-
// Here we do a little extra.
2225-
// We'd like to make else/ifs on the same line to be highlighted together
2232+
// We'd like to highlight else/ifs together if they are only separated by spaces/tabs
2233+
// (i.e. the keywords are separated by no comments, no newlines).
22262234
for (var i = 0; i < keywords.length; i++) {
22272235
if (keywords[i].kind === SyntaxKind.ElseKeyword && i < keywords.length - 1) {
22282236
var elseKeyword = keywords[i];
22292237
var ifKeyword = keywords[i + 1]; // this *should* always be an 'if' keyword.
22302238

2231-
// Ensure that the keywords are only separated by trivia.
2232-
if (elseKeyword.end === ifKeyword.getFullStart()) {
2233-
var elseLine = sourceFile.getLineAndCharacterFromPosition(elseKeyword.end);
2234-
var ifLine = sourceFile.getLineAndCharacterFromPosition(ifKeyword.getStart());
2239+
var shouldHighlightNextKeyword = true;
22352240

2236-
if (elseLine.line === ifLine.line) {
2237-
result.push(new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(elseKeyword.getStart(), ifKeyword.end), /* isWriteAccess */ false));
2238-
i++; // skip the next keyword
2239-
continue;
2241+
// Avoid recalculating getStart() by iterating backwards.
2242+
for (var j = ifKeyword.getStart() - 1; j >= elseKeyword.end; j--) {
2243+
var c = sourceFile.text.charCodeAt(j);
2244+
if (c !== CharacterCodes.space && c !== CharacterCodes.tab) {
2245+
shouldHighlightNextKeyword = false;
2246+
break;
22402247
}
22412248
}
2249+
2250+
if (shouldHighlightNextKeyword) {
2251+
result.push(new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(elseKeyword.getStart(), ifKeyword.end), /* isWriteAccess */ false));
2252+
i++; // skip the next keyword
2253+
continue;
2254+
}
22422255
}
22432256

2257+
// Ordinary case: just highlight the keyword.
22442258
result.push(keywordToReferenceEntry(keywords[i]));
22452259
}
22462260

22472261
return result;
2248-
2249-
function pushIfAndElseKeywords() {
2250-
var children = ifStatement.getChildren();
2251-
pushKeywordIf(keywords, children[0], SyntaxKind.IfKeyword);
2252-
2253-
// Generally the 'else' keyword is second-to-last, so we traverse backwards.
2254-
for (var i = children.length - 1; i >= 0; i--) {
2255-
if (pushKeywordIf(keywords, children[i], SyntaxKind.ElseKeyword)) {
2256-
break;
2257-
}
2258-
}
2259-
}
22602262
}
22612263

22622264
function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] {
@@ -2350,7 +2352,7 @@ module ts {
23502352
}
23512353

23522354
function pushKeywordIf(keywordList: Node[], token: Node, ...expected: SyntaxKind[]): boolean {
2353-
if (token && contains(<SyntaxKind[]>expected, token.kind)) {
2355+
if (token && contains(expected, token.kind)) {
23542356
keywordList.push(token);
23552357
return true;
23562358
}

tests/cases/fourslash/getOccurrencesIfElse.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
//// var x = undefined;
1414
//// }
1515
////}
16-
////[|else i/**/f|] (null) {
16+
////[|else i/**/f|] (null) {
1717
////}
18-
////[|else /* whar garbl */ if|] (undefined) {
18+
////[|else|] /* whar garbl */ [|if|] (undefined) {
1919
////}
2020
////[|else|]
2121
////[|if|] (false) {

tests/cases/fourslash/getOccurrencesIfElse2.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//// var x = undefined;
1414
//// }
1515
////}
16-
////else if (null) {
16+
////else if (null) {
1717
////}
1818
////else /* whar garbl */ if (undefined) {
1919
////}

tests/cases/fourslash/getOccurrencesIfElse3.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//// var x = undefined;
1414
//// }
1515
////}
16-
////else if (null) {
16+
////else if (null) {
1717
////}
1818
////else /* whar garbl */ if (undefined) {
1919
////}

tests/cases/fourslash/getOccurrencesIfElse4.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//// var x = undefined;
1414
//// }
1515
////}
16-
////else if (null) {
16+
////else if (null) {
1717
////}
1818
////else /* whar garbl */ if (undefined) {
1919
////}

tests/cases/fourslash/getOccurrencesIfElseNegatives.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//// var x = undefined;
1414
//// }
1515
////}
16-
////else/*8*/ if (null) {
16+
////else/*8*/ if (null) {
1717
////}
1818
////else/*9*/ /* whar garbl */ if/*10*/ (undefined) {
1919
////}
@@ -23,7 +23,7 @@
2323
////else/*13*/ { }
2424

2525

26-
for (var i = 1; i <= test.markers().length; i++) {
27-
goTo.marker("" + i);
26+
test.markers().forEach(m => {
27+
goTo.position(m.position, m.fileName)
2828
verify.occurrencesAtPositionCount(0);
29-
}
29+
});

0 commit comments

Comments
 (0)