"Select all occurrences of find match" selects weirdly when word wrapping

[Zarel]

Issue Type: Bug

With word wrap enabled, "select all occurrences of find match", if matching the end of a line, will select those weirdly, so that pressing Left will, instead of moving to the beginning of the line, move one beyond.

For instance, here, I'm searching for the string ,[space]:

After I press Left, I expect the cursor to be at ,, but instead:

Most cursors are correctly at ,, but the cursors at the ends of lines are instead at l.

Here's a GIF showing the full process:

This makes it very hard to process a list like foo,bar,baz into 'foo','bar','baz', because the misalignment will instead turn it into 'foo','ba'r',baz'

VS Code version: Code 1.48.2 (a047975, 2020-08-25T10:09:08.021Z)
OS version: Darwin x64 19.3.0
Remote OS version: Linux x64 4.15.0-72-generic

System Info

Item	Value
CPUs	Intel(R) Xeon(R) W-2140B CPU @ 3.20GHz (16 x 3200)
GPU Status	2d_canvas: enabled flash_3d: enabled flash_stage3d: enabled flash_stage3d_baseline: enabled gpu_compositing: enabled metal: disabled_off multiple_raster_threads: enabled_on oop_rasterization: disabled_off protected_video_decode: unavailable_off rasterization: enabled skia_renderer: disabled_off_ok video_decode: enabled viz_display_compositor: enabled_on viz_hit_test_surface_layer: disabled_off_ok webgl: enabled webgl2: enabled
Load (avg)	1, 1, 1
Memory (System)	32.00GB (11.55GB free)
Process Argv	-psn_0_81940
Screen Reader	no
VM	0%

Item	Value
Remote	SSH: ps-sim
OS	Linux x64 4.15.0-72-generic
CPUs	Intel(R) Xeon(R) E-2136 CPU @ 3.30GHz (12 x 4298)
Memory (System)	62.69GB (4.80GB free)
VM	0%

Extensions (7)

Extension	Author (truncated)	Version
gitlens	eam	10.2.2
remote-containers	ms-	0.134.1
remote-ssh	ms-	0.51.0
remote-ssh-edit	ms-	0.51.0
remote-wsl	ms-	0.44.4
vscode-remote-extensionpack	ms-	0.20.0
sublime-commands	Zar	0.1.0

[alexdima]

@hediet Here is the patch we sketched:

diff --git a/src/vs/editor/common/controller/cursorCommon.ts b/src/vs/editor/common/controller/cursorCommon.ts
index 0077027b265..5001e544f29 100644
--- a/src/vs/editor/common/controller/cursorCommon.ts
+++ b/src/vs/editor/common/controller/cursorCommon.ts
@@ -219,6 +219,8 @@ export interface ICursorSimpleModel {
 	getLineMaxColumn(lineNumber: number): number;
 	getLineFirstNonWhitespaceColumn(lineNumber: number): number;
 	getLineLastNonWhitespaceColumn(lineNumber: number): number;
+
+	getLeftPosition(lineNumber: number, column: number): Position;
 }
 
 /**
diff --git a/src/vs/editor/common/controller/cursorMoveCommands.ts b/src/vs/editor/common/controller/cursorMoveCommands.ts
index 2ae510c30a0..3302e57536a 100644
--- a/src/vs/editor/common/controller/cursorMoveCommands.ts
+++ b/src/vs/editor/common/controller/cursorMoveCommands.ts
@@ -419,26 +419,10 @@ export class CursorMoveCommands {
 	}
 
 	private static _moveLeft(viewModel: IViewModel, cursors: CursorState[], inSelectionMode: boolean, noOfColumns: number): PartialCursorState[] {
-		const hasMultipleCursors = (cursors.length > 1);
 		let result: PartialCursorState[] = [];
 		for (let i = 0, len = cursors.length; i < len; i++) {
 			const cursor = cursors[i];
-			const skipWrappingPointStop = hasMultipleCursors || !cursor.viewState.hasSelection();
 			let newViewState = MoveOperations.moveLeft(viewModel.cursorConfig, viewModel, cursor.viewState, inSelectionMode, noOfColumns);
-
-			if (skipWrappingPointStop
-				&& noOfColumns === 1
-				&& cursor.viewState.position.column === viewModel.getLineMinColumn(cursor.viewState.position.lineNumber)
-				&& newViewState.position.lineNumber !== cursor.viewState.position.lineNumber
-			) {
-				// moved over to the previous view line
-				const newViewModelPosition = viewModel.coordinatesConverter.convertViewPositionToModelPosition(newViewState.position);
-				if (newViewModelPosition.lineNumber === cursor.modelState.position.lineNumber) {
-					// stayed on the same model line => pass wrapping point where 2 view positions map to a single model position
-					newViewState = MoveOperations.moveLeft(viewModel.cursorConfig, viewModel, newViewState, inSelectionMode, 1);
-				}
-			}
-
 			result[i] = CursorState.fromViewState(newViewState);
 		}
 		return result;
diff --git a/src/vs/editor/common/controller/cursorMoveOperations.ts b/src/vs/editor/common/controller/cursorMoveOperations.ts
index 7d94c437434..772b0accfbe 100644
--- a/src/vs/editor/common/controller/cursorMoveOperations.ts
+++ b/src/vs/editor/common/controller/cursorMoveOperations.ts
@@ -27,13 +27,7 @@ export class CursorPosition {
 export class MoveOperations {
 
 	public static leftPosition(model: ICursorSimpleModel, lineNumber: number, column: number): Position {
-		if (column > model.getLineMinColumn(lineNumber)) {
-			column = column - strings.prevCharLength(model.getLineContent(lineNumber), column - 1);
-		} else if (lineNumber > 1) {
-			lineNumber = lineNumber - 1;
-			column = model.getLineMaxColumn(lineNumber);
-		}
-		return new Position(lineNumber, column);
+		return model.getLeftPosition(lineNumber, column);
 	}
 
 	public static leftPositionAtomicSoftTabs(model: ICursorSimpleModel, lineNumber: number, column: number, tabSize: number): Position {
diff --git a/src/vs/editor/common/model.ts b/src/vs/editor/common/model.ts
index 3f1a239dcbf..be75f5a10ad 100644
--- a/src/vs/editor/common/model.ts
+++ b/src/vs/editor/common/model.ts
@@ -716,6 +716,11 @@ export interface ITextModel {
 	 */
 	getLineLastNonWhitespaceColumn(lineNumber: number): number;
 
