Fixes #1857: next/prev match actions seed search string only if search string...

Fixes #1857: next/prev match actions seed search string only if search string is empty and do not focus find widget

src/vs/editor/contrib/find/common/findController.ts

@@ -173,14 +173,8 @@ export class CommonFindController extends Disposable implements EditorCommon.IEd
 		}
 	}
 
-	public startFromAction(withReplace:boolean): void {
-		this._start({
-			forceRevealReplace: withReplace,
-			seedSearchStringFromSelection: true,
-			seedSearchScopeFromSelection: true,
-			shouldFocus: withReplace ? FindStartFocusAction.FocusReplaceInput : FindStartFocusAction.FocusFindInput,
-			shouldAnimate: true
-		});
+	public start(opts:IFindStartOptions): void {
+		this._start(opts);
 	}
 
 	public moveToNextMatch(): boolean {
@@ -224,7 +218,13 @@ export class StartFindAction extends EditorAction {
 
 	public run(): TPromise<boolean> {
 		let controller = CommonFindController.getFindController(this.editor);
-		controller.startFromAction(false);
+		controller.start({
+			forceRevealReplace: false,
+			seedSearchStringFromSelection: true,
+			seedSearchScopeFromSelection: true,
+			shouldFocus: FindStartFocusAction.FocusFindInput,
+			shouldAnimate: true
+		});
 		return TPromise.as(true);
 	}
 }
@@ -238,7 +238,13 @@ export class NextMatchFindAction extends EditorAction {
 	public run(): TPromise<boolean> {
 		let controller = CommonFindController.getFindController(this.editor);
 		if (!controller.moveToNextMatch()) {
-			controller.startFromAction(false);
+			controller.start({
+				forceRevealReplace: false,
+				seedSearchStringFromSelection: (controller.getState().searchString.length === 0),
+				seedSearchScopeFromSelection: false,
+				shouldFocus: FindStartFocusAction.NoFocusChange,
+				shouldAnimate: true
+			});
 			controller.moveToNextMatch();
 		}
 		return TPromise.as(true);
@@ -254,7 +260,13 @@ export class PreviousMatchFindAction extends EditorAction {
 	public run(): TPromise<boolean> {
 		let controller = CommonFindController.getFindController(this.editor);
 		if (!controller.moveToPrevMatch()) {
-			controller.startFromAction(false);
+			controller.start({
+				forceRevealReplace: false,
+				seedSearchStringFromSelection: (controller.getState().searchString.length === 0),
+				seedSearchScopeFromSelection: false,
+				shouldFocus: FindStartFocusAction.NoFocusChange,
+				shouldAnimate: true
+			});
 			controller.moveToPrevMatch();
 		}
 		return TPromise.as(true);
@@ -269,7 +281,13 @@ export class StartFindReplaceAction extends EditorAction {
 
 	public run(): TPromise<boolean> {
 		let controller = CommonFindController.getFindController(this.editor);
-		controller.startFromAction(true);
+		controller.start({
+			forceRevealReplace: true,
+			seedSearchStringFromSelection: (controller.getState().searchString.length === 0),
+			seedSearchScopeFromSelection: true,
+			shouldFocus: FindStartFocusAction.FocusReplaceInput,
+			shouldAnimate: true
+		});
 		return TPromise.as(true);
 	}
 }
@@ -335,7 +353,7 @@ export function multiCursorFind(editor:EditorCommon.ICommonCodeEditor, changeFin
 	};
 }
 
-class SelectNextFindMatchAction extends EditorAction {
+export class SelectNextFindMatchAction extends EditorAction {
 	constructor(descriptor:EditorCommon.IEditorActionDescriptorData, editor:EditorCommon.ICommonCodeEditor, @INullService ns) {
 		super(descriptor, editor, Behaviour.WidgetFocus);
 	}
@@ -362,7 +380,7 @@ class SelectNextFindMatchAction extends EditorAction {
 	}
 }
 
-class AddSelectionToNextFindMatchAction extends SelectNextFindMatchAction {
+export class AddSelectionToNextFindMatchAction extends SelectNextFindMatchAction {
 	static ID = 'editor.action.addSelectionToNextFindMatch';
 
 	constructor(descriptor:EditorCommon.IEditorActionDescriptorData, editor:EditorCommon.ICommonCodeEditor, @INullService ns) {
@@ -384,7 +402,7 @@ class AddSelectionToNextFindMatchAction extends SelectNextFindMatchAction {
 	}
 }
 
-class MoveSelectionToNextFindMatchAction extends SelectNextFindMatchAction {
+export class MoveSelectionToNextFindMatchAction extends SelectNextFindMatchAction {
 	static ID = 'editor.action.moveSelectionToNextFindMatch';
 
 	constructor(descriptor:EditorCommon.IEditorActionDescriptorData, editor:EditorCommon.ICommonCodeEditor, @INullService ns) {
@@ -407,7 +425,7 @@ class MoveSelectionToNextFindMatchAction extends SelectNextFindMatchAction {
 	}
 }
 
-class SelectHighlightsAction extends EditorAction {
+export class SelectHighlightsAction extends EditorAction {
 	static ID = 'editor.action.selectHighlights';
 	static COMPAT_ID = 'editor.action.changeAll';
 

src/vs/editor/contrib/find/test/common/findController.test.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+'use strict';
+
+import assert = require('assert');
+import {FindModelBoundToEditorModel, parseReplaceString} from 'vs/editor/contrib/find/common/findModel';
+import * as EditorCommon from 'vs/editor/common/editorCommon';
+import {withMockCodeEditor} from 'vs/editor/test/common/mocks/mockCodeEditor';
+import {Cursor} from 'vs/editor/common/controller/cursor';
+import {INewFindReplaceState, FindReplaceStateChangedEvent, FindReplaceState} from 'vs/editor/contrib/find/common/findState';
+import {Range} from 'vs/editor/common/core/range';
+import {Position} from 'vs/editor/common/core/position';
+import {
+	CommonFindController,
+	IFindStartOptions,
+	FindStartFocusAction,
+	StartFindAction,
+	NextMatchFindAction
+} from 'vs/editor/contrib/find/common/findController';
+import {EditOperation} from 'vs/editor/common/core/editOperation';
+
+class TestFindController extends CommonFindController {
+
+	public hasFocus: boolean;
+
+	protected _start(opts:IFindStartOptions): void {
+		super._start(opts);
+
+		if (opts.shouldFocus !== FindStartFocusAction.NoFocusChange) {
+			this.hasFocus = true;
+		}
+	}
+}
+
+suite('FindController', () => {
+
+	function fromRange(rng:EditorCommon.IRange): number[] {
+		return [rng.startLineNumber, rng.startColumn, rng.endLineNumber, rng.endColumn];
+	}
+
+	test('issue #1857: F3, Find Next, acts like "Find Under Cursor"', () => {
+		withMockCodeEditor([
+			'ABC',
+			'ABC',
+			'XYZ',
+			'ABC'
+		], {}, (editor, cursor) => {
+
+			// The cursor is at the very top, of the file, at the first ABC
+			let findController = editor.registerAndInstantiateContribution<TestFindController>(TestFindController);
+			let findState = findController.getState();
+			let startFindAction = new StartFindAction({id:'',label:''}, editor, null);
+			let nextMatchFindAction = new NextMatchFindAction({id:'',label:''}, editor, null);
+
+			// I hit Ctrl+F to show the Find dialog
+			startFindAction.run();
+
+			// I type ABC.
+			findState.change({ searchString: 'A' }, true);
+			findState.change({ searchString: 'AB' }, true);
+			findState.change({ searchString: 'ABC' }, true);
+
+			// The first ABC is highlighted.
+			assert.deepEqual(fromRange(editor.getSelection()), [1, 1, 1, 4]);
+
+			// I hit Esc to exit the Find dialog.
+			findController.closeFindWidget();
+			findController.hasFocus = false;
+
+			// The cursor is now at end of the first line, with ABC on that line highlighted.
+			assert.deepEqual(fromRange(editor.getSelection()), [1, 1, 1, 4]);
+
+			// I hit delete to remove it and change the text to XYZ.
+			editor.executeEdits('test', [EditOperation.delete(new Range(1, 1, 1, 4))]);
+			editor.executeEdits('test', [EditOperation.insert(new Position(1, 1), 'XYZ')]);
+
+			// At this point the text editor looks like this:
+			//   XYZ
+			//   ABC
+			//   XYZ
+			//   ABC
+			assert.equal(editor.getModel().getLineContent(1), 'XYZ');
+
+			// The cursor is at end of the first line.
+			assert.deepEqual(fromRange(editor.getSelection()), [1, 4, 1, 4]);
+
+			// I hit F3 to "Find Next" to find the next occurrence of ABC, but instead it searches for XYZ.
+			nextMatchFindAction.run();
+
+			assert.equal(findState.searchString, 'ABC');
+			assert.equal(findController.hasFocus, false);
+
+			findController.dispose();
+			startFindAction.dispose();
+			nextMatchFindAction.dispose();
+		});
+	});
+});

src/vs/editor/test/common/mocks/mockCodeEditor.ts

@@ -41,6 +41,12 @@ export class MockCodeEditor extends CommonCodeEditor {
 	getCursor(): Cursor {
 		return this.cursor;
 	}
+
+	public registerAndInstantiateContribution<T extends EditorCommon.IEditorContribution>(ctor:any): T {
+		let r = <T>this._instantiationService.createInstance(ctor, this);
+		this.contributions[r.getId()] = r;
+		return r;
+	}
 }
 
 export class MockScopeLocation implements IKeybindingScopeLocation {
@@ -48,7 +54,7 @@ export class MockScopeLocation implements IKeybindingScopeLocation {
 	removeAttribute(attr:string): void { }
 }
 
-export function withMockCodeEditor(text:string[], options:EditorCommon.ICodeEditorWidgetCreationOptions, callback:(editor:EditorCommon.ICommonCodeEditor, cursor:Cursor)=>void): void {
+export function withMockCodeEditor(text:string[], options:EditorCommon.ICodeEditorWidgetCreationOptions, callback:(editor:MockCodeEditor, cursor:Cursor)=>void): void {
 
 	let codeEditorService = new MockCodeEditorService();
 	let keybindingService = new MockKeybindingService();