diff --git a/commands/internal/arguments/arguments.go b/commands/internal/arguments/arguments.go index fdddb01..ed611ca 100644 --- a/commands/internal/arguments/arguments.go +++ b/commands/internal/arguments/arguments.go @@ -60,12 +60,40 @@ func convertToRelativeIfFilePath(arg string) (string, error) { // environment variable symbolic representation, func Expand(args []string) []string { var results []string - for _, arg := range args { - results = append(results, expandArg(arg)...) + for i, arg := range args { + // for special casing, try to understand if we are expanding a full git + // command invocation, so we can apply edge case rules + if refuseExpandArg(args, i) { + results = append(results, arg) + } else { + results = append(results, expandArg(arg)...) + } } return results } +func refuseExpandArg(args []string, pos int) bool { + // at least two args, first being the registered git cmd, and we're past those + if len(args) <= 2 || args[0] != os.Getenv("SCMPUFF_GIT_CMD") || pos < 2 { + return false + } + + switch args[1] { + case "log": + switch args[pos-1] { + case "-n", "--max-count", "--skip", "--min-parents", "--max-parents": + return true + } + case "blame": + switch args[pos-1] { + case "-L": + return true + } + } + + return false +} + // expandArg "expands" a single argument we received on the command line. // // It's possible that argument represents a numeric file placeholder, in which diff --git a/commands/internal/arguments/arguments_test.go b/commands/internal/arguments/arguments_test.go index f8a5df9..1f1ab2c 100644 --- a/commands/internal/arguments/arguments_test.go +++ b/commands/internal/arguments/arguments_test.go @@ -15,18 +15,42 @@ var testExpandCases = []struct { {"1 3 7", "$e1 $e3 $e7"}, {"1-3 6", "$e1 $e2 $e3 $e6"}, {"seven 2-5 1", "seven $e2 $e3 $e4 $e5 $e1"}, + /* + Testcases for https://github.com/mroth/scmpuff/issues/69, git subcommand + that can accept a dangling numeric argument. This is annoying because it + looks like a scmpuff numeric ref, so we'll have to special case those if + we want to allow this git porcelain CLI parsing behavior. + */ + {"git log -1 1", "git log -1 $e1"}, // was not an issue to begin with + {"git log -n1 2", "git log -n1 $e2"}, // was not an issue to begin with + {"git log -n 1", "git log -n 1"}, // simple case + {"git log -n 1 2", "git log -n 1 $e2"}, // simple case + {"git log --max-count 1 2", "git log --max-count 1 $e2"}, // other log instances + {"git log --skip 1 2", "git log --skip 1 $e2"}, // other log instances + {"git log --min-parents 2 --max-parents 5 1-3", "git log --min-parents 2 --max-parents 5 $e1 $e2 $e3"}, + {"git log -g -n 1 -i 1", "git log -g -n 1 -i $e1"}, // mixed in + {"git rm -n 1", "git rm -n $e1"}, // don't just check for -n, its subcommand specific + // more evaluated edge cases... + // git diff -U does not allow space + // git blame -M and -C take optional integers, but no space + // git blame -L (not ,) is an edge case to handle + {"git blame -L 1 1", "git blame -L 1 $e1"}, + // TODO: git rebase -C is not documented with a space, but needs to be tested to see if it accepts one } func TestExpand(t *testing.T) { + t.Setenv("SCMPUFF_GIT_CMD", "git") for _, tc := range testExpandCases { - // split here to emulate what Cobra will pass us but still write tests with - // normal looking strings - args := strings.Split(tc.args, " ") - expected := strings.Split(tc.expected, " ") - actual := Expand(args) - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("ExpandArgs(%v): expected %v, actual %v", tc.args, expected, actual) - } + t.Run(tc.args, func(t *testing.T) { + // split here to emulate what Cobra will pass us but still write tests with + // normal looking strings + args := strings.Split(tc.args, " ") + expected := strings.Split(tc.expected, " ") + actual := Expand(args) + if !reflect.DeepEqual(actual, expected) { + t.Errorf("ExpandArgs(%v): expected %v, actual %v", tc.args, expected, actual) + } + }) } } @@ -44,7 +68,7 @@ func TestExpandArg(t *testing.T) { for _, tc := range testExpandArgCases { actual := expandArg(tc.arg) if !reflect.DeepEqual(actual, tc.expected) { - t.Fatalf("ExpandArg(%v): expected %v, actual %v", tc.arg, tc.expected, actual) + t.Errorf("ExpandArg(%v): expected %v, actual %v", tc.arg, tc.expected, actual) } } }