FIX: positional-arguments parser was too greedy

bkirwi · May 31, 2017 · e9fadd3 · e9fadd3
1 parent 65cf21d
commit e9fadd3
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 10 deletions.
diff --git a/src/main/scala/com/monovore/decline/Parser.scala b/src/main/scala/com/monovore/decline/Parser.scala
@@ -301,20 +301,22 @@ private[decline] object Parser {
       override def result = Result.missingArgument
     }
 
-    case class Arguments(stack: List[String]) extends Accumulator[List[String]] {
+    case class Arguments(stack: List[String]) extends Accumulator[NonEmptyList[String]] {
 
       override def parseArg(arg: String) = {
-        val newStack = arg :: stack
-        val noMore = Pure(Result(Valid { () => Valid(newStack.reverse) }))
-        val yesMore = Arguments(newStack)
+        val noMore = Pure(Result(Valid { () => Valid(NonEmptyList(arg, stack).reverse) }))
+        val yesMore = Arguments(arg :: stack)
         Right(OrElse(noMore, yesMore))
       }
 
       override def parseOption(name: Name) = None
 
       override def parseSub(command: String) = None
 
-      override def result: Result[List[String]] = Result.success(stack.reverse)
+      override def result: Result[NonEmptyList[String]] =
+        NonEmptyList.fromList(stack.reverse)
+          .map(Result.success)
+          .getOrElse(Result.missingArgument)
     }
 
     case class Subcommand[A](name: String, action: Parser[A]) extends Accumulator[A] {
@@ -349,8 +351,7 @@ private[decline] object Parser {
     def repeated[A](opt: Opt[A]): Accumulator[NonEmptyList[A]] = opt match {
       case Opt.Regular(name, _, _, visibility) => Regular(name, visibility)
       case Opt.Flag(name, _, visibility) => Flag(name, visibility)
-      case Opt.Argument(_) =>
-        ap(Argument.map { x: String => xs: List[String] => Validated.valid(NonEmptyList(x, xs)) }, Arguments(Nil))
+      case Opt.Argument(_) => Arguments(Nil)
     }
 
     def fromOpts[A](opts: Opts[A]): Accumulator[A] = opts match {

diff --git a/src/test/scala/com/monovore/decline/ParseSpec.scala b/src/test/scala/com/monovore/decline/ParseSpec.scala
@@ -324,9 +324,10 @@ class ParseSpec extends WordSpec with Matchers with Checkers {
     }
 
     "handle large argument lists" in {
-      val bigNumber = 100000
-      val opts = (Opts.argument[Int]() |@| Opts.arguments[Int]() |@| Opts.argument[Int]()).tupled
-      opts.parse((1 to bigNumber).map(_.toString)) should equal(Valid((1, NonEmptyList(2, (3 to (bigNumber-1)).toList), bigNumber)))
+      for (max <- List(3, 10, 100000)) {
+        val opts = (Opts.argument[Int]() |@| Opts.arguments[Int]() |@| Opts.argument[Int]()).tupled
+        opts.parse((1 to max).map(_.toString)) should equal(Valid((1, NonEmptyList(2, (3 until max).toList), max)))
+      }
     }
   }
 }