Merge pull request #280 from citizenmatt/ideavim-sidescroll

Caret position and view scrolling fixes

src/com/maddyhome/idea/vim/KeyHandler.java

@@ -33,6 +33,7 @@
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.ui.popup.JBPopupFactory;
 import com.intellij.openapi.ui.popup.ListPopup;
+import com.intellij.openapi.util.Ref;
 import com.maddyhome.idea.vim.action.change.VimRepeater;
 import com.maddyhome.idea.vim.action.change.insert.InsertCompletedDigraphAction;
 import com.maddyhome.idea.vim.action.change.insert.InsertCompletedLiteralAction;
@@ -327,11 +328,20 @@ private void handleEditorReset(@NotNull Editor editor, @NotNull KeyStroke key, f
     if (editorState.getCommandBuilder().isAtDefaultState()) {
       RegisterGroup register = VimPlugin.getRegister();
       if (register.getCurrentRegister() == register.getDefaultRegister()) {
+        boolean indicateError = true;
+
         if (key.getKeyCode() == KeyEvent.VK_ESCAPE) {
+          Ref<Boolean> executed = Ref.create();
           CommandProcessor.getInstance()
-            .executeCommand(editor.getProject(), () -> KeyHandler.executeAction("EditorEscape", context), "", null);
+            .executeCommand(editor.getProject(),
+                            () -> executed.set(KeyHandler.executeAction(IdeActions.ACTION_EDITOR_ESCAPE, context)),
+                            "", null);
+          indicateError = !executed.get();
+        }
+
+        if (indicateError) {
+          VimPlugin.indicateError();
         }
-        VimPlugin.indicateError();
       }
     }
     reset(editor);

src/com/maddyhome/idea/vim/action/VimShortcutKeyAction.kt

@@ -115,8 +115,20 @@ class VimShortcutKeyAction : AnAction(), DumbAware/*, LightEditCompatible*/ {
 
       if ((keyCode == KeyEvent.VK_TAB || keyCode == KeyEvent.VK_ENTER) && editor.appCodeTemplateCaptured()) return false
 
-      if (editor.inInsertMode) { // XXX: <Tab> won't be recorded in macros
+      if (editor.inInsertMode) {
         if (keyCode == KeyEvent.VK_TAB) {
+          // TODO: This stops VimEditorTab seeing <Tab> in insert mode and correctly scrolling the view
+          // There are multiple actions registered for VK_TAB. The important items, in order, are this, the Live
+          // Templates action and TabAction. Returning false in insert mode means that the Live Template action gets to
+          // execute, and this allows Emmet to work (VIM-674). But it also means that the VimEditorTab handle is never
+          // called, so we can't scroll the caret into view correctly.
+          // If we do return true, VimEditorTab handles the Vim side of things and then invokes
+          // IdeActions.ACTION_EDITOR_TAB, which inserts the tab. It also bypasses the Live Template action, and Emmet
+          // no longer works.
+          // This flag is used when recording text entry/keystrokes for repeated insertion. Because we return false and
+          // don't execute the VimEditorTab handler, we don't record tab as an action. Instead, we see an incoming text
+          // change of multiple whitespace characters, which is normally ignored because it's auto-indent content from
+          // hitting <Enter>. When this flag is set, we record the whitespace as the output of the <Tab>
           VimPlugin.getChange().tabAction = true
           return false
         }

src/com/maddyhome/idea/vim/action/change/insert/InsertEnterAction.kt

@@ -22,6 +22,7 @@ import com.intellij.openapi.editor.Editor
 import com.maddyhome.idea.vim.VimPlugin
 import com.maddyhome.idea.vim.command.Command
 import com.maddyhome.idea.vim.command.CommandFlags
+import com.maddyhome.idea.vim.group.MotionGroup
 import com.maddyhome.idea.vim.handler.VimActionHandler
 import com.maddyhome.idea.vim.helper.enumSetOf
 import com.maddyhome.idea.vim.helper.getTopLevelEditor
@@ -34,6 +35,7 @@ class InsertEnterAction : VimActionHandler.SingleExecution() {
 
   override fun execute(editor: Editor, context: DataContext, cmd: Command): Boolean {
     VimPlugin.getChange().processEnter(editor.getTopLevelEditor(), context)
+    MotionGroup.scrollCaretIntoView(editor)
     return true
   }
 }

src/com/maddyhome/idea/vim/action/editor/VimEditorActions.kt

@@ -19,111 +19,67 @@
 package com.maddyhome.idea.vim.action.editor
 
 import com.intellij.openapi.actionSystem.DataContext
+import com.intellij.openapi.actionSystem.IdeActions
 import com.intellij.openapi.editor.Editor
 import com.maddyhome.idea.vim.KeyHandler
 import com.maddyhome.idea.vim.action.ComplicatedKeysAction
 import com.maddyhome.idea.vim.command.Command
 import com.maddyhome.idea.vim.command.CommandFlags
+import com.maddyhome.idea.vim.handler.IdeActionHandler
 import com.maddyhome.idea.vim.handler.VimActionHandler
 import com.maddyhome.idea.vim.helper.enumSetOf
 import java.awt.event.KeyEvent
 import java.util.*
 import javax.swing.KeyStroke
 
-class VimEditorBackSpace : VimActionHandler.SingleExecution(), ComplicatedKeysAction {
-  private val actionName: String = "EditorBackSpace"
-
+class VimEditorBackSpace : IdeActionHandler(IdeActions.ACTION_EDITOR_BACKSPACE), ComplicatedKeysAction {
   override val keyStrokesSet: Set<List<KeyStroke>> = setOf(
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_H, KeyEvent.CTRL_DOWN_MASK)),
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_BACK_SPACE, 0))
   )
-
-  override val type: Command.Type = Command.Type.INSERT
-
-  override fun execute(editor: Editor, context: DataContext, cmd: Command): Boolean {
-    KeyHandler.executeAction(actionName, context)
-    return true
-  }
+  override val type: Command.Type = Command.Type.DELETE
 }
 
-class VimEditorDelete : VimActionHandler.SingleExecution(), ComplicatedKeysAction {
-  private val actionName: String = "EditorDelete"
-
+class VimEditorDelete : IdeActionHandler(IdeActions.ACTION_EDITOR_DELETE), ComplicatedKeysAction {
   override val keyStrokesSet: Set<List<KeyStroke>> = setOf(
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_DELETE, 0))
   )
-
-  override val type: Command.Type = Command.Type.INSERT
-
+  override val type: Command.Type = Command.Type.DELETE
   override val flags: EnumSet<CommandFlags> = enumSetOf(CommandFlags.FLAG_SAVE_STROKE)
-
-  override fun execute(editor: Editor, context: DataContext, cmd: Command): Boolean {
-    KeyHandler.executeAction(actionName, context)
-    return true
-  }
 }
 
-class VimEditorDown : VimActionHandler.SingleExecution(), ComplicatedKeysAction {
-  private val actionName: String = "EditorDown"
-
+class VimEditorDown : IdeActionHandler(IdeActions.ACTION_EDITOR_MOVE_CARET_DOWN), ComplicatedKeysAction {
   override val keyStrokesSet: Set<List<KeyStroke>> = setOf(
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_DOWN, 0)),
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_KP_DOWN, 0))
   )
-
-  override val type: Command.Type = Command.Type.INSERT
-
+  override val type: Command.Type = Command.Type.MOTION
   override val flags: EnumSet<CommandFlags> = enumSetOf(CommandFlags.FLAG_CLEAR_STROKES)
-
-  override fun execute(editor: Editor, context: DataContext, cmd: Command): Boolean {
-    KeyHandler.executeAction(actionName, context)
-    return true
-  }
 }
 
-class VimEditorTab : VimActionHandler.SingleExecution(), ComplicatedKeysAction {
-  private val actionName: String = "EditorTab"
-
+class VimEditorTab : IdeActionHandler(IdeActions.ACTION_EDITOR_TAB), ComplicatedKeysAction {
   override val keyStrokesSet: Set<List<KeyStroke>> = setOf(
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_I, KeyEvent.CTRL_DOWN_MASK)),
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_TAB, 0))
   )
-
   override val type: Command.Type = Command.Type.INSERT
-
   override val flags: EnumSet<CommandFlags> = enumSetOf(CommandFlags.FLAG_SAVE_STROKE)
-
-  override fun execute(editor: Editor, context: DataContext, cmd: Command): Boolean {
-    KeyHandler.executeAction(actionName, context)
-    return true
-  }
 }
 
-class VimEditorUp : VimActionHandler.SingleExecution(), ComplicatedKeysAction {
-  private val actionName: String = "EditorUp"
-
+class VimEditorUp : IdeActionHandler(IdeActions.ACTION_EDITOR_MOVE_CARET_UP), ComplicatedKeysAction {
   override val keyStrokesSet: Set<List<KeyStroke>> = setOf(
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_UP, 0)),
     listOf(KeyStroke.getKeyStroke(KeyEvent.VK_KP_UP, 0))
   )
-
-  override val type: Command.Type = Command.Type.INSERT
-
+  override val type: Command.Type = Command.Type.MOTION
   override val flags: EnumSet<CommandFlags> = enumSetOf(CommandFlags.FLAG_CLEAR_STROKES)
-
-  override fun execute(editor: Editor, context: DataContext, cmd: Command): Boolean {
-    KeyHandler.executeAction(actionName, context)
-    return true
-  }
 }
 
 class VimQuickJavaDoc : VimActionHandler.SingleExecution() {
-  private val actionName: String = "QuickJavaDoc"
-
   override val type: Command.Type = Command.Type.OTHER_READONLY
 
   override fun execute(editor: Editor, context: DataContext, cmd: Command): Boolean {
-    KeyHandler.executeAction(actionName, context)
+    KeyHandler.executeAction(IdeActions.ACTION_QUICK_JAVADOC, context)
     return true
   }
 }

src/com/maddyhome/idea/vim/group/ChangeGroup.java

@@ -170,26 +170,37 @@ public void insertAfterLineEnd(@NotNull Editor editor, @NotNull DataContext cont
   public void insertNewLineAbove(final @NotNull Editor editor, @NotNull DataContext context) {
     if (editor.isOneLineMode()) return;
 
+    // See also EditorStartNewLineBefore. That will move the caret to line start, call EditorEnter to create a new line,
+    // and then move up and call EditorLineEnd. We get better indent positioning by going to the line end of the
+    // previous line and hitting enter, especially with plain text files.
+
+    // Note that we're deliberately bypassing MotionGroup.moveCaret to avoid side effects, most notably unncessary
+    // scrolling
     Set<Caret> firstLiners = new HashSet<>();
     for (Caret caret : editor.getCaretModel().getAllCarets()) {
+      final int offset;
       if (caret.getVisualPosition().line == 0) {
-        MotionGroup.moveCaret(editor, caret, VimPlugin.getMotion().moveCaretToLineStart(editor, caret));
+        // Fake indenting for the first line. Works well for plain text to match the existing indent
+        offset = VimPlugin.getMotion().moveCaretToLineStartSkipLeading(editor, caret);
         firstLiners.add(caret);
       }
       else {
-        MotionGroup.moveCaret(editor, caret, VimPlugin.getMotion().moveCaretVertical(editor, caret, -1));
-        MotionGroup.moveCaret(editor, caret, VimPlugin.getMotion().moveCaretToLineEnd(editor, caret));
+        offset = VimPlugin.getMotion().moveCaretToLineEnd(editor, caret.getLogicalPosition().line - 1, true);
       }
+      caret.moveToOffset(offset);
     }
 
     initInsert(editor, context, CommandState.Mode.INSERT);
     runEnterAction(editor, context);
 
     for (Caret caret : editor.getCaretModel().getAllCarets()) {
       if (firstLiners.contains(caret)) {
-        MotionGroup.moveCaret(editor, caret, VimPlugin.getMotion().moveCaretVertical(editor, caret, -1));
+        final int offset = VimPlugin.getMotion().moveCaretToLineEnd(editor, 0,true);
+        MotionGroup.moveCaret(editor, caret, offset);
       }
     }
+
+    MotionGroup.scrollCaretIntoView(editor);
   }
 
   /**
@@ -236,6 +247,8 @@ public void insertNewLineBelow(final @NotNull Editor editor, final @NotNull Data
 
     initInsert(editor, context, CommandState.Mode.INSERT);
     runEnterAction(editor, context);
+
+    MotionGroup.scrollCaretIntoView(editor);
   }
 
   /**
@@ -258,7 +271,7 @@ private void runEnterAction(Editor editor, @NotNull DataContext context) {
     if (!state.isDotRepeatInProgress()) {
       // While repeating the enter action has been already executed because `initInsert` repeats the input
       final ActionManager actionManager = ActionManager.getInstance();
-      final AnAction action = actionManager.getAction("EditorEnter");
+      final AnAction action = actionManager.getAction(IdeActions.ACTION_EDITOR_ENTER);
       if (action != null) {
         strokes.add(action);
         KeyHandler.executeAction(action, context);
@@ -308,14 +321,11 @@ public void insertPreviousInsert(@NotNull Editor editor, @NotNull DataContext co
   public boolean insertRegister(@NotNull Editor editor, @NotNull DataContext context, char key) {
     final Register register = VimPlugin.getRegister().getRegister(key);
     if (register != null) {
-      final String text = register.getText();
-      if (text != null) {
-        final int length = text.length();
-        for (int i = 0; i < length; i++) {
-          processKey(editor, context, KeyStroke.getKeyStroke(text.charAt(i)));
-        }
-        return true;
+      final List<KeyStroke> keys = register.getKeys();
+      for (KeyStroke k:keys) {
+        processKey(editor, context, k);
       }
+      return true;
     }
 
     return false;
@@ -718,11 +728,17 @@ public boolean deleteCharacter(@NotNull Editor editor, @NotNull Caret caret, int
     final int endOffset = VimPlugin.getMotion().getOffsetOfHorizontalMotion(editor, caret, count, true);
     if (endOffset != -1) {
       final boolean res = deleteText(editor, new TextRange(caret.getOffset(), endOffset), SelectionType.CHARACTER_WISE);
+
       final int pos = caret.getOffset();
       final int norm = EditorHelper.normalizeOffset(editor, caret.getLogicalPosition().line, pos, isChange);
       if (norm != pos || editor.offsetToVisualPosition(norm) != EditorUtil.inlayAwareOffsetToVisualPosition(editor, norm)) {
         MotionGroup.moveCaret(editor, caret, norm);
       }
+      // Always move the caret. Our position might or might not have changed, but an inlay might have been moved to our
+      // location, or deleting the character(s) might have caused us to scroll sideways in long files. Moving the caret
+      // will make sure it's in the right place, and visible
+      final int offset = EditorHelper.normalizeOffset(editor, caret.getLogicalPosition().line, caret.getOffset(), isChange);
+      MotionGroup.moveCaret(editor, caret, offset);
 
       return res;
     }
@@ -843,7 +859,7 @@ public boolean processKey(final @NotNull Editor editor,
       CommandProcessor.getInstance().executeCommand(editor.getProject(), () -> ApplicationManager.getApplication()
                                                       .runWriteAction(() -> KeyHandler.getInstance().getOriginalHandler().execute(editor, key.getKeyChar(), context)),
                                                     "", doc, UndoConfirmationPolicy.DEFAULT, doc);
-
+      MotionGroup.scrollCaretIntoView(editor);
       return true;
     }