Fix #59919 - text search combines matches on one line

src/vs/base/common/arrays.ts

@@ -540,3 +540,9 @@ export function find<T>(arr: ArrayLike<T>, predicate: (value: T, index: number,
 
 	return undefined;
 }
+
+export function mapArrayOrNot<T, U>(items: T | T[], fn: (_: T) => U): U | U[] {
+	return Array.isArray(items) ?
+		items.map(fn) :
+		fn(items);
+}

src/vs/platform/search/common/search.ts

@@ -3,17 +3,18 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { mapArrayOrNot } from 'vs/base/common/arrays';
+import { CancellationToken } from 'vs/base/common/cancellation';
 import { Event } from 'vs/base/common/event';
 import * as glob from 'vs/base/common/glob';
 import { IDisposable } from 'vs/base/common/lifecycle';
 import * as objects from 'vs/base/common/objects';
 import * as paths from 'vs/base/common/paths';
+import { getNLines } from 'vs/base/common/strings';
 import { URI as uri, UriComponents } from 'vs/base/common/uri';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { IFilesConfiguration } from 'vs/platform/files/common/files';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
-import { CancellationToken } from 'vs/base/common/cancellation';
-import { getNLines } from 'vs/base/common/strings';
 
 export const VIEW_ID = 'workbench.view.search';
 
@@ -170,12 +171,12 @@ export interface ISearchRange {
 
 export interface ITextSearchResultPreview {
 	text: string;
-	match: ISearchRange;
+	matches: ISearchRange | ISearchRange[];
 }
 
 export interface ITextSearchResult {
 	uri?: uri;
-	range: ISearchRange;
+	ranges: ISearchRange | ISearchRange[];
 	preview: ITextSearchResultPreview;
 }
 
@@ -248,13 +249,13 @@ export class FileMatch implements IFileMatch {
 }
 
 export class TextSearchResult implements ITextSearchResult {
-	range: ISearchRange;
+	ranges: ISearchRange | ISearchRange[];
 	preview: ITextSearchResultPreview;
 
-	constructor(text: string, range: ISearchRange, previewOptions?: ITextSearchPreviewOptions) {
-		this.range = range;
+	constructor(text: string, range: ISearchRange | ISearchRange[], previewOptions?: ITextSearchPreviewOptions) {
+		this.ranges = range;
 
-		if (previewOptions && previewOptions.matchLines === 1) {
+		if (previewOptions && previewOptions.matchLines === 1 && !Array.isArray(range)) {
 			// 1 line preview requested
 			text = getNLines(text, previewOptions.matchLines);
 			const leadingChars = Math.floor(previewOptions.charsPerLine / 5);
@@ -267,13 +268,15 @@ export class TextSearchResult implements ITextSearchResult {
 
 			this.preview = {
 				text: previewText,
-				match: new OneLineRange(0, range.startColumn - previewStart, endColInPreview)
+				matches: new OneLineRange(0, range.startColumn - previewStart, endColInPreview)
 			};
 		} else {
-			// n line or no preview requested
+			const firstMatchLine = Array.isArray(range) ? range[0].startLineNumber : range.startLineNumber;
+
+			// n line, no preview requested, or multiple matches in the preview
 			this.preview = {
 				text,
-				match: new SearchRange(0, range.startColumn, range.endLineNumber - range.startLineNumber, range.endColumn)
+				matches: mapArrayOrNot(range, r => new SearchRange(r.startLineNumber - firstMatchLine, r.startColumn, r.endLineNumber - firstMatchLine, r.endColumn))
 			};
 		}
 	}

src/vs/platform/search/test/common/search.test.ts

@@ -14,56 +14,56 @@ suite('TextSearchResult', () => {
 
 	function assertPreviewRangeText(text: string, result: TextSearchResult): void {
 		assert.equal(
-			result.preview.text.substring(result.preview.match.startColumn, result.preview.match.endColumn),
+			result.preview.text.substring((<SearchRange>result.preview.matches).startColumn, (<SearchRange>result.preview.matches).endColumn),
 			text);
 	}
 
 	test('empty without preview options', () => {
 		const range = new OneLineRange(5, 0, 0);
 		const result = new TextSearchResult('', range);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('', result);
 	});
 
 	test('empty with preview options', () => {
 		const range = new OneLineRange(5, 0, 0);
 		const result = new TextSearchResult('', range, previewOptions1);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('', result);
 	});
 
 	test('short without preview options', () => {
 		const range = new OneLineRange(5, 4, 7);
 		const result = new TextSearchResult('foo bar', range);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('bar', result);
 	});
 
 	test('short with preview options', () => {
 		const range = new OneLineRange(5, 4, 7);
 		const result = new TextSearchResult('foo bar', range, previewOptions1);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('bar', result);
 	});
 
 	test('leading', () => {
 		const range = new OneLineRange(5, 25, 28);
 		const result = new TextSearchResult('long text very long text foo', range, previewOptions1);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('foo', result);
 	});
 
 	test('trailing', () => {
 		const range = new OneLineRange(5, 0, 3);
 		const result = new TextSearchResult('foo long text very long text long text very long text long text very long text long text very long text long text very long text', range, previewOptions1);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('foo', result);
 	});
 
 	test('middle', () => {
 		const range = new OneLineRange(5, 30, 33);
 		const result = new TextSearchResult('long text very long text long foo text very long text long text very long text long text very long text long text very long text', range, previewOptions1);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('foo', result);
 	});
 
@@ -75,7 +75,7 @@ suite('TextSearchResult', () => {
 
 		const range = new OneLineRange(0, 4, 7);
 		const result = new TextSearchResult('foo bar', range, previewOptions);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('b', result);
 	});
 
@@ -87,7 +87,7 @@ suite('TextSearchResult', () => {
 
 		const range = new SearchRange(5, 4, 6, 3);
 		const result = new TextSearchResult('foo bar\nfoo bar', range, previewOptions);
-		assert.deepEqual(result.range, range);
+		assert.deepEqual(result.ranges, range);
 		assertPreviewRangeText('bar', result);
 	});
 

src/vs/vscode.proposed.d.ts

@@ -234,8 +234,9 @@ declare module 'vscode' {
 
 		/**
 		 * The Range within `text` corresponding to the text of the match.
+		 * The number of matches must match the TextSearchResult's range property.
 		 */
-		match: Range;
+		matches: Range | Range[];
 	}
 
 	/**
@@ -248,9 +249,9 @@ declare module 'vscode' {
 		uri: Uri;
 
 		/**
-		 * The range of the match within the document.
+		 * The range of the match within the document, or multiple ranges for multiple matches.
 		 */
-		range: Range;
+		ranges: Range | Range[];
 
 		/**
 		 * A preview of the text result.

src/vs/workbench/api/node/extHostWorkspace.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { join, relative } from 'path';
-import { delta as arrayDelta } from 'vs/base/common/arrays';
+import { delta as arrayDelta, mapArrayOrNot } from 'vs/base/common/arrays';
 import { Emitter, Event } from 'vs/base/common/event';
 import { TernarySearchTree } from 'vs/base/common/map';
 import { Counter } from 'vs/base/common/numbers';
@@ -425,9 +425,13 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape {
 					uri: URI.revive(p.resource),
 					preview: {
 						text: match.preview.text,
-						match: new Range(match.preview.match.startLineNumber, match.preview.match.startColumn, match.preview.match.endLineNumber, match.preview.match.endColumn)
+						matches: mapArrayOrNot(
+							match.preview.matches,
+							m => new Range(m.startLineNumber, m.startColumn, m.endLineNumber, m.endColumn))
 					},
-					range: new Range(match.range.startLineNumber, match.range.startColumn, match.range.endLineNumber, match.range.endColumn)
+					ranges: mapArrayOrNot(
+						match.ranges,
+						r => new Range(r.startLineNumber, r.startColumn, r.endLineNumber, r.endColumn))
 				});
 			});
 		};

src/vs/workbench/parts/search/common/searchModel.ts

@@ -21,7 +21,7 @@ import { IModelService } from 'vs/editor/common/services/modelService';
 import { createDecorator, IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IProgressRunner } from 'vs/platform/progress/common/progress';
 import { ReplacePattern } from 'vs/platform/search/common/replace';
-import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchService, ITextSearchPreviewOptions, ITextSearchResult, ITextSearchStats, TextSearchResult, ITextQuery } from 'vs/platform/search/common/search';
+import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchService, ITextQuery, ITextSearchPreviewOptions, ITextSearchResult, ITextSearchStats, TextSearchResult } from 'vs/platform/search/common/search';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { overviewRulerFindMatchForeground } from 'vs/platform/theme/common/colorRegistry';
 import { themeColorFromId } from 'vs/platform/theme/common/themeService';
@@ -37,17 +37,21 @@ export class Match {
 	private _rangeInPreviewText: Range;
 
 	constructor(private _parent: FileMatch, _result: ITextSearchResult) {
+		if (Array.isArray(_result.ranges) || Array.isArray(_result.preview.matches)) {
+			throw new Error('A Match can only be built from a single search result');
+		}
+
 		this._range = new Range(
-			_result.range.startLineNumber + 1,
-			_result.range.startColumn + 1,
-			_result.range.endLineNumber + 1,
-			_result.range.endColumn + 1);
+			_result.ranges.startLineNumber + 1,
+			_result.ranges.startColumn + 1,
+			_result.ranges.endLineNumber + 1,
+			_result.ranges.endColumn + 1);
 
 		this._rangeInPreviewText = new Range(
-			_result.preview.match.startLineNumber + 1,
-			_result.preview.match.startColumn + 1,
-			_result.preview.match.endLineNumber + 1,
-			_result.preview.match.endColumn + 1);
+			_result.preview.matches.startLineNumber + 1,
+			_result.preview.matches.startColumn + 1,
+			_result.preview.matches.endLineNumber + 1,
+			_result.preview.matches.endColumn + 1);
 		this._previewText = _result.preview.text;
 
 		this._id = this._parent.id() + '>' + this._range + this.getMatchString();
@@ -171,8 +175,8 @@ export class FileMatch extends Disposable {
 			this.updateMatchesForModel();
 		} else {
 			this.rawMatch.matches.forEach(rawMatch => {
-				let match = new Match(this, rawMatch);
-				this.add(match);
+				textSearchResultToMatches(rawMatch, this)
+					.forEach(m => this.add(m));
 			});
 		}
 	}
@@ -416,8 +420,8 @@ export class FolderMatch extends Disposable {
 			if (this._fileMatches.has(rawFileMatch.resource)) {
 				const existingFileMatch = this._fileMatches.get(rawFileMatch.resource);
 				rawFileMatch.matches.forEach(m => {
-					let match = new Match(existingFileMatch, m);
-					existingFileMatch.add(match);
+					textSearchResultToMatches(m, existingFileMatch)
+						.forEach(m => existingFileMatch.add(m));
 				});
 				updated.push(existingFileMatch);
 			} else {
@@ -1011,3 +1015,21 @@ export function editorMatchToTextSearchResult(match: FindMatch, model: ITextMode
 		new Range(match.range.startLineNumber - 1, match.range.startColumn - 1, match.range.endLineNumber - 1, match.range.endColumn - 1),
 		previewOptions);
 }
+
+function textSearchResultToMatches(rawMatch: ITextSearchResult, fileMatch: FileMatch): Match[] {
+	if (Array.isArray(rawMatch.ranges)) {
+		return rawMatch.ranges.map((r, i) => {
+			return new Match(fileMatch, {
+				uri: rawMatch.uri,
+				ranges: r,
+				preview: {
+					text: rawMatch.preview.text,
+					matches: rawMatch.preview.matches[i]
+				}
+			});
+		});
+	} else {
+		let match = new Match(fileMatch, rawMatch);
+		return [match];
+	}
+}

src/vs/workbench/parts/search/test/browser/searchActions.test.ts

@@ -134,7 +134,7 @@ suite('Search Actions', () => {
 
 	function aMatch(fileMatch: FileMatch): Match {
 		const line = ++counter;
-		const range = {
+		const ranges = {
 			startLineNumber: line,
 			startColumn: 0,
 			endLineNumber: line,
@@ -143,9 +143,9 @@ suite('Search Actions', () => {
 		let match = new Match(fileMatch, {
 			preview: {
 				text: 'some match',
-				match: range
+				matches: ranges
 			},
-			range
+			ranges
 		});
 		fileMatch.add(match);
 		return match;

src/vs/workbench/parts/search/test/browser/searchViewlet.test.ts

@@ -36,7 +36,7 @@ suite('Search - Viewlet', () => {
 			}]
 		};
 
-		const range = {
+		const ranges = {
 			startLineNumber: 1,
 			startColumn: 0,
 			endLineNumber: 1,
@@ -47,9 +47,9 @@ suite('Search - Viewlet', () => {
 			matches: [{
 				preview: {
 					text: 'bar',
-					match: range
+					matches: ranges
 				},
-				range
+				ranges
 			}]
 		}]);
 

src/vs/workbench/services/search/node/ripgrepSearchUtils.ts

@@ -7,33 +7,40 @@ import { startsWith } from 'vs/base/common/strings';
 import { ILogService } from 'vs/platform/log/common/log';
 import { SearchRange, TextSearchResult } from 'vs/platform/search/common/search';
 import * as vscode from 'vscode';
+import { mapArrayOrNot } from 'vs/base/common/arrays';
 
 export type Maybe<T> = T | null | undefined;
 
 export function anchorGlob(glob: string): string {
 	return startsWith(glob, '**') || startsWith(glob, '/') ? glob : `/${glob}`;
 }
 
-export function createTextSearchResult(uri: vscode.Uri, text: string, range: Range, previewOptions?: vscode.TextSearchPreviewOptions): vscode.TextSearchResult {
-	const searchRange: SearchRange = {
-		startLineNumber: range.start.line,
-		startColumn: range.start.character,
-		endLineNumber: range.end.line,
-		endColumn: range.end.character,
-	};
+/**
+ * Create a vscode.TextSearchResult by using our internal TextSearchResult type for its previewOptions logic.
+ */
+export function createTextSearchResult(uri: vscode.Uri, text: string, range: Range | Range[], previewOptions?: vscode.TextSearchPreviewOptions): vscode.TextSearchResult {
+	const searchRange = mapArrayOrNot(range, rangeToSearchRange);
 
 	const internalResult = new TextSearchResult(text, searchRange, previewOptions);
-	const internalPreviewRange = internalResult.preview.match;
+	const internalPreviewRange = internalResult.preview.matches;
 	return {
-		range: new Range(internalResult.range.startLineNumber, internalResult.range.startColumn, internalResult.range.endLineNumber, internalResult.range.endColumn),
+		ranges: mapArrayOrNot(searchRange, searchRangeToRange),
 		uri,
 		preview: {
 			text: internalResult.preview.text,
-			match: new Range(internalPreviewRange.startLineNumber, internalPreviewRange.startColumn, internalPreviewRange.endLineNumber, internalPreviewRange.endColumn),
+			matches: mapArrayOrNot(internalPreviewRange, searchRangeToRange)
 		}
 	};
 }
 
+function rangeToSearchRange(range: Range): SearchRange {
+	return new SearchRange(range.start.line, range.start.character, range.end.line, range.end.character);
+}
+
+function searchRangeToRange(range: SearchRange): Range {
+	return new Range(range.startLineNumber, range.startColumn, range.endLineNumber, range.endColumn);
+}
+
 export class Position {
 	constructor(public readonly line, public readonly character) { }