fix(arguments): handle numeric single flag edge cases in git porcelain

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

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<n> does not allow space
+ // git blame -M and -C take optional integers, but no space
+ // git blame -L <start> (not <start>,<end>) is an edge case to handle
+ {"git blame -L 1 1", "git blame -L 1 $e1"},
+ // TODO: git rebase -C<n> 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)
  }
  }
 }