diff --git a/CHANGELOG.md b/CHANGELOG.md index ce31d40..333b08a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased] +### Fixed +- A value shaped like `--flag=…` is quoted from the `=` onward, so `--preview` + shows `git commit --am='two words'` instead of `git commit '--am=two words'`, + which read as though the flag name were part of the message. Identical single + argument to the shell — rendering only. + ## [0.2.0] — 2026-09-19 Breaking. Placeholder syntax inside script bodies changed; catalogs need editing. diff --git a/docs/contract.md b/docs/contract.md index 0f3e6e6..9becfee 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -94,6 +94,10 @@ no escape syntax. Expanded values are **shell-quoted** for the host shell (`sh` / `cmd`): one argument in is one argument out. `:raw` opts a single placeholder out. +A value shaped like `--flag=…` is quoted from the `=` onward, so a preview +reads `--am='two words'` rather than `'--am=two words'`. Same single argument +to the shell; the flag name is not part of the value. + Windows caveat: `cmd.exe` expands `%VAR%` and `!VAR!` before a command sees its arguments, and no quoting on the command line fully suppresses that. diff --git a/internal/expand/expand_test.go b/internal/expand/expand_test.go index 7459e41..d4c8c0a 100644 --- a/internal/expand/expand_test.go +++ b/internal/expand/expand_test.go @@ -1,6 +1,8 @@ package expand_test import ( + "os/exec" + "runtime" "strings" "testing" @@ -201,3 +203,50 @@ func TestExpandInvocation_unknownCaptureFailsClosed(t *testing.T) { t.Fatalf("%v", err) } } + +// --- flag=value rendering --------------------------------------------------- + +// A shell hands godo "--am=two words" whole, so quoting it whole would read as +// though the flag name were part of the message. Quoting only the value is the +// same single argument and looks like the command a person would type. +func TestQuote_flagAssignmentQuotesOnlyTheValue(t *testing.T) { + cases := []struct{ in, want string }{ + {"--am=two words", "--am='two words'"}, + {"-m=two words", "-m='two words'"}, + {"--pretty-format=a b", "--pretty-format='a b'"}, + {"--am=it's", `--am='it'\''s'`}, + // No space, nothing to quote. + {"--am=plain", "--am=plain"}, + // Not a flag: quoted whole. + {"am=two words", "'am=two words'"}, + {"two words", "'two words'"}, + {"--=two words", "'--=two words'"}, + {"-=a b", "'-=a b'"}, + // Only the first "=" splits; the rest is value. + {"--x=a=b c", "--x='a=b c'"}, + } + for _, tc := range cases { + if runtime.GOOS == "windows" { + t.Skip("posix quoting") + } + if got := expand.Quote(tc.in); got != tc.want { + t.Fatalf("Quote(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +// The rendering must not change what the shell actually receives. +func TestQuote_flagAssignmentIsTheSameSingleArgument(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + for _, v := range []string{"--am=two words", "--am=it's", "--x=a=b c", "--am=;rm -rf /"} { + out, err := exec.Command("/bin/sh", "-c", "printf '[%s]' "+expand.Quote(v)).Output() + if err != nil { + t.Fatalf("%q: %v", v, err) + } + if want := "[" + v + "]"; string(out) != want { + t.Fatalf("%q reached the shell as %s, want %s", v, out, want) + } + } +} diff --git a/internal/expand/quote.go b/internal/expand/quote.go index ed28c76..9ae7696 100644 --- a/internal/expand/quote.go +++ b/internal/expand/quote.go @@ -1,6 +1,7 @@ package expand import ( + "regexp" "runtime" "strings" ) @@ -15,12 +16,42 @@ import ( // that. Values containing % or ! are quoted defensively, but a catalog that // must handle them exactly on Windows should not rely on cmd. func Quote(s string) string { + if prefix, rest, ok := splitFlagAssignment(s); ok { + return prefix + quoteForHost(rest) + } + return quoteForHost(s) +} + +func quoteForHost(s string) string { if runtime.GOOS == "windows" { return quoteWindows(s) } return quotePOSIX(s) } +// reFlagAssignment matches "--flag=" and "-f=" at the start of a value. +var reFlagAssignment = regexp.MustCompile(`^(--?[A-Za-z0-9][A-Za-z0-9_.-]*=)(.*)$`) + +// splitFlagAssignment separates a leading "--flag=" from the value behind it. +// +// A shell hands godo one argument, so "--am=two words" arrives whole and would +// otherwise be quoted whole: git commit '--am=two words'. That reads as though +// the flag name were part of the message. Quoting only the value — +// --am='two words' — is the same single argument to the shell, and is what a +// person would have typed. Preview is the feature; it should look like the +// command it is. +func splitFlagAssignment(s string) (prefix, rest string, ok bool) { + m := reFlagAssignment.FindStringSubmatch(s) + if m == nil { + return "", "", false + } + // The prefix goes in unquoted, so it has to be inert on its own. + if !shellSafePOSIX(m[1]) || (runtime.GOOS == "windows" && !shellSafeWindows(m[1])) { + return "", "", false + } + return m[1], m[2], true +} + // shellSafePOSIX reports whether s can be interpolated bare into an sh line. // Keeping common tokens unquoted is what makes previews readable: a preview of // "godo test -v ./..." should read "go test -v ./...", not "go test '-v' './...'".