Fix TextMate missing item #8251
Conversation
|
I believe this should fix both https://devdiv.visualstudio.com/DevDiv/_build?definitionId=15591&_a=summary and https://dev.azure.com/dnceng/internal/_build?definitionId=262&_a=summary. |
| @@ -96,15 +96,19 @@ public async Task VerifyGetClassificationsAsync(IEnumerable<ClassificationSpan> | |||
| var expectedArray = expectedClassifications.ToArray(); | |||
| var actualOffset = 0; | |||
|
|
|||
| for (var i = 0; i < actualArray.Length; i++) | |||
| for (var i = 0; i < expectedArray.Length; i++) | |||
There was a problem hiding this comment.
I think if actualClassifications << expectedClassifications, this could still throw, but I see how if actual contains "Syntactic" and expected does not, then the new code is the much more likely to succeed state. You could also have (theoretically) a scenario where there are trailing syntactic classifications causes the check on L131 to fail.
Would it make sense to filter actualArray into actualSemanticClassifications with the clause on L104 (Linq.Where or similar), check the lengths, and then proceed through the equal length arrays?
There was a problem hiding this comment.
I like the Where suggestion, much cleaner. I still prefer that the length is checked at the bottom so that we proceed through the arrays before checking length (ie, finding out what area is messed up is more informative than finding out the difference in expected length).
Summary of the changes