From 9924ec4461bc4fec32a85c16970c9a2a691d41c9 Mon Sep 17 00:00:00 2001
From: Bob Lail <lail@squareup.com>
Date: Tue, 10 Sep 2024 17:51:37 -0700
Subject: [PATCH] fix!: Include `--` in passthrough args (#436)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Given a grammar like this:

```golang
var cli struct {
        Args []string `arg:"" optional:"" passthrough:""`
}
```

If Kong parses `cli foo -- bar`, it will populate `Args` with `[]string{"foo", "--", "bar"}` (including "`--`").
However, if Kong parses `cli -- foo bar`, will populate `Args` with `[]string{"foo", "bar"}` (leaving off `"--"`).

This differs from the behavior of a passthrough Command, where `"--"` is included with the args in both cases.

There are 3 places where `c.endParsing()` is called
1. When `node.Passthrough` is true: https://github.com/alecthomas/kong/blob/5f9c5cc822bdb888a3671c44d4688a6f602ecb90/context.go#L366-L368
2. When `arg.Passthrough` is true: https://github.com/alecthomas/kong/blob/5f9c5cc822bdb888a3671c44d4688a6f602ecb90/context.go#L451-L453
3. When `"--"` is encountered: https://github.com/alecthomas/kong/blob/5f9c5cc822bdb888a3671c44d4688a6f602ecb90/context.go#L384-L387

The first two do not also pop any tokens. The third one does.

This commit makes `c.scan.Pop()` conditional, skipping it when the next positional argument is passthrough.

I believe this will cause Kong to behave a little more consistently — and from my perspective, `--` is relevant for args intended to be passed through! — but it will change the behavior of existing projects that use `arg:"" passthrough:""`.
---
 context.go   |  6 +++++-
 kong_test.go | 10 ++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/context.go b/context.go
index c97aa84..2ed5d28 100644
--- a/context.go
+++ b/context.go
@@ -383,9 +383,13 @@ func (c *Context) trace(node *Node) (err error) { //nolint: gocyclo
 
 				// Indicates end of parsing. All remaining arguments are treated as positional arguments only.
 				case v == "--":
-					c.scan.Pop()
 					c.endParsing()
 
+					// Pop the -- token unless the next positional argument accepts passthrough arguments.
+					if !(positional < len(node.Positional) && node.Positional[positional].Passthrough) {
+						c.scan.Pop()
+					}
+
 				// Long flag.
 				case strings.HasPrefix(v, "--"):
 					c.scan.Pop()
diff --git a/kong_test.go b/kong_test.go
index 7abf0ac..f05fc18 100644
--- a/kong_test.go
+++ b/kong_test.go
@@ -1743,10 +1743,16 @@ func TestPassthroughArgs(t *testing.T) {
 			[]string{"something"},
 		},
 		{
-			"DashDashBeforeRecognizedFlag",
+			"DashDashBetweenArgs",
+			[]string{"foo", "--", "bar"},
+			"",
+			[]string{"foo", "--", "bar"},
+		},
+		{
+			"DashDash",
 			[]string{"--", "--flag", "foobar"},
 			"",
-			[]string{"--flag", "foobar"},
+			[]string{"--", "--flag", "foobar"},
 		},
 		{
 			"UnrecognizedFlagAndArgs",