some updateMarkers polish, more tests, #49924

src/vs/workbench/parts/outline/electron-browser/outlineModel.ts

@@ -143,13 +143,16 @@ export class OutlineGroup extends TreeElement {
 		}
 	}
 
-	private _updateMarker(marker: IMarker[], item: OutlineElement): void {
-		let idx = binarySearch<IRange>(marker, item.symbol.definingRange, Range.compareRangesUsingStarts);
+	private _updateMarker(markers: IMarker[], item: OutlineElement): void {
+
+		item.marker = undefined;
+
+		// find the proper start index to check for item/marker overlap.
+		let idx = binarySearch<IRange>(markers, item.symbol.definingRange, Range.compareRangesUsingStarts);
 		let start: number;
 		if (idx < 0) {
-			// ~idx is the index at which the symbol should be... start search from there
 			start = ~idx;
-			if (start > 0 && Range.areIntersecting(marker[start - 1], item.symbol.definingRange)) {
+			if (start > 0 && Range.areIntersecting(markers[start - 1], item.symbol.definingRange)) {
 				start -= 1;
 			}
 		} else {
@@ -159,35 +162,30 @@ export class OutlineGroup extends TreeElement {
 		let myMarkers: IMarker[] = [];
 		let myTopSev: MarkerSeverity;
 
-		while (start < marker.length) {
-			if (!Range.areIntersecting(marker[start], item.symbol.definingRange)) {
-				break;
-			}
-			// this marker belongs to this element and it takes it away.
-			// children of this marker might take it away again tho...
-			let myMarker = marker.splice(start, 1)[0];
-			myMarkers.push(myMarker);
-			if (!myTopSev || myMarker.severity > myTopSev) {
-				myTopSev = myMarker.severity;
+		while (start < markers.length && Range.areIntersecting(markers[start], item.symbol.definingRange)) {
+			// remove markers intersecting with this outline element
+			// and store them in a 'private' array.
+			let marker = markers.splice(start, 1)[0];
+			myMarkers.push(marker);
+			if (!myTopSev || marker.severity > myTopSev) {
+				myTopSev = marker.severity;
 			}
 		}
 
-		// recursivion into children. this might cause myMarkers to become empty
-		// and because of that we store the top marker to which tell me what the
-		// most severe marker of my children is
+		// Recurse into children and let them match markers that have matched
+		// this outline element. This might remove markers from this element and
+		// therefore we remember that we have had markers. That allows us to render
+		// the dot, saying 'this element has children with markers'
 		for (const key in item.children) {
 			this._updateMarker(myMarkers, item.children[key]);
 		}
 
-		if (!myTopSev) {
-			item.marker = undefined;
-		} else {
+		if (myTopSev) {
 			item.marker = {
 				count: myMarkers.length,
 				topSev: myTopSev
 			};
 		}
-
 	}
 }
 

src/vs/workbench/parts/outline/test/electron-browser/outlineModel.test.ts

@@ -43,6 +43,7 @@ suite('OutlineModel', function () {
 		data.sort(Range.compareRangesUsingStarts); // model does this
 
 		group.updateMarker(data);
+		assert.equal(data.length, 0); // all 'stolen'
 		assert.equal(e0.marker.count, 1);
 		assert.equal(e1.marker, undefined);
 		assert.equal(e2.marker.count, 2);
@@ -53,4 +54,43 @@ suite('OutlineModel', function () {
 		assert.equal(e2.marker, undefined);
 	});
 
+	test('OutlineElement - updateMarker, 2', function () {
+
+		let p = new OutlineElement('A', null, fakeSymbolInformation(new Range(1, 1, 11, 1)));
+		let c1 = new OutlineElement('A/B', null, fakeSymbolInformation(new Range(2, 4, 5, 4)));
+		let c2 = new OutlineElement('A/C', null, fakeSymbolInformation(new Range(6, 4, 9, 4)));
+
+		let group = new OutlineGroup('group', null, null, 1);
+		group.children[p.id] = p;
+		p.children[c1.id] = c1;
+		p.children[c2.id] = c2;
+
+		let data = [
+			fakeMarker(new Range(2, 4, 5, 4))
+		];
+
+		group.updateMarker(data);
+		assert.equal(p.marker.count, 0);
+		assert.equal(c1.marker.count, 1);
+		assert.equal(c2.marker, undefined);
+
+		data = [
+			fakeMarker(new Range(2, 4, 5, 4)),
+			fakeMarker(new Range(2, 6, 2, 8)),
+			fakeMarker(new Range(7, 6, 7, 8)),
+		];
+		group.updateMarker(data);
+		assert.equal(p.marker.count, 0);
+		assert.equal(c1.marker.count, 2);
+		assert.equal(c2.marker.count, 1);
+
+		data = [
+			fakeMarker(new Range(1, 4, 1, 11)),
+			fakeMarker(new Range(7, 6, 7, 8)),
+		];
+		group.updateMarker(data);
+		assert.equal(p.marker.count, 1);
+		assert.equal(c1.marker, undefined);
+		assert.equal(c2.marker.count, 1);
+	});
 });