Fix quoting of shell commands on Windows

pkg/commands/oscommands/cmd_obj_builder.go

@@ -48,26 +48,34 @@ func (self *CmdObjBuilder) NewShell(commandStr string, shellFunctionsFile string
 	if len(shellFunctionsFile) > 0 {
 		commandStr = fmt.Sprintf("%ssource %s\n%s", self.platform.PrefixForShellFunctionsFile, shellFunctionsFile, commandStr)
 	}
-	quotedCommand := self.quotedCommandString(commandStr)
+
+	if self.platform.OS == "windows" {
+		return self.newWindowsShell(commandStr)
+	}
+
+	quotedCommand := self.Quote(commandStr)
 	cmdArgs := str.ToArgv(fmt.Sprintf("%s %s %s", self.platform.Shell, self.platform.ShellArg, quotedCommand))
 
 	return self.New(cmdArgs)
 }
 
-func (self *CmdObjBuilder) quotedCommandString(commandStr string) string {
-	// Windows does not seem to like quotes around the command
-	if self.platform.OS == "windows" {
-		return strings.NewReplacer(
-			"^", "^^",
-			"&", "^&",
-			"|", "^|",
-			"<", "^<",
-			">", "^>",
-			"%", "^%",
-		).Replace(commandStr)
-	}
+// newWindowsShell wraps the command in `cmd.exe /s /c "<command>"`. The /s
+// flag tells cmd to strip exactly the outermost pair of quotes and pass the
+// rest through unchanged, which preserves any quoting the command itself
+// contains (e.g. `"C:\Program Files\my-editor.exe" file.txt`). Without /s,
+// cmd's default rules drop the wrong quotes once the command line contains
+// more than two of them.
+//
+// We bypass Go's standard arg quoting via SysProcAttr.CmdLine: it follows the
+// CommandLineToArgvW convention (`\"` for inner quotes), but cmd.exe doesn't.
+func (self *CmdObjBuilder) newWindowsShell(commandStr string) *CmdObj {
+	args := []string{self.platform.Shell, "/s", self.platform.ShellArg, commandStr}
+	cmdObj := self.New(args)
 
-	return self.Quote(commandStr)
+	cmdLine := fmt.Sprintf(`%s /s %s "%s"`, self.platform.Shell, self.platform.ShellArg, commandStr)
+	setRawCmdLine(cmdObj.GetCmd(), cmdLine)
+
+	return cmdObj
 }
 
 func (self *CmdObjBuilder) CloneWithNewRunner(decorate func(ICmdObjRunner) ICmdObjRunner) *CmdObjBuilder {
@@ -80,21 +88,47 @@ func (self *CmdObjBuilder) CloneWithNewRunner(decorate func(ICmdObjRunner) ICmdO
 }
 
 func (self *CmdObjBuilder) Quote(message string) string {
-	var quote string
 	if self.platform.OS == "windows" {
-		quote = `\"`
-		message = strings.NewReplacer(
-			`"`, `"'"'"`,
-			`\"`, `\\"`,
-		).Replace(message)
-	} else {
-		quote = `"`
-		message = strings.NewReplacer(
-			`\`, `\\`,
-			`"`, `\"`,
-			`$`, `\$`,
-			"`", "\\`",
-		).Replace(message)
+		return quoteForWindows(message)
+	}
+	message = strings.NewReplacer(
+		`\`, `\\`,
+		`"`, `\"`,
+		`$`, `\$`,
+		"`", "\\`",
+	).Replace(message)
+	return `"` + message + `"`
+}
+
+// quoteForWindows encodes a value using the standard Windows command-line
+// convention (the algorithm behind syscall.EscapeArg, reimplemented here so
+// it's available on all platforms). The result is always wrapped in double
+// quotes so cmd.exe and CommandLineToArgvW treat it as a single argument
+// regardless of what shell metacharacters it contains.
+func quoteForWindows(s string) string {
+	var b strings.Builder
+	b.WriteByte('"')
+	slashes := 0
+	for i := range len(s) {
+		c := s[i]
+		switch c {
+		case '\\':
+			slashes++
+			b.WriteByte(c)
+		case '"':
+			for ; slashes > 0; slashes-- {
+				b.WriteByte('\\')
+			}
+			b.WriteByte('\\')
+			b.WriteByte(c)
+		default:
+			slashes = 0
+			b.WriteByte(c)
+		}
+	}
+	for ; slashes > 0; slashes-- {
+		b.WriteByte('\\')
 	}
-	return quote + message + quote
+	b.WriteByte('"')
+	return b.String()
 }