Make combine short args that fail to parse go through normal leftover-token code paths (#112)

Previously, we aggressively errored out, which prevented the normal `def
complete` logic from happening: reporting missing arguments, passing
remaining tokens to `Leftover`, etc. This change makes it so that we
just go through the normal code paths when combined short args fail to
parse, making the error reporting more consistent with the rest of
MainArgs and allowing tokens such as `-foo` to be passed to `Leftover`
when possible

Covered by additional unit tests

Author: lihaoyi

mainargs/src/TokenGrouping.scala

@@ -57,7 +57,7 @@ object TokenGrouping {
       var rest2 = rest
       var i = 0
       var currentMap = current
-      var failure: Result.Failure = null
+      var failure = false
 
       while (i < chars.length) {
         val c = chars(i)
@@ -79,7 +79,7 @@ object TokenGrouping {
                   rest2 match {
                     case Nil =>
                       // If there is no next token, it is an error
-                      failure = Result.Failure.MismatchedArguments(incomplete = Some(a))
+                      failure = true
                     case next :: remaining =>
                       currentMap = Util.appendMap(currentMap, a, next)
                       rest2 = remaining
@@ -88,15 +88,14 @@ object TokenGrouping {
               case None =>
                 // If we encounter a character that is neither a short flag or a
                 // short argument, it is an error
-                failure = Result.Failure.MismatchedArguments(unknown = Seq("-" + c.toString))
+                failure = true
             }
             i = chars.length
         }
 
       }
 
-      if (failure != null) Left(failure)
-      else Right((rest2, currentMap))
+      if (failure) None else Some((rest2, currentMap))
     }
 
     def lookupArgMap(k: String, m: Map[String, ArgSig]): Option[(ArgSig, mainargs.TokensReader[_])] = {
@@ -112,8 +111,8 @@ object TokenGrouping {
           // special handling for combined short args of the style "-xvf" or "-j10"
           if (head.startsWith("-") && head.lift(1).exists(c => c != '-')){
               parseCombinedShortTokens(current, head, rest) match{
-                case Left(failure) => failure
-                case Right((rest2, currentMap)) => rec(rest2, currentMap)
+                case None => complete(remaining, current)
+                case Some((rest2, currentMap)) => rec(rest2, currentMap)
             }
 
           } else if (head.startsWith("-") && head.exists(_ != '-')) {

mainargs/test/src/FlagTests.scala

@@ -112,12 +112,10 @@ object FlagTests extends TestSuite {
       test - check(
         List("bool", "-ab"),
         Result.Failure.MismatchedArguments(
+          Vector(new ArgSig(None, Some('b'), None, None, TokensReader.BooleanRead, false, false)),
+          List("-ab"),
           Nil,
-          Nil,
-          Nil,
-          Some(
-            new ArgSig(None, Some('b'), None, None, TokensReader.BooleanRead, false, false)
-          )
+          None
         )
       )
 
@@ -128,7 +126,7 @@ object FlagTests extends TestSuite {
 
       test - check(
         List("str", "-bvalue", "-akey=value"),
-        Result.Failure.MismatchedArguments(Nil, List("-k"), Nil, None)
+        Result.Failure.MismatchedArguments(Nil, List("-akey=value"), Nil, None)
       )
     }
 

mainargs/test/src/PositionalTests.scala

@@ -44,7 +44,31 @@ object PositionalTests extends TestSuite {
     )
     test - check(
       List("-x", "true", "-y", "false", "-z", "false"),
-      Result.Failure.MismatchedArguments(List(), List("-y"), List(), None)
+      Result.Failure.MismatchedArguments(
+        List(
+          ArgSig(
+            None,
+            Some('y'),
+            None,
+            None,
+            TokensReader.BooleanRead,
+            positional = true,
+            hidden = false
+          ),
+          ArgSig(
+            None,
+            Some('z'),
+            None,
+            None,
+            TokensReader.BooleanRead,
+            positional = false,
+            hidden = false
+          )
+        ),
+        List("-y", "false", "-z", "false"),
+        List(),
+        None
+      )
     )
   }
 }

mainargs/test/src/VarargsBaseTests.scala

@@ -113,5 +113,20 @@ trait VarargsBaseTests extends TestSuite {
             ) =>
       }
     }
+
+    test("failedCombinedShortArgsGoToLeftover"){
+      test - check(
+        List("mixedVariadic", "-f", "123", "abc", "xyz"),
+        Result.Success("123abcxyz")
+      )
+      test - check(
+        List("mixedVariadic", "-f123", "456", "abc", "xyz"),
+        Result.Success("123456abcxyz")
+      )
+      test - check(
+        List("mixedVariadic", "-f123", "-unknown", "456", "abc", "xyz"),
+        Result.Success("123-unknown456abcxyz")
+      )
+    }
   }
 }