From 32bd120072cf29d457bb852d7886bf33c104e650 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 16 Jun 2026 08:11:22 +0200 Subject: [PATCH 1/4] Demonstrate that Quote produces invalid Windows quoting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, Quote wraps arguments in bash-style `\"…\"` and rewrites embedded double quotes as `"'"'"`. Neither convention is understood by cmd.exe or CommandLineToArgvW, so commands built from quoted arguments are mis-parsed once they contain quotes or spaces (#5560). Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/oscommands/os_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/commands/oscommands/os_test.go b/pkg/commands/oscommands/os_test.go index 54d9f3a80d4..33bb9a6db9e 100644 --- a/pkg/commands/oscommands/os_test.go +++ b/pkg/commands/oscommands/os_test.go @@ -75,6 +75,9 @@ func TestOSCommandQuoteWindows(t *testing.T) { actual := osCommand.Quote(`hello "test" 'test2'`) + /* EXPECTED: + expected := `"hello \"test\" 'test2'"` + ACTUAL: */ expected := `\"hello "'"'"test"'"'" 'test2'\"` assert.EqualValues(t, expected, actual) From ef9eee9da20b77eca0db465c64891ba382a88521 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 16 Jun 2026 11:55:54 +0200 Subject: [PATCH 2/4] Demonstrate that shell metacharacters are mangled on Windows On Windows, NewShell escapes shell metacharacters (`&`, `|`, `<`, `>`, `%`) with `^` and splits the command into separate arguments. The operators in a custom command therefore never reach cmd as operators, so command chaining (`&&`), pipes, redirection and `%VAR%` expansion all silently break (#2427, #4147, #5113; the stray `^` is also what #3092 reports). Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/oscommands/os_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/commands/oscommands/os_test.go b/pkg/commands/oscommands/os_test.go index 33bb9a6db9e..e15e4de2d55 100644 --- a/pkg/commands/oscommands/os_test.go +++ b/pkg/commands/oscommands/os_test.go @@ -83,6 +83,24 @@ func TestOSCommandQuoteWindows(t *testing.T) { assert.EqualValues(t, expected, actual) } +// On Windows, NewShell must hand the command to cmd.exe verbatim. +func TestNewShellWindowsPassesMetacharactersVerbatim(t *testing.T) { + osCommand := NewDummyOSCommand() + platform := &Platform{OS: "windows", Shell: "cmd", ShellArg: "/c"} + osCommand.Platform = platform + osCommand.Cmd.platform = platform + + command := `echo a && echo b | sort > out.txt < in.txt %PATH%` + + assert.Equal(t, + /* EXPECTED: + []string{"cmd", "/s", "/c", command}, + ACTUAL: */ + []string{"cmd", "/c", "echo", "a", "^&^&", "echo", "b", "^|", "sort", "^>", "out.txt", "^<", "in.txt", "^%PATH^%"}, + osCommand.Cmd.NewShell(command, "").Args(), + ) +} + func TestOSCommandFileType(t *testing.T) { type scenario struct { path string From eb9ca3c25f4cc93380a2ddec191d4a8b178bb1d6 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 16 Jun 2026 08:16:54 +0200 Subject: [PATCH 3/4] Fix quoting of shell commands on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lazygit builds a shell command by interpolating Quote'd arguments into a template and running the result via `cmd /c`. Several things were wrong on Windows: - Quote emitted bash-style `\"…\"` quoting, which cmd.exe doesn't understand. Making it usable at all previously required a fragile round-trip through str.ToArgv and re-escaping. - The assembled command line was handed to `cmd /c` without `/s`, so cmd's default rules stripped the wrong quotes once the line contained more than two of them (e.g. a quoted editor path at a location with spaces, plus a quoted filename that also contains spaces). - Shell metacharacters were escaped with `^` (`&` → `^&`, etc.), which neutralised command chaining, pipes, redirection and `%VAR%` expansion in custom commands. Quote now emits the standard Windows convention directly, and NewShell hands cmd.exe the fully-assembled line verbatim via SysProcAttr.CmdLine, wrapped as `cmd /s /c ""`. The /s flag strips exactly the outer quote pair we add, leaving each argument's own quoting intact. With the `^` escaping gone, metacharacters in a custom command reach cmd as the author intended; this also removes the spurious `^` reported in #3092. Fixes #5560 Fixes #2427 Fixes #4147 Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/oscommands/cmd_obj_builder.go | 92 +++++++++++++------ .../oscommands/os_default_platform.go | 5 + pkg/commands/oscommands/os_test.go | 6 -- pkg/commands/oscommands/os_windows.go | 17 ++++ pkg/commands/oscommands/os_windows_test.go | 10 +- 5 files changed, 90 insertions(+), 40 deletions(-) diff --git a/pkg/commands/oscommands/cmd_obj_builder.go b/pkg/commands/oscommands/cmd_obj_builder.go index fde64258210..9084fd994cf 100644 --- a/pkg/commands/oscommands/cmd_obj_builder.go +++ b/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 ""`. 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() } diff --git a/pkg/commands/oscommands/os_default_platform.go b/pkg/commands/oscommands/os_default_platform.go index 06684434e7e..5e73c994f30 100644 --- a/pkg/commands/oscommands/os_default_platform.go +++ b/pkg/commands/oscommands/os_default_platform.go @@ -40,6 +40,11 @@ func (c *OSCommand) UpdateWindowTitle() error { return nil } +// setRawCmdLine is the non-Windows no-op counterpart of the Windows shim +// (see the comment there). NewShell's shell-building logic is portable, so +// this call is reached on every host; only the Windows build does anything. +func setRawCmdLine(cmd *exec.Cmd, cmdLine string) {} + func TerminateProcessGracefully(cmd *exec.Cmd) error { if cmd.Process == nil { return nil diff --git a/pkg/commands/oscommands/os_test.go b/pkg/commands/oscommands/os_test.go index e15e4de2d55..1ccfbc02580 100644 --- a/pkg/commands/oscommands/os_test.go +++ b/pkg/commands/oscommands/os_test.go @@ -75,10 +75,7 @@ func TestOSCommandQuoteWindows(t *testing.T) { actual := osCommand.Quote(`hello "test" 'test2'`) - /* EXPECTED: expected := `"hello \"test\" 'test2'"` - ACTUAL: */ - expected := `\"hello "'"'"test"'"'" 'test2'\"` assert.EqualValues(t, expected, actual) } @@ -93,10 +90,7 @@ func TestNewShellWindowsPassesMetacharactersVerbatim(t *testing.T) { command := `echo a && echo b | sort > out.txt < in.txt %PATH%` assert.Equal(t, - /* EXPECTED: []string{"cmd", "/s", "/c", command}, - ACTUAL: */ - []string{"cmd", "/c", "echo", "a", "^&^&", "echo", "b", "^|", "sort", "^>", "out.txt", "^<", "in.txt", "^%PATH^%"}, osCommand.Cmd.NewShell(command, "").Args(), ) } diff --git a/pkg/commands/oscommands/os_windows.go b/pkg/commands/oscommands/os_windows.go index 605ed768275..bd4cc515190 100644 --- a/pkg/commands/oscommands/os_windows.go +++ b/pkg/commands/oscommands/os_windows.go @@ -5,8 +5,25 @@ import ( "os" "os/exec" "path/filepath" + "syscall" ) +// setRawCmdLine hands cmd.exe the exact command line we built, bypassing +// os/exec's default composition (which quotes args with the +// CommandLineToArgvW `\"` convention that cmd.exe doesn't understand). +// +// The shell-building logic in NewShell is portable and dispatches on +// platform.OS, which keeps it (and its quoting) unit-testable on any host. +// Assigning SysProcAttr.CmdLine is the only step that needs a Windows-only +// field, so it's the single piece split out behind a build tag; every other +// platform gets the no-op in os_default_platform.go. +func setRawCmdLine(cmd *exec.Cmd, cmdLine string) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.CmdLine = cmdLine +} + func GetPlatform() *Platform { return &Platform{ OS: "windows", diff --git a/pkg/commands/oscommands/os_windows_test.go b/pkg/commands/oscommands/os_windows_test.go index 60ba495bf4e..495e23f7257 100644 --- a/pkg/commands/oscommands/os_windows_test.go +++ b/pkg/commands/oscommands/os_windows_test.go @@ -20,7 +20,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "test", runner: NewFakeRunner(t). - ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", errors.New("error")), + ExpectArgs([]string{"cmd", "/s", "/c", `start "" "test"`}, "", errors.New("error")), test: func(err error) { assert.Error(t, err) }, @@ -28,7 +28,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "test", runner: NewFakeRunner(t). - ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", nil), + ExpectArgs([]string{"cmd", "/s", "/c", `start "" "test"`}, "", nil), test: func(err error) { assert.NoError(t, err) }, @@ -36,7 +36,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "filename with spaces", runner: NewFakeRunner(t). - ExpectArgs([]string{"cmd", "/c", "start", "", "filename with spaces"}, "", nil), + ExpectArgs([]string{"cmd", "/s", "/c", `start "" "filename with spaces"`}, "", nil), test: func(err error) { assert.NoError(t, err) }, @@ -44,7 +44,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "let's_test_with_single_quote", runner: NewFakeRunner(t). - ExpectArgs([]string{"cmd", "/c", "start", "", "let's_test_with_single_quote"}, "", nil), + ExpectArgs([]string{"cmd", "/s", "/c", `start "" "let's_test_with_single_quote"`}, "", nil), test: func(err error) { assert.NoError(t, err) }, @@ -52,7 +52,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) { { filename: "$USER.txt", runner: NewFakeRunner(t). - ExpectArgs([]string{"cmd", "/c", "start", "", "$USER.txt"}, "", nil), + ExpectArgs([]string{"cmd", "/s", "/c", `start "" "$USER.txt"`}, "", nil), test: func(err error) { assert.NoError(t, err) }, From 93d2c0b9295eee0b9c49438b076f655d3c752c11 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 16 Jun 2026 08:27:02 +0200 Subject: [PATCH 4/4] Add end-to-end test for shell command quoting on Windows The unit tests only assert the arguments lazygit constructs; they can't catch cmd.exe's own quote-stripping, which is where #5560 actually manifested. This test builds a small editor executable, places it and the file it opens at paths containing spaces, runs it through real cmd.exe via NewShell, and checks the editor received the intended args. It runs only on Windows. Co-Authored-By: Antoine Gaudreau Simard Co-Authored-By: Claude Opus 4.8 (1M context) --- .../oscommands/new_shell_windows_test.go | 389 ++++++++++++++++++ 1 file changed, 389 insertions(+) create mode 100644 pkg/commands/oscommands/new_shell_windows_test.go diff --git a/pkg/commands/oscommands/new_shell_windows_test.go b/pkg/commands/oscommands/new_shell_windows_test.go new file mode 100644 index 00000000000..c7fe5aef116 --- /dev/null +++ b/pkg/commands/oscommands/new_shell_windows_test.go @@ -0,0 +1,389 @@ +//go:build windows + +package oscommands + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/stretchr/testify/assert" +) + +// These tests run only on Windows because they exercise real cmd.exe +// quote-parsing behaviour, which has only been a problem on Windows. + +// makeWindowsShellBuilder returns a CmdObjBuilder configured for a real +// Windows cmd shell, bypassing the test "dummy" platform (which is darwin). +func makeWindowsShellBuilder() *CmdObjBuilder { + log := utils.NewDummyLog() + return &CmdObjBuilder{ + runner: &cmdObjRunner{log: log, guiIO: NewNullGuiIO(log)}, + platform: &Platform{OS: "windows", Shell: "cmd", ShellArg: "/c"}, + } +} + +// fakeEditorSrc is a minimal Go program that records the args it received, +// one per line, to marker.txt in its own directory. Using a real .exe (not a +// .bat) means args are parsed by Go's runtime via CommandLineToArgvW — the +// same algorithm used by ~all real Windows GUI editors. A .bat would parse +// args via cmd.exe's own rules, which can hide bugs that affect editors. +const fakeEditorSrc = `package main + +import ( + "os" + "path/filepath" + "strings" +) + +func main() { + exe, err := os.Executable() + if err != nil { + os.Exit(2) + } + marker := filepath.Join(filepath.Dir(exe), "marker.txt") + body := strings.Join(os.Args[1:], "\n") + if err := os.WriteFile(marker, []byte(body), 0o644); err != nil { + os.Exit(3) + } +} +` + +var ( + fakeEditorOnce sync.Once + fakeEditorBytes []byte + fakeEditorErr error +) + +// loadFakeEditorBytes builds the fake editor exactly once per test process +// and returns its bytes. Tests then drop a copy at a path containing spaces. +func loadFakeEditorBytes(t *testing.T) []byte { + t.Helper() + fakeEditorOnce.Do(func() { + buildDir, err := os.MkdirTemp("", "lazygit-fake-editor-build-*") + if err != nil { + fakeEditorErr = err + return + } + defer os.RemoveAll(buildDir) + + srcPath := filepath.Join(buildDir, "main.go") + binPath := filepath.Join(buildDir, "fake-editor.exe") + if err := os.WriteFile(srcPath, []byte(fakeEditorSrc), 0o644); err != nil { + fakeEditorErr = err + return + } + cmd := exec.Command("go", "build", "-o", binPath, srcPath) + cmd.Stderr = os.Stderr + if err := cmd.Run(); err != nil { + fakeEditorErr = err + return + } + fakeEditorBytes, fakeEditorErr = os.ReadFile(binPath) + }) + if fakeEditorErr != nil { + t.Fatalf("failed to build fake editor helper: %v", fakeEditorErr) + } + return fakeEditorBytes +} + +// placeFakeEditor builds the fake editor and places it at a path containing +// a space (mirroring `C:\Program Files\...`). The marker the editor writes +// lives next to the exe. +func placeFakeEditor(t *testing.T) (exe, markerFile string) { + t.Helper() + bin := loadFakeEditorBytes(t) + exeDir := filepath.Join(t.TempDir(), "Program Files", "FakeEditor") + if err := os.MkdirAll(exeDir, 0o755); err != nil { + t.Fatalf("mkdir exeDir: %v", err) + } + exe = filepath.Join(exeDir, "fake-editor.exe") + markerFile = filepath.Join(exeDir, "marker.txt") + if err := os.WriteFile(exe, bin, 0o755); err != nil { + t.Fatalf("write fake editor: %v", err) + } + return exe, markerFile +} + +// placeTargetFile creates a file at // with +// a trivial body. Use a dirName containing a space (e.g. "my repo") to put +// the file at a path with spaces. +func placeTargetFile(t *testing.T, dirName, basename string) string { + t.Helper() + dir := filepath.Join(t.TempDir(), dirName) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("mkdir target dir: %v", err) + } + target := filepath.Join(dir, basename) + if err := os.WriteFile(target, []byte("hello"), 0o644); err != nil { + t.Fatalf("write target: %v", err) + } + return target +} + +// setupFakeEditor is a convenience wrapper for the common case: editor at a +// spacey path AND target file at a spacey path — the conditions that +// trigger the cmd.exe quote-stripping bug. +func setupFakeEditor(t *testing.T) (fakeExe, targetFile, markerFile string) { + t.Helper() + fakeExe, markerFile = placeFakeEditor(t) + targetFile = placeTargetFile(t, "my repo", "file.txt") + return fakeExe, targetFile, markerFile +} + +// resolveTemplate mirrors what pkg/commands/git_commands/file.go does: it +// substitutes {{filename}} with the Windows-quoted filename and {{line}} +// with a line number. +func resolveTemplate(builder *CmdObjBuilder, template, filename, line string) string { + out := strings.ReplaceAll(template, "{{filename}}", builder.Quote(filename)) + out = strings.ReplaceAll(out, "{{line}}", line) + return out +} + +// readMarkerArgs reads the args the fake editor recorded. Each arg is on its +// own line (so an arg that itself contains a space stays one element). An +// empty file means the editor ran with zero args. +func readMarkerArgs(t *testing.T, markerFile string) []string { + t.Helper() + data, err := os.ReadFile(markerFile) + if err != nil { + t.Fatalf("marker file was not written; the fake editor never ran: %v", err) + } + s := strings.TrimRight(string(data), "\r\n") + if s == "" { + return nil + } + return strings.Split(s, "\n") +} + +func TestNewShell_QuotedExePath_FilenameWithSpaces_EditAtLineAndWait(t *testing.T) { + builder := makeWindowsShellBuilder() + fakeExe, targetFile, markerFile := setupFakeEditor(t) + + template := `"` + fakeExe + `" -multiInst -nosession -noPlugin -n{{line}} {{filename}}` + cmdStr := resolveTemplate(builder, template, targetFile, "42") + + out, err := builder.NewShell(cmdStr, "").GetCmd().CombinedOutput() + if err != nil { + t.Fatalf("shell command failed: %v\ncmd.exe output:\n%s", err, string(out)) + } + + assert.Equal(t, + []string{"-multiInst", "-nosession", "-noPlugin", "-n42", targetFile}, + readMarkerArgs(t, markerFile), + ) +} + +func TestNewShell_QuotedExePath_FilenameWithSpaces_EditAtLine(t *testing.T) { + builder := makeWindowsShellBuilder() + fakeExe, targetFile, markerFile := setupFakeEditor(t) + + template := `"` + fakeExe + `" -n{{line}} {{filename}}` + cmdStr := resolveTemplate(builder, template, targetFile, "42") + + out, err := builder.NewShell(cmdStr, "").GetCmd().CombinedOutput() + if err != nil { + t.Fatalf("shell command failed: %v\ncmd.exe output:\n%s", err, string(out)) + } + + assert.Equal(t, + []string{"-n42", targetFile}, + readMarkerArgs(t, markerFile), + ) +} + +func TestNewShell_QuotedExePath_FilenameWithSpaces_Edit(t *testing.T) { + builder := makeWindowsShellBuilder() + fakeExe, targetFile, markerFile := setupFakeEditor(t) + + template := `"` + fakeExe + `" {{filename}}` + cmdStr := resolveTemplate(builder, template, targetFile, "") + + out, err := builder.NewShell(cmdStr, "").GetCmd().CombinedOutput() + if err != nil { + t.Fatalf("shell command failed: %v\ncmd.exe output:\n%s", err, string(out)) + } + + assert.Equal(t, + []string{targetFile}, + readMarkerArgs(t, markerFile), + ) +} + +// Sanity check: for a filename WITHOUT spaces the same templates already work, +// because the resulting cmd.exe line has exactly two quote characters and +// cmd /c keeps them. This pins the difference down to filename quoting and +// guards against a regression where the no-spaces case starts failing too. +func TestNewShell_QuotedExePath_FilenameWithoutSpaces_StillWorks(t *testing.T) { + builder := makeWindowsShellBuilder() + fakeExe, markerFile := placeFakeEditor(t) + plainTarget := placeTargetFile(t, "repo", "plain.txt") // no-space dir + + template := `"` + fakeExe + `" -multiInst -nosession -noPlugin -n{{line}} {{filename}}` + cmdStr := resolveTemplate(builder, template, plainTarget, "42") + + out, err := builder.NewShell(cmdStr, "").GetCmd().CombinedOutput() + if err != nil { + t.Fatalf("shell command failed: %v\ncmd.exe output:\n%s", err, string(out)) + } + + assert.Equal(t, + []string{"-multiInst", "-nosession", "-noPlugin", "-n42", plainTarget}, + readMarkerArgs(t, markerFile), + ) +} + +// TestNewShell_VarietyOfEditorTemplates exercises NewShell with a range of +// realistic editor templates, all with the trigger conditions of the bug +// (quoted exe at a spacey path + filename at a spacey path). Each subtest +// asserts the editor receives the exact args lazygit intended. +// +// Args in `wantArgs` may use the literal "" placeholder; it gets +// substituted with the resolved target file path before comparison. +func TestNewShell_VarietyOfEditorTemplates(t *testing.T) { + const filePlaceholder = "" + + cases := []struct { + name string + template string // stands for the fake editor's full path + line string + wantArgs []string + }{ + { + name: "vim/nvim style: +line filename", + template: `"" +{{line}} {{filename}}`, + line: "42", + wantArgs: []string{"+42", filePlaceholder}, + }, + { + name: "emacs-like with explicit +N", + template: `"" +{{line}} -nw {{filename}}`, + line: "7", + wantArgs: []string{"+7", "-nw", filePlaceholder}, + }, + { + name: "long flag with =value", + template: `"" --line={{line}} --tab-size=4 {{filename}}`, + line: "42", + wantArgs: []string{"--line=42", "--tab-size=4", filePlaceholder}, + }, + { + name: "many short and long flags before filename", + template: `"" -a -b -c --foo --bar -n{{line}} {{filename}}`, + line: "42", + wantArgs: []string{"-a", "-b", "-c", "--foo", "--bar", "-n42", filePlaceholder}, + }, + { + name: "flag after filename", + template: `"" {{filename}} --readonly`, + line: "", + wantArgs: []string{filePlaceholder, "--readonly"}, + }, + { + name: "single short flag attached to value", + template: `"" -n{{line}} {{filename}}`, + line: "1", + wantArgs: []string{"-n1", filePlaceholder}, + }, + { + name: "no flags, just filename", + template: `"" {{filename}}`, + line: "", + wantArgs: []string{filePlaceholder}, + }, + { + name: "flag with separate value (space-separated)", + template: `"" --goto {{line}} {{filename}}`, + line: "42", + wantArgs: []string{"--goto", "42", filePlaceholder}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + builder := makeWindowsShellBuilder() + fakeExe, targetFile, markerFile := setupFakeEditor(t) + + template := strings.ReplaceAll(tc.template, "", fakeExe) + cmdStr := resolveTemplate(builder, template, targetFile, tc.line) + + out, err := builder.NewShell(cmdStr, "").GetCmd().CombinedOutput() + if err != nil { + t.Fatalf("shell command failed: %v\ncmd.exe output:\n%s", err, string(out)) + } + + want := make([]string, len(tc.wantArgs)) + for i, a := range tc.wantArgs { + want[i] = strings.ReplaceAll(a, filePlaceholder, targetFile) + } + assert.Equal(t, want, readMarkerArgs(t, markerFile)) + }) + } +} + +// TestNewShell_FilenameSpecialCharacters varies the basename of the target +// file across characters that are legal in Windows filenames but might +// interact badly with cmd.exe / Quote(): parentheses, brackets, single +// quote, comma, semicolon, equals, etc. The exe is at a spacey path and +// the target dir has spaces, so the bug-trigger conditions are still met. +func TestNewShell_FilenameSpecialCharacters(t *testing.T) { + cases := []struct { + name string + basename string + }{ + {"parens", "file (1).txt"}, + {"brackets", "file[v2].txt"}, + {"single quote", "it's a file.txt"}, + {"comma", "a,b,c.txt"}, + {"semicolon", "a;b.txt"}, + {"equals", "key=value.txt"}, + {"plus", "a+b.txt"}, + {"hash", "issue#42.txt"}, + {"at sign", "user@host.txt"}, + {"tilde", "~backup.txt"}, + {"dot leading", ".gitignore.txt"}, + {"multiple dots", "v1.2.3.txt"}, + {"dash leading", "-flag-looking.txt"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + builder := makeWindowsShellBuilder() + fakeExe, markerFile := placeFakeEditor(t) + targetFile := placeTargetFile(t, "my repo", tc.basename) + + template := `"` + fakeExe + `" -n{{line}} {{filename}}` + cmdStr := resolveTemplate(builder, template, targetFile, "42") + + out, err := builder.NewShell(cmdStr, "").GetCmd().CombinedOutput() + if err != nil { + t.Fatalf("shell command failed: %v\ncmd.exe output:\n%s", err, string(out)) + } + + assert.Equal(t, + []string{"-n42", targetFile}, + readMarkerArgs(t, markerFile), + ) + }) + } +} + +// Command chaining with && must work: cmd /s /c runs the assembled line verbatim, +// so cmd treats && as a separator and runs both commands. The two echoes +// therefore produce two separate output lines. +func TestNewShell_CommandChaining(t *testing.T) { + builder := makeWindowsShellBuilder() + + out, err := builder.NewShell("echo first&&echo second", "").GetCmd().CombinedOutput() + if err != nil { + t.Fatalf("shell command failed: %v\ncmd.exe output:\n%s", err, string(out)) + } + + normalized := strings.ReplaceAll(string(out), "\r\n", "\n") + lines := strings.Split(strings.TrimSpace(normalized), "\n") + assert.Equal(t, []string{"first", "second"}, lines) +}