From 9deb17049560a394ff05ec542253d3c292931953 Mon Sep 17 00:00:00 2001
From: Alexander Terp <alexander.terp@gmail.com>
Date: Sat, 28 Dec 2024 14:49:01 +1100
Subject: [PATCH] Fix 'double print bug' for unrecognized flags

We moved to using our own flagset in the previous commit, which lets us
then instantiate it with 'ContinueOnError', and catch the parsing error
ourselves and print it once, rather than being stuck with the pflag bug
that results in the error being double printed
(https://github.com/spf13/pflag/issues/352).
---
 core/runner.go            | 13 +++++++------
 core/testing/args_test.go | 13 +++++--------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/core/runner.go b/core/runner.go
index 1e19688..985fa57 100644
--- a/core/runner.go
+++ b/core/runner.go
@@ -21,7 +21,7 @@ func NewRadRunner(runnerInput RunnerInput) *RadRunner {
 
 func (r *RadRunner) Run() error {
 	// don't fail on unknown flags. they may be intended for the script, which we won't have parsed initially
-	RFlagSet = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
+	RFlagSet = pflag.NewFlagSet(os.Args[0], pflag.ContinueOnError)
 	RFlagSet.ParseErrorsWhitelist.UnknownFlags = true
 
 	RFlagSet.Usage = func() {
@@ -105,13 +105,13 @@ func (r *RadRunner) Run() error {
 	// help not explicitly invoked, so let's try parsing other args
 
 	// re-enable erroring on unknown flags. note: maybe remove for 'catchall' args?
-	// todo if unknown flag passed, pflag handles the error & prints a kinda ugly msg (twice, bug).
-	//  continue allowing unknown flags and then detect ourselves?
 	RFlagSet.ParseErrorsWhitelist.UnknownFlags = false
 
-	// todo apparently this is not recommended, I should be using flagsets? i.e. create new one? I THINK I DO, FOR TESTS?
-	//  RAD-67 will prevent double-error print (see https://github.com/spf13/pflag/issues/352)
-	RFlagSet.Parse(os.Args[1:])
+	// technically re-using the flagset is apparently discouraged, but i've yet to see where it goes wrong
+	err = RFlagSet.Parse(os.Args[1:])
+	if err != nil {
+		RP.UsageErrorExit(err.Error())
+	}
 
 	posArgsIndex := 0
 	if FlagStdinScriptName.Value == "" {
@@ -196,6 +196,7 @@ func (r *RadRunner) createRslArgsFromScript() []RslArg {
 
 	return flags
 }
+
 func readSource(scriptPath string) string {
 	source, err := os.ReadFile(scriptPath)
 	if err != nil {
diff --git a/core/testing/args_test.go b/core/testing/args_test.go
index 4367501..cae68cc 100644
--- a/core/testing/args_test.go
+++ b/core/testing/args_test.go
@@ -1,7 +1,6 @@
 package testing
 
 import (
-	"fmt"
 	"testing"
 )
 
@@ -414,22 +413,20 @@ Script args:
 	resetTestState()
 }
 
-// todo RAD-67 - pflag currently ExitsOnError, I think that's why this test doesn't work
 func TestArgs_InvalidFlagPrintsUsageAndReturnsError(t *testing.T) {
-	t.Skip("TODO: RAD-67")
 	rsl := `
 args:
 	name string
 	age int
 `
-	fmt.Println("hi")
 	setupAndRunCode(t, rsl, "alice", "2", "-s", "--NO-COLOR")
-	expected := `Usage:
- test <name> <age>
+	expected := `unknown shorthand flag: 's' in -s
+Usage:
+  test <name> <age>
 
 Script args:
-     --name string
-     --age int
+      --name string   
+      --age int       
 
 ` + globalFlagHelp
 	assertError(t, 1, expected)