VIM-1357+VIM-1566: Use OS shell to run filter command

resources/dictionaries/ideavim.dic

@@ -16,6 +16,9 @@ relativenumber
 scrolljump
 scrolloff
 selectmode
+shellcmdflag
+shellxescape
+shellxquote
 showcmd
 showmode
 sidescroll

resources/messages/IdeaVimBundle.properties

@@ -72,6 +72,9 @@ e_patnotf2=Pattern not found: {0}
 unkopt=Unknown option: {0}
 e_invarg=Invalid argument: {0}
 E475=E475: Invalid argument: {0}
+# Vim's message includes alternate files and the :p:h file name modifier, which we don't support
+# E499: Empty file name for '%' or '#', only works with ":p:h"
+E499=E499: Empty file name for '%'
 E774=E774: 'operatorfunc' is empty
 
 action.VimPluginToggle.text=Vim Emulator

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

@@ -370,6 +370,10 @@ private void turnOnPlugin() {
     // Initialize extensions
     VimExtensionRegistrar.enableDelayedExtensions();
 
+    // Some options' default values are based on values set in .ideavimrc, e.g. 'shellxquote' on Windows when 'shell'
+    // is cmd.exe has a different default to when 'shell' contains "sh"
+    OptionsManager.INSTANCE.completeInitialisation();
+
     // Turing on should be performed after all commands registration
     getSearch().turnOn();
     VimListenerManager.INSTANCE.turnOn();

src/com/maddyhome/idea/vim/ex/handler/CmdFilterHandler.kt

@@ -19,49 +19,87 @@
 package com.maddyhome.idea.vim.ex.handler
 
 import com.intellij.openapi.actionSystem.DataContext
+import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.editor.Editor
+import com.intellij.openapi.progress.ProcessCanceledException
 import com.maddyhome.idea.vim.VimPlugin
 import com.maddyhome.idea.vim.ex.CommandHandler
 import com.maddyhome.idea.vim.ex.ExCommand
 import com.maddyhome.idea.vim.ex.ExException
 import com.maddyhome.idea.vim.ex.ExOutputModel
 import com.maddyhome.idea.vim.ex.flags
+import com.maddyhome.idea.vim.helper.EditorHelper
 import com.maddyhome.idea.vim.helper.MessageHelper
-import com.maddyhome.idea.vim.helper.Msg
-import java.io.IOException
 
 class CmdFilterHandler : CommandHandler.SingleExecution() {
-  override val argFlags = flags(RangeFlag.RANGE_OPTIONAL, ArgumentFlag.ARGUMENT_OPTIONAL, Access.WRITABLE)
+  override val argFlags = flags(RangeFlag.RANGE_OPTIONAL, ArgumentFlag.ARGUMENT_OPTIONAL, Access.SELF_SYNCHRONIZED)
+
   override fun execute(editor: Editor, context: DataContext, cmd: ExCommand): Boolean {
     logger.debug("execute")
 
-    var command = cmd.argument
-    if (command.isEmpty()) {
-      return false
-    }
+    val command = buildString {
+      var inBackslash = false
+      cmd.argument.forEach { c ->
+        when {
+          !inBackslash && c == '!' -> {
+            val last = VimPlugin.getProcess().lastCommand
+            if (last.isNullOrEmpty()) {
+              VimPlugin.showMessage(MessageHelper.message("e_noprev"))
+              return false
+            }
+            append(last)
+          }
+          !inBackslash && c == '%' -> {
+            val virtualFile = EditorHelper.getVirtualFile(editor)
+            if (virtualFile == null) {
+              // Note that we use a slightly different error message to Vim, because we don't support alternate files or file
+              // name modifiers. (I also don't know what the :p:h means)
+              // (Vim) E499: Empty file name for '%' or '#', only works with ":p:h"
+              // (IdeaVim) E499: Empty file name for '%'
+              VimPlugin.showMessage(MessageHelper.message("E499"))
+              return false
+            }
+            append(virtualFile.path)
+          }
+          else -> append(c)
+        }
 
-    if ('!' in command) {
-      val last = VimPlugin.getProcess().lastCommand
-      if (last.isNullOrEmpty()) {
-        VimPlugin.showMessage(MessageHelper.message(Msg.e_noprev))
-        return false
+        inBackslash = c == '\\'
       }
-      command = command.replace("!".toRegex(), last)
+    }
+
+    if (command.isEmpty()) {
+      return false
     }
 
     return try {
       if (cmd.ranges.size() == 0) {
         // Show command output in a window
-        val commandOutput = VimPlugin.getProcess().executeCommand(command, null)
-        ExOutputModel.getInstance(editor).output(commandOutput)
+        VimPlugin.getProcess().executeCommand(editor, command, null)?.let {
+          ExOutputModel.getInstance(editor).output(it)
+        }
         true
       } else {
         // Filter
         val range = cmd.getTextRange(editor, false)
-        VimPlugin.getProcess().executeFilter(editor, range, command)
+        val input = editor.document.charsSequence.subSequence(range.startOffset, range.endOffset)
+        VimPlugin.getProcess().executeCommand(editor, command, input)?.let {
+          ApplicationManager.getApplication().runWriteAction {
+            val start = editor.offsetToLogicalPosition(range.startOffset)
+            val end = editor.offsetToLogicalPosition(range.endOffset)
+            editor.document.replaceString(range.startOffset, range.endOffset, it)
+            val linesFiltered = end.line - start.line
+            if (linesFiltered > 2) {
+              VimPlugin.showMessage("$linesFiltered lines filtered")
+            }
+          }
+        }
+        true
       }
-    } catch (e: IOException) {
+    } catch (e: ProcessCanceledException) {
+      throw ExException("Command terminated")
+    } catch (e: Exception) {
       throw ExException(e.message)
     }
   }

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

@@ -18,25 +18,37 @@
 
 package com.maddyhome.idea.vim.group;
 
+import com.intellij.execution.ExecutionException;
+import com.intellij.execution.configurations.GeneralCommandLine;
+import com.intellij.execution.process.CapturingProcessHandler;
+import com.intellij.execution.process.ProcessAdapter;
+import com.intellij.execution.process.ProcessEvent;
+import com.intellij.execution.process.ProcessOutput;
 import com.intellij.openapi.actionSystem.DataContext;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.progress.ProcessCanceledException;
+import com.intellij.openapi.progress.ProgressIndicator;
+import com.intellij.openapi.progress.ProgressIndicatorProvider;
+import com.intellij.openapi.progress.ProgressManager;
+import com.intellij.util.execution.ParametersListUtil;
 import com.intellij.util.text.CharSequenceReader;
 import com.maddyhome.idea.vim.KeyHandler;
 import com.maddyhome.idea.vim.VimPlugin;
 import com.maddyhome.idea.vim.command.Command;
 import com.maddyhome.idea.vim.command.CommandState;
-import com.maddyhome.idea.vim.common.TextRange;
 import com.maddyhome.idea.vim.ex.CommandParser;
 import com.maddyhome.idea.vim.ex.ExException;
 import com.maddyhome.idea.vim.ex.InvalidCommandException;
 import com.maddyhome.idea.vim.helper.UiHelper;
+import com.maddyhome.idea.vim.option.OptionsManager;
 import com.maddyhome.idea.vim.ui.ex.ExEntryPanel;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import javax.swing.*;
 import java.io.*;
+import java.util.ArrayList;
 
 
 public class ProcessGroup {
@@ -159,38 +171,93 @@ else if (cmd.getRawCount() > 0) {
     return initText;
   }
 
-  public boolean executeFilter(@NotNull Editor editor, @NotNull TextRange range,
-                               @NotNull String command) throws IOException {
-    final CharSequence charsSequence = editor.getDocument().getCharsSequence();
-    final int startOffset = range.getStartOffset();
-    final int endOffset = range.getEndOffset();
-    final String output = executeCommand(command, charsSequence.subSequence(startOffset, endOffset));
-    editor.getDocument().replaceString(startOffset, endOffset, output);
-    return true;
-  }
+  public @Nullable String executeCommand(@NotNull Editor editor, @NotNull String command, @Nullable CharSequence input)
+    throws ExecutionException, ProcessCanceledException {
+
+    // This is a much simplified version of how Vim does this. We're using stdin/stdout directly, while Vim will
+    // redirect to temp files ('shellredir' and 'shelltemp') or use pipes. We don't support 'shellquote', because we're
+    // not handling redirection, but we do use 'shellxquote' and 'shellxescape', because these have defaults that work
+    // better with Windows. We also don't bother using ShellExecute for Windows commands beginning with `start`.
+    // Finally, we're also not bothering with the crazy space and backslash handling of the 'shell' options content.
+    return ProgressManager.getInstance().runProcessWithProgressSynchronously(() -> {
+      final String shell = OptionsManager.INSTANCE.getShell().getValue();
+      final String shellcmdflag = OptionsManager.INSTANCE.getShellcmdflag().getValue();
+      final String shellxescape = OptionsManager.INSTANCE.getShellxescape().getValue();
+      final String shellxquote = OptionsManager.INSTANCE.getShellxquote().getValue();
+
+      // For Win32. See :help 'shellxescape'
+      final String escapedCommand = shellxquote.equals("(")
+                                    ? doEscape(command, shellxescape, "^")
+                                    : command;
+      // Required for Win32+cmd.exe, defaults to "(". See :help 'shellxquote'
+      final String quotedCommand = shellxquote.equals("(")
+                                   ? "(" + escapedCommand + ")"
+                                   : (shellxquote.equals("\"(")
+                                      ? "\"(" + escapedCommand + ")\""
+                                      : shellxquote + escapedCommand + shellxquote);
+
+      final ArrayList<String> commands = new ArrayList<>();
+      commands.add(shell);
+      if (!shellcmdflag.isEmpty()) {
+        // Note that Vim also does a simple whitespace split for multiple parameters
+        commands.addAll(ParametersListUtil.parse(shellcmdflag));
+      }
+      commands.add(quotedCommand);
 
-  public @NotNull String executeCommand(@NotNull String command, @Nullable CharSequence input) throws IOException {
-    if (logger.isDebugEnabled()) {
-      logger.debug("command=" + command);
-    }
+      if (logger.isDebugEnabled()) {
+        logger.debug(String.format("shell=%s shellcmdflag=%s command=%s", shell, shellcmdflag, quotedCommand));
+      }
 
-    final Process process = Runtime.getRuntime().exec(command);
+      final GeneralCommandLine commandLine = new GeneralCommandLine(commands);
+      final CapturingProcessHandler handler = new CapturingProcessHandler(commandLine);
+      if (input != null) {
+        handler.addProcessListener(new ProcessAdapter() {
+          @Override
+          public void startNotified(@NotNull ProcessEvent event) {
+            try {
+              final CharSequenceReader charSequenceReader = new CharSequenceReader(input);
+              final BufferedWriter outputStreamWriter = new BufferedWriter(new OutputStreamWriter(handler.getProcessInput()));
+              copy(charSequenceReader, outputStreamWriter);
+              outputStreamWriter.close();
+            }
+            catch (IOException e) {
+              logger.error(e);
+            }
+          }
+        });
+      }
 
-    if (input != null) {
-      final BufferedWriter outputWriter = new BufferedWriter(new OutputStreamWriter(process.getOutputStream()));
-      copy(new CharSequenceReader(input), outputWriter);
-      outputWriter.close();
-    }
+      final ProgressIndicator progressIndicator = ProgressIndicatorProvider.getInstance().getProgressIndicator();
+      final ProcessOutput output = handler.runProcessWithProgressIndicator(progressIndicator);
 
-    final BufferedReader inputReader = new BufferedReader(new InputStreamReader(process.getInputStream()));
-    final StringWriter writer = new StringWriter();
-    copy(inputReader, writer);
-    writer.close();
+      lastCommand = command;
 
-    lastCommand = command;
-    return writer.toString();
+      if (output.isCancelled()) {
+        // TODO: Vim will use whatever text has already been written to stdout
+        // For whatever reason, we're not getting any here, so just throw an exception
+        throw new ProcessCanceledException();
+      }
+
+      final Integer exitCode = handler.getExitCode();
+      if (exitCode != null && exitCode != 0) {
+        VimPlugin.showMessage("shell returned " + exitCode);
+        VimPlugin.indicateError();
+        return output.getStderr() + output.getStdout();
+      }
+
+      return output.getStdout();
+    }, "IdeaVim - !" + command, true, editor.getProject());
+  }
+
+  private String doEscape(String original, String charsToEscape, String escapeChar) {
+    String result = original;
+    for (char c : charsToEscape.toCharArray()) {
+      result = result.replace("" + c, escapeChar + c);
+    }
+    return result;
   }
 
+  // TODO: Java 10 has a transferTo method we could use instead
   private void copy(@NotNull Reader from, @NotNull Writer to) throws IOException {
     char[] buf = new char[2048];
     int cnt;

src/com/maddyhome/idea/vim/option/OptionsManager.kt

@@ -26,6 +26,7 @@ import com.intellij.codeInsight.template.impl.TemplateManagerImpl
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.diagnostic.debug
 import com.intellij.openapi.editor.Editor
+import com.intellij.openapi.util.SystemInfo
 import com.maddyhome.idea.vim.VimPlugin
 import com.maddyhome.idea.vim.ex.ExOutputModel
 import com.maddyhome.idea.vim.helper.EditorHelper
@@ -70,6 +71,10 @@ object OptionsManager {
   val scrolloff = addOption(NumberOption(ScrollOffData.name, "so", 0))
   val selection = addOption(BoundStringOption("selection", "sel", "inclusive", arrayOf("old", "inclusive", "exclusive")))
   val selectmode = addOption(SelectModeOptionData.option)
+  val shell = addOption(ShellOptionData.option)
+  val shellcmdflag = addOption(ShellCmdFlagOptionData.option)
+  val shellxescape = addOption(ShellXEscapeOptionData.option)
+  val shellxquote = addOption(ShellXQuoteOptionData.option)
   val showcmd = addOption(ToggleOption("showcmd", "sc", true)) // Vim: Off by default on platforms with possibly slow tty. On by default elsewhere.
   val showmode = addOption(ToggleOption("showmode", "smd", false))
   val sidescroll = addOption(NumberOption("sidescroll", "ss", 0))
@@ -99,6 +104,12 @@ object OptionsManager {
 
   val ideatracetime = addOption(ToggleOption("ideatracetime", "ideatracetime", false))
 
+  fun completeInitialisation() {
+    // These options have default values that are based on the contents of 'shell', which can be set in .ideavimrc
+    ShellCmdFlagOptionData.updateDefaultValue(shell)
+    ShellXQuoteOptionData.updateDefaultValue(shell)
+  }
+
   fun isSet(name: String): Boolean {
     val option = getOption(name)
     return option is ToggleOption && option.getValue()
@@ -564,6 +575,71 @@ object ScrollJumpData {
   const val name = "scrolljump"
 }
 
+object ShellOptionData {
+  const val name = "shell"
+
+  val defaultValue = if (SystemInfo.isWindows) "cmd.exe" else System.getenv("SHELL") ?: "sh"
+
+  val option = StringOption(name, "sh", defaultValue)
+}
+
+@Suppress("SpellCheckingInspection")
+object ShellCmdFlagOptionData {
+  const val name = "shellcmdflag"
+
+  fun updateDefaultValue(shell: StringOption) {
+    val resetOption = option.isDefault
+    defaultValue = calculateDefaultValue(shell.value)
+    if (resetOption) {
+      option.resetDefault()
+    }
+  }
+
+  private var defaultValue = calculateDefaultValue(ShellOptionData.defaultValue)
+
+  private fun calculateDefaultValue(shell: String): String {
+    return if (SystemInfo.isWindows && !shell.contains("sh")) "/c" else "-c"
+  }
+
+  val option = object : StringOption(name, "shcf", defaultValue) {
+    override fun getDefaultValue() = ShellCmdFlagOptionData.defaultValue
+  }
+}
+
+@Suppress("SpellCheckingInspection")
+object ShellXEscapeOptionData {
+  const val name = "shellxescape"
+
+  val option = StringOption(name, "sxe", if (SystemInfo.isWindows) "\"&|<>()@^" else "")
+}
+
+@Suppress("SpellCheckingInspection")
+object ShellXQuoteOptionData {
+  const val name = "shellxquote"
+
+  fun updateDefaultValue(shell: StringOption) {
+    val resetOption = option.isDefault
+    defaultValue = calculateDefaultValue(shell.value)
+    if (resetOption) {
+      option.resetDefault()
+    }
+  }
+
+  private var defaultValue = calculateDefaultValue(ShellOptionData.defaultValue)
+
+  private fun calculateDefaultValue(shell: String): String {
+    return when {
+      SystemInfo.isWindows && shell == "cmd.exe" -> "("
+      SystemInfo.isWindows && shell.contains("sh") -> "\""
+      else -> ""
+    }
+  }
+
+  val option = object : StringOption(name, "sxq", defaultValue) {
+    override fun getDefaultValue() = ShellXQuoteOptionData.defaultValue
+  }
+}
+
 object StrictMode {
   val on: Boolean
     get() = OptionsManager.ideastrictmode.isSet