Ability to focus diff editor (#2388)

AndreasArvidsson · pokey · web-flow · commit 008baa11ebcf · 2024-06-14T14:49:56.000Z
Also took a crack at the problem of focusing the search editor Fixes #1722 ## Checklist - [/] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [/] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [/] I have not broken the cheatsheet --------- Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
diff --git a/packages/common/src/types/TextEditor.ts b/packages/common/src/types/TextEditor.ts
@@ -53,8 +53,25 @@ export interface TextEditor {
   isEqual(other: TextEditor): boolean;
 }
 
+export interface SetSelectionsOpts {
+  focusEditor?: boolean;
+}
+
 export interface EditableTextEditor extends TextEditor {
-  setSelections(selections: Selection[]): Promise<void>;
+  /**
+   * Set the selections in this text editor, optionally focusing the editor
+   * and/or revealing the ranges.
+   *
+   * Note that if your editor requires unique selections, you should deduplicate
+   * them in your implementation of this method.
+   *
+   * @param selections The new selections
+   * @param opts The options for setting the selections
+   */
+  setSelections(
+    selections: Selection[],
+    opts?: SetSelectionsOpts,
+  ): Promise<void>;
 
   options: TextEditorOptions;
 
diff --git a/packages/cursorless-engine/src/actions/BringMoveSwap.ts b/packages/cursorless-engine/src/actions/BringMoveSwap.ts
@@ -14,7 +14,6 @@ import {
 import { ide } from "../singletons/ide.singleton";
 import { EditWithRangeUpdater } from "../typings/Types";
 import { Destination, Target } from "../typings/target.types";
-import { setSelectionsWithoutFocusingEditor } from "../util/setSelectionsAndFocusEditor";
 import {
   flashTargets,
   getContentRange,
@@ -209,10 +208,7 @@ abstract class BringMoveSwap {
           // NB: We set the selections here because we don't trust vscode to
           // properly move the cursor on a bring. Sometimes it will smear an
           // empty selection
-          await setSelectionsWithoutFocusingEditor(
-            editableEditor,
-            cursorSelections,
-          );
+          await editableEditor.setSelections(cursorSelections);
 
           const marks = [
             ...this.getMarks(sourceEdits, updatedSourceEditSelections),
diff --git a/packages/cursorless-engine/src/actions/CallbackAction.ts b/packages/cursorless-engine/src/actions/CallbackAction.ts
@@ -5,10 +5,6 @@ import { RangeUpdater } from "../core/updateSelections/RangeUpdater";
 import { callFunctionAndUpdateSelections } from "../core/updateSelections/updateSelections";
 import { ide } from "../singletons/ide.singleton";
 import { Target } from "../typings/target.types";
-import {
-  setSelectionsAndFocusEditor,
-  setSelectionsWithoutFocusingEditor,
-} from "../util/setSelectionsAndFocusEditor";
 import {
   ensureSingleEditor,
   ensureSingleTarget,
@@ -95,11 +91,9 @@ export class CallbackAction {
 
     // For this callback/command to the work we have to have the correct editor focused
     if (options.setSelection) {
-      await setSelectionsAndFocusEditor(
-        editableEditor,
-        targetSelections,
-        false,
-      );
+      await editableEditor.setSelections(targetSelections, {
+        focusEditor: true,
+      });
     }
 
     const [updatedOriginalSelections, updatedTargetSelections] =
@@ -116,10 +110,7 @@ export class CallbackAction {
       // very end. This code can run on multiple editors in the course of
       // one command, so we want to avoid focusing the editor multiple
       // times.
-      await setSelectionsWithoutFocusingEditor(
-        editableEditor,
-        updatedOriginalSelections,
-      );
+      await editableEditor.setSelections(updatedOriginalSelections);
     }
 
     // If the document hasn't changed then we just return the original targets
diff --git a/packages/cursorless-engine/src/actions/Clear.ts b/packages/cursorless-engine/src/actions/Clear.ts
@@ -1,7 +1,6 @@
 import { PlainTarget } from "../processTargets/targets";
 import { ide } from "../singletons/ide.singleton";
 import { Target } from "../typings/target.types";
-import { setSelectionsAndFocusEditor } from "../util/setSelectionsAndFocusEditor";
 import { ensureSingleEditor } from "../util/targetUtils";
 import { Actions } from "./Actions";
 import { SimpleAction, ActionReturnValue } from "./actions.types";
@@ -27,10 +26,12 @@ export default class Clear implements SimpleAction {
     const { thatTargets } = await this.actions.remove.run(plainTargets);
 
     if (thatTargets != null) {
-      await setSelectionsAndFocusEditor(
-        ide().getEditableTextEditor(editor),
-        thatTargets.map(({ contentSelection }) => contentSelection),
-      );
+      await ide()
+        .getEditableTextEditor(editor)
+        .setSelections(
+          thatTargets.map(({ contentSelection }) => contentSelection),
+          { focusEditor: true },
+        );
     }
 
     return { thatTargets };
diff --git a/packages/cursorless-engine/src/actions/Deselect.ts b/packages/cursorless-engine/src/actions/Deselect.ts
@@ -1,6 +1,5 @@
 import { ide } from "../singletons/ide.singleton";
 import { Target } from "../typings/target.types";
-import { setSelectionsWithoutFocusingEditor } from "../util/setSelectionsAndFocusEditor";
 import { runOnTargetsForEachEditor } from "../util/targetUtils";
 import { SimpleAction, ActionReturnValue } from "./actions.types";
 
@@ -24,10 +23,7 @@ export default class Deselect implements SimpleAction {
         throw new SelectionRequiredError();
       }
 
-      await setSelectionsWithoutFocusingEditor(
-        ide().getEditableTextEditor(editor),
-        newSelections,
-      );
+      await ide().getEditableTextEditor(editor).setSelections(newSelections);
     });
 
     return {
diff --git a/packages/cursorless-engine/src/actions/EditNew/EditNew.ts b/packages/cursorless-engine/src/actions/EditNew/EditNew.ts
@@ -1,7 +1,6 @@
 import { RangeUpdater } from "../../core/updateSelections/RangeUpdater";
 import { ide } from "../../singletons/ide.singleton";
 import { Destination } from "../../typings/target.types";
-import { setSelectionsAndFocusEditor } from "../../util/setSelectionsAndFocusEditor";
 import { createThatMark, ensureSingleEditor } from "../../util/targetUtils";
 import { Actions } from "../Actions";
 import { ActionReturnValue } from "../actions.types";
@@ -54,7 +53,7 @@ export class EditNew {
     const newSelections = state.destinations.map((destination, index) =>
       state.cursorRanges[index]!.toSelection(destination.target.isReversed),
     );
-    await setSelectionsAndFocusEditor(editableEditor, newSelections);
+    await editableEditor.setSelections(newSelections, { focusEditor: true });
 
     return {
       thatSelections: createThatMark(
diff --git a/packages/cursorless-engine/src/actions/InsertCopy.ts b/packages/cursorless-engine/src/actions/InsertCopy.ts
@@ -12,7 +12,6 @@ import { ModifierStageFactory } from "../processTargets/ModifierStageFactory";
 import { containingLineIfUntypedModifier } from "../processTargets/modifiers/commonContainingScopeIfUntypedModifiers";
 import { ide } from "../singletons/ide.singleton";
 import { Target } from "../typings/target.types";
-import { setSelectionsWithoutFocusingEditor } from "../util/setSelectionsAndFocusEditor";
 import { createThatMark, runOnTargetsForEachEditor } from "../util/targetUtils";
 import { SimpleAction, ActionReturnValue } from "./actions.types";
 
@@ -86,10 +85,7 @@ class InsertCopy implements SimpleAction {
       ([edit, selection]) => edit!.updateRange(selection!),
     );
 
-    await setSelectionsWithoutFocusingEditor(
-      editableEditor,
-      updatedCursorSelections,
-    );
+    await editableEditor.setSelections(updatedCursorSelections);
     const primarySelection = editor.selections[0];
 
     if (
diff --git a/packages/cursorless-engine/src/actions/InsertEmptyLines.ts b/packages/cursorless-engine/src/actions/InsertEmptyLines.ts
@@ -4,7 +4,6 @@ import { RangeUpdater } from "../core/updateSelections/RangeUpdater";
 import { performEditsAndUpdateSelections } from "../core/updateSelections/updateSelections";
 import { ide } from "../singletons/ide.singleton";
 import { Target } from "../typings/target.types";
-import { setSelectionsWithoutFocusingEditor } from "../util/setSelectionsAndFocusEditor";
 import { runOnTargetsForEachEditor } from "../util/targetUtils";
 import { SimpleAction, ActionReturnValue } from "./actions.types";
 
@@ -61,10 +60,7 @@ class InsertEmptyLines implements SimpleAction {
             ],
           );
 
-        await setSelectionsWithoutFocusingEditor(
-          editableEditor,
-          updatedCursorSelections,
-        );
+        await editableEditor.setSelections(updatedCursorSelections);
 
         return {
           thatMark: updatedThatSelections.map((selection) => ({
diff --git a/packages/cursorless-engine/src/actions/PasteFromClipboard.ts b/packages/cursorless-engine/src/actions/PasteFromClipboard.ts
@@ -10,7 +10,6 @@ import {
 } from "../core/updateSelections/updateSelections";
 import { ide } from "../singletons/ide.singleton";
 import { Destination } from "../typings/target.types";
-import { setSelectionsWithoutFocusingEditor } from "../util/setSelectionsAndFocusEditor";
 import { ensureSingleEditor } from "../util/targetUtils";
 import { Actions } from "./Actions";
 import { ActionReturnValue } from "./actions.types";
@@ -60,7 +59,7 @@ export class PasteFromClipboard {
     // Reset cursors on the editor where the edits took place.
     // NB: We don't focus the editor here because we want to focus the original
     // editor, not the one where the edits took place
-    await setSelectionsWithoutFocusingEditor(editor, updatedCursorSelections);
+    await editor.setSelections(updatedCursorSelections);
 
     // If necessary focus back original editor
     if (originalEditor != null && !originalEditor.isActive) {
diff --git a/packages/cursorless-engine/src/actions/Remove.ts b/packages/cursorless-engine/src/actions/Remove.ts
@@ -8,7 +8,6 @@ import { Target } from "../typings/target.types";
 import { flashTargets, runOnTargetsForEachEditor } from "../util/targetUtils";
 import { unifyRemovalTargets } from "../util/unifyRanges";
 import { SimpleAction, ActionReturnValue } from "./actions.types";
-import { setSelectionsWithoutFocusingEditor } from "../util/setSelectionsAndFocusEditor";
 
 export default class Delete implements SimpleAction {
   constructor(private rangeUpdater: RangeUpdater) {
@@ -51,10 +50,7 @@ export default class Delete implements SimpleAction {
         [cursorSelections, editSelections],
       );
 
-    await setSelectionsWithoutFocusingEditor(
-      editableEditor,
-      updatedCursorSelections,
-    );
+    await editableEditor.setSelections(updatedCursorSelections);
 
     return zip(targets, updatedEditSelections).map(
       ([target, range]) =>
diff --git a/packages/cursorless-engine/src/actions/SetSelection.ts b/packages/cursorless-engine/src/actions/SetSelection.ts
@@ -1,7 +1,6 @@
 import { Selection } from "@cursorless/common";
 import { ide } from "../singletons/ide.singleton";
 import { Target } from "../typings/target.types";
-import { setSelectionsAndFocusEditor } from "../util/setSelectionsAndFocusEditor";
 import { ensureSingleEditor } from "../util/targetUtils";
 import { SimpleAction, ActionReturnValue } from "./actions.types";
 
@@ -18,10 +17,9 @@ export class SetSelection implements SimpleAction {
     const editor = ensureSingleEditor(targets);
 
     const selections = targets.map(this.getSelection);
-    await setSelectionsAndFocusEditor(
-      ide().getEditableTextEditor(editor),
-      selections,
-    );
+    await ide()
+      .getEditableTextEditor(editor)
+      .setSelections(selections, { focusEditor: true });
 
     return {
       thatTargets: targets,
diff --git a/packages/cursorless-engine/src/actions/Wrap.ts b/packages/cursorless-engine/src/actions/Wrap.ts
@@ -13,7 +13,6 @@ import {
 import { ide } from "../singletons/ide.singleton";
 import { Target } from "../typings/target.types";
 import { FullSelectionInfo } from "../typings/updateSelections";
-import { setSelectionsWithoutFocusingEditor } from "../util/setSelectionsAndFocusEditor";
 import { runOnTargetsForEachEditor } from "../util/targetUtils";
 import { ActionReturnValue } from "./actions.types";
 
@@ -111,10 +110,7 @@ export default class Wrap {
           ],
         );
 
-        await setSelectionsWithoutFocusingEditor(
-          editableEditor,
-          cursorSelections,
-        );
+        await editableEditor.setSelections(cursorSelections);
 
         await ide().flashRanges(
           delimiterSelections.map((selection) => ({
diff --git a/packages/cursorless-engine/src/util/setSelectionsAndFocusEditor.ts b/packages/cursorless-engine/src/util/setSelectionsAndFocusEditor.ts
diff --git a/packages/cursorless-vscode/src/ide/vscode/VscodeFocusEditor.ts b/packages/cursorless-vscode/src/ide/vscode/VscodeFocusEditor.ts
@@ -23,19 +23,27 @@ const columnFocusCommands = {
   [ViewColumn.Beside]: "",
 };
 
-export default async function vscodeFocusEditor(editor: VscodeTextEditorImpl) {
-  // Focusing the search editor brings focus back to the input field.
-  // FIXME: This is a hack. There is no way to focus the search editor. If we
-  // could figure out if the editor was not focused, we could issue
-  // `search.action.focusNextSearchResult`.
-  // Issue: https://github.com/cursorless-dev/cursorless/issues/1722
-  if (editor.document.uri.scheme === "search-editor") {
-    return;
-  }
-
+/**
+ * Focus editor. Returns true if selection needs to be set again.
+ */
+export default async function vscodeFocusEditor(
+  editor: VscodeTextEditorImpl,
+): Promise<void> {
   const viewColumn = getViewColumn(editor.vscodeEditor);
   if (viewColumn != null) {
     await commands.executeCommand(columnFocusCommands[viewColumn]);
+
+    if (editor.isDiffEditorOriginal && !editor.isActive) {
+      // There is no way of directly focusing the left hand side of a diff
+      // editor. Switch side if needed.
+
+      await commands.executeCommand("diffEditor.switchSide");
+    } else if (editor.isSearchEditor) {
+      // Focusing the search editor brings focus back to the input field. This
+      // command moves selection into the actual editor text.
+
+      await commands.executeCommand("search.action.focusNextSearchResult");
+    }
   } else {
     // If the view column is null we see if it's a notebook and try to see if we
     // can just move around in the notebook to focus the correct editor
@@ -52,7 +60,9 @@ function getViewColumn(editor: TextEditor): ViewColumn | undefined {
   const tabGroup = window.tabGroups.all.find((tabGroup) =>
     tabGroup.tabs.find(
       (tab: any) =>
-        (tab?.input?.uri ?? tab?.input?.modified)?.toString() === uri,
+        tab.input.uri?.toString() === uri ||
+        tab.input.original?.toString() === uri ||
+        tab.input.modified?.toString() === uri,
     ),
   );
   return tabGroup?.viewColumn;
diff --git a/packages/cursorless-vscode/src/ide/vscode/VscodeTextEditorImpl.ts b/packages/cursorless-vscode/src/ide/vscode/VscodeTextEditorImpl.ts
diff --git a/packages/cursorless-vscode/src/ide/vscode/isDiffEditorOriginal.ts b/packages/cursorless-vscode/src/ide/vscode/isDiffEditorOriginal.ts