+	/**
+	 * @internal
+	 */
+	getLeftPosition(lineNumber: number, column: number): Position;
+
 	/**
 	 * Create a valid position,
 	 */
diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts
index 61452e16c78..685792587f9 100644
--- a/src/vs/editor/common/model/textModel.ts
+++ b/src/vs/editor/common/model/textModel.ts
@@ -865,6 +865,16 @@ export class TextModel extends Disposable implements model.ITextModel {
 		return this._buffer.getLineLastNonWhitespaceColumn(lineNumber);
 	}
 
+	public getLeftPosition(lineNumber: number, column: number): Position {
+		if (column > this.getLineMinColumn(lineNumber)) {
+			column = column - strings.prevCharLength(this.getLineContent(lineNumber), column - 1);
+		} else if (lineNumber > 1) {
+			lineNumber = lineNumber - 1;
+			column = this.getLineMaxColumn(lineNumber);
+		}
+		return new Position(lineNumber, column);
+	}
+
 	/**
 	 * Validates `range` is within buffer bounds, but allows it to sit in between surrogate pairs, etc.
 	 * Will try to not allocate if possible.
diff --git a/src/vs/editor/common/viewModel/viewModel.ts b/src/vs/editor/common/viewModel/viewModel.ts
index 92966555364..11de82c685c 100644
--- a/src/vs/editor/common/viewModel/viewModel.ts
+++ b/src/vs/editor/common/viewModel/viewModel.ts
@@ -185,6 +185,8 @@ export interface IViewModel extends ICursorSimpleModel {
 	getLineMaxColumn(lineNumber: number): number;
 	getLineFirstNonWhitespaceColumn(lineNumber: number): number;
 	getLineLastNonWhitespaceColumn(lineNumber: number): number;
+	getLeftPosition(lineNumber: number, column: number): Position;
+
 	getAllOverviewRulerDecorations(theme: EditorTheme): IOverviewRulerDecorations;
 	invalidateOverviewRulerColorCache(): void;
 	invalidateMinimapColorCache(): void;
diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts
index fccd9d50f9b..50afb58bef7 100644
--- a/src/vs/editor/common/viewModel/viewModelImpl.ts
+++ b/src/vs/editor/common/viewModel/viewModelImpl.ts
@@ -633,6 +633,11 @@ export class ViewModel extends Disposable implements IViewModel {
 		return result + 2;
 	}
 
+	public getLeftPosition(viewLineNumber: number, viewColumn: number): Position {
+		// TODO
+		throw new Error(`TODO: delegate to this._lines`);
+	}
+
 	public getDecorationsInViewport(visibleRange: Range): ViewModelDecoration[] {
 		return this._decorations.getDecorationsViewportData(visibleRange).decorations;
 	}
@@ -926,6 +931,7 @@ export class ViewModel extends Disposable implements IViewModel {
 	public setPrevEditOperationType(type: EditOperationType): void {
 		this._cursor.setPrevEditOperationType(type);
 	}
+
 	public getSelection(): Selection {
 		return this._cursor.getSelection();
 	}