passthrough passes through a flag as positional

[dbohdan]

I have found a surprising passthrough behavior in Kong v1.2.1. The following example demonstrates it.

package main

import (
	"encoding/json"
	"fmt"

	"github.com/alecthomas/kong"
)

type cli struct {
	Command string   `arg:"" passthrough:""`
	Args    []string `arg:"" optional:"" name:"args"`
	Timeout float64  `short:"t" default:"0"`
}

func main() {
	var cliConfig cli
	kong.Parse(&cliConfig)

	m, err := json.Marshal(cliConfig)
	if err != nil {
		fmt.Println(err)
		return
	}
	fmt.Println(string(m))
}

$ ./foo -t 9 ls -la
{"Command":"ls","Args":["-la"],"Timeout":9}
$ ./foo -t 9 -la
{"Command":"-l","Args":["a"],"Timeout":9}

Kong takes the first flag and passes it through as a positional argument. It seems like this should be an error. Instead of -l becoming a positional argument, I expected it to be treated as an unknown flag and flags to only be passed through after a non-flag argument.

Edit: I have just learned about github.com/alecthomas/repr. :-)

[boblail]

@dbohdan, in your example, you're defining Command (ls) as something that wants to slurp up and pass through remaining arguments unparsed. The purpose for this feature is often to forward the remaining command line to another executable. It seems to me that Kong is correctly in this case.

Related: #435

[dbohdan]

To me, it seems passthrough on a positional argument should have the semantics of a regular positional argument. The readme currently suggests it does:

If present on a positional argument, it stops flag parsing when encountered, as if -- was processed before. Useful for external command wrappers, like exec. On a command it requires that the command contains only one argument of type []string which is then filled with everything following the command, unparsed.

IMO, it is surprising and contradicts the readme that passthrough treats a flag as a positional argument. I think #435 passthrough and pre-#435 passthrough should be separate tags available in the same version of Kong or an extra tag should activate the old behavior of passthrough. (Since Kong is now stable and you can't change the default.)

Also note that because of the flag behavior the -la argument in my example is not unparsed. When Kong splits the string -la into {"Command":"-l","Args":["a"]}, the result is parsed, just in an unexpected manner.

[alecthomas]

Yep, I agree it is unexpected behaviour.

[i4ki]

I also found another strange behavior when upgrading to v1.4.0. I don't know if it's the same issue described by @dbohdan, if not please let me know and I open another issue.

I don't know if I'm doing something wrong but we used the configuration below for a long time and it did exactly what we wanted:

type runCommandFlags struct { // this defines the "terramate run" command
  // <any other flags here>

  Command    []string `arg:"" name:"cmd" predictor:"file" passthrough:"" help:"Command to execute"`
}

Before v1.2.1, it detected unknown flags correctly in the parser. Example:

$ go install github.com/terramate-io/terramate/cmd/[email protected]
$ terramate run -X --does-not-exist echo ok
Error: parsing cli args [run -X --does-not-exist echo ok]
> unknown flag --does-not-exist
exit status 1

But now:

$ go install github.com/terramate-io/terramate/cmd/[email protected]
$ terramate run -X --does-not-exist echo ok -X
terramate: Entering stack in /
Error: one or more commands failed
> executable file not found in $PATH: running `--does-not-exist echo ok -X` in stack /: --does-not-exist
exit status 1

Another regression was the handling of --.
Before if you did terramate run -X -c -- echo ok, it parsed []string{"echo", "ok"} in the Command field but in v1.2.1 it parses []string{"--", "echo", "ok"}.
We tried to account for this in [email protected] but later we found other issues and we had to rollback to v0.7.1.

Is there a configuration change to retain the old behavior or is this a bug?

[alecthomas]

@i4ki these aren't regressions, this is a behaviour change (modulo the bug this issue is for) - everything gets passed through.

I'd be okay with optionally supporting the old behaviour via eg. passthrough:"partial"

[dbohdan]

I'd be okay with optionally supporting the old behaviour via eg. passthrough:"partial"

This would be great. Besides passthrough:"partial", you could call the old behaviour passthrough:"positional" or passthrough:"noflag". The new behavior could have an alias like passthrough:"all" as a self-documenting alternative to passthrough:"".

[alecthomas]

That would be a breaking change

[dbohdan]

I am suggesting passthrough:"all" as an alias or synonym that will coexist with passthrough:"", not a replacement for it. Would this break something?

[alecthomas]

Ah I see, it wasn't clear that was an alias. Sounds good.

[alecthomas]

That is to say, I missed you saying it :)

[alecthomas]

"partial" is implemented in 96647c3.

[dbohdan]

Great! If this resolves it for @i4ki, you can close the issue.