flags : return HelpError and print at the callsite

cat/cat.go

@@ -1,6 +1,7 @@
 package cat
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"os"
@@ -82,7 +83,14 @@ func cat(files []string, opts options) (string, error) {
 
 func Cat(args []string) {
 	opts := initOpts()
-	flags.ParseArgs(CMD, SUMMARY, args, &opts)
+	if err := flags.ParseArgs(CMD, SUMMARY, args, &opts); err != nil {
+		var herr flags.HelpError
+		if errors.As(err, &herr) {
+			fmt.Println(herr)
+			return
+		}
+		log.Fatalf("um %s: %s", CMD, err)
+	}
 	files, err := pipe.FileListSplitMaybeStdin(opts.Filelist.Val)
 	if err != nil {
 		log.Fatalf("um %s: %s", CMD, err)

flags/flags.go

@@ -155,17 +155,19 @@ func expandOpts(opts any) []Flag {
 }
 
 // internal for type safety testing
-func parseArgsInternal(sub cmd.Subcommand, summary string, args []string, opts any, flags []Flag) {
+func parseArgsInternal(sub cmd.Subcommand, summary string, args []string, opts any, flags []Flag) error {
 argloop:
 	for i := 0; i < len(args); i++ {
 		arg := args[i]
 		for j, f := range flags {
 			if f.Match(arg, i, j) {
 				if f.IsHelp() {
-					Help(sub, summary, opts)
+					// so Help.Val is true to avoid confusion:
+					f.Set(arg)
+					return Help(sub, summary, opts)
 				}
 				if !f.Valid(args, i) {
-					HelpMissingAssignment(sub, arg)
+					return HelpMissingAssignment(sub, arg)
 				}
 				i = f.MaybeIncrement(i)
 				f.Set(args[i])
@@ -173,13 +175,13 @@ argloop:
 			}
 		}
 		// NOTE: if we didn't skip ahead, arg didn't match:
-		HelpInvalidArg(sub, arg)
+		return HelpInvalidArg(sub, arg)
-		Help(sub, summary, opts)
 	}
+	return nil
 }
 
 // assign values of args to opts struct using Flag interface methods
-func ParseArgs(sub cmd.Subcommand, summary string, args []string, opts any) {
+func ParseArgs(sub cmd.Subcommand, summary string, args []string, opts any) error {
 	flags := expandOpts(opts)
-	parseArgsInternal(sub, summary, args, opts, flags)
+	return parseArgsInternal(sub, summary, args, opts, flags)
 }

flags/flags_test.go

@@ -46,11 +46,20 @@ func TestParseArgsInternal(t *testing.T) {
 
 }
 
-// TODO: return errors from Help routines and print at top level, so I can test those cases
+func TestParseArgsHelp(t *testing.T) {
+	args := []string{"--help"}
+	opts := initOpts()
+	err := ParseArgs(CMD, SUMMARY, args, &opts)
+	assert.ErrorContains(t, err, "um TEST [descriptor] [tags] [--source] [--write]\n\nSUMMARY")
+	// we set the Val true although we don't expect to use it:
+	assert.True(t, opts.Help.Val)
+}
+
 func TestParseArgsString(t *testing.T) {
 	args := []string{"--source", "foo"}
 	opts := initOpts()
-	ParseArgs(CMD, SUMMARY, args, &opts)
+	err := ParseArgs(CMD, SUMMARY, args, &opts)
+	assert.NoError(t, err)
 	assert.Equal(t, opts.Source.Val, "foo")
 }
 
@@ -73,7 +82,8 @@ func TestParseArgs(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			ParseArgs(CMD, SUMMARY, tc.args, &opts)
+			err := ParseArgs(CMD, SUMMARY, tc.args, &opts)
+			assert.NoError(t, err)
 			if tc.boolVal != nil {
 				assert.True(t, *tc.boolVal)
 			}

flags/help.go

@@ -1,9 +1,8 @@
 package flags
 
 import (
+	"bytes"
 	"fmt"
-	"log"
-	"os"
 	"reflect"
 	"strings"
 	"text/tabwriter"
@@ -11,30 +10,43 @@ import (
 	"github.com/brtholomy/um/go/cmd"
 )
 
-func HelpRequired(sub cmd.Subcommand, long string) {
+type HelpError struct {
-	log.Printf("um %s: %s is required", sub, long)
+	message string
 }
 
-func HelpInvalidArg(sub cmd.Subcommand, arg string) {
+func (h HelpError) Error() string {
-	log.Printf("um %s: invalid argument: %s", sub, arg)
+	return h.message
 }
 
-func HelpMissingAssignment(sub cmd.Subcommand, arg string) {
+func HelpRequired(sub cmd.Subcommand, long string) error {
-	log.Printf("um %s: %s needs a value assignment\n", sub, arg)
+	return HelpError{
-	log.Fatal("try: um [cmd] --help")
+		fmt.Sprintf("um %s: %s is required", sub, long),
+	}
+}
+
+func HelpInvalidArg(sub cmd.Subcommand, arg string) error {
+	return HelpError{
+		fmt.Sprintf("um %s: invalid argument: %s", sub, arg),
+	}
+}
+
+func HelpMissingAssignment(sub cmd.Subcommand, arg string) error {
+	return HelpError{
+		fmt.Sprintf("um %s: %s needs a value assignment", sub, arg),
+	}
 }
 
 // print out help string by reflecting over fields of provided opts struct
 // and os.Exit(0)
-func Help(sub cmd.Subcommand, summary string, opts any) {
+func Help(sub cmd.Subcommand, summary string, opts any) error {
 	v := reflect.ValueOf(opts)
 	if v.Kind() == reflect.Ptr {
 		v = v.Elem()
 	}
 	t := v.Type()
 
-	// everything should go to either stderr or stdout:
+	buf := &bytes.Buffer{}
-	w := tabwriter.NewWriter(os.Stderr, 0, 0, 3, ' ', 0)
+	w := tabwriter.NewWriter(buf, 0, 0, 3, ' ', 0)
 	positional := ""
 
 	for i := 0; i < v.NumField(); i++ {
@@ -56,8 +68,9 @@ func Help(sub cmd.Subcommand, summary string, opts any) {
 			)
 		}
 	}
-	log.Printf("um %s%s\n\n", sub, positional)
+	if _, err := buf.WriteString(fmt.Sprintf("um %s%s\n\n%s\n\n", sub, positional, summary)); err != nil {
-	log.Printf("%s\n\n", summary)
+		return err
-	w.Flush()
+	}
-	os.Exit(0)
+	w.Flush()
+	return HelpError{buf.String()}
 }

last/last.go

@@ -46,7 +46,14 @@ func GlobLast(glob string) (string, error) {
 
 func Last(args []string) {
 	opts := initOpts()
-	flags.ParseArgs(CMD, SUMMARY, args, &opts)
+	if err := flags.ParseArgs(CMD, SUMMARY, args, &opts); err != nil {
+		var herr flags.HelpError
+		if errors.As(err, &herr) {
+			fmt.Println(herr)
+			return
+		}
+		log.Fatalf("um %s: %s", CMD, err)
+	}
 
 	s, err := GlobLast(GLOB)
 	if err != nil {

mv/mv.go

@@ -1,6 +1,7 @@
 package mv
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"os"
@@ -89,7 +90,14 @@ func mv(opts options) error {
 
 func Mv(args []string) {
 	opts := initOpts()
-	flags.ParseArgs(CMD, SUMMARY, args, &opts)
+	if err := flags.ParseArgs(CMD, SUMMARY, args, &opts); err != nil {
+		var herr flags.HelpError
+		if errors.As(err, &herr) {
+			fmt.Println(herr)
+			return
+		}
+		log.Fatalf("um %s: %s", CMD, err)
+	}
 	err := mv(opts)
 	if err != nil {
 		log.Fatalf("um %s: %v", CMD, err)

next/next.go

@@ -1,6 +1,7 @@
 package next
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"os/exec"
@@ -99,8 +100,14 @@ func emacsNext(f string, tags string) error {
 
 func Next(args []string) {
 	opts := initOpts()
-	flags.ParseArgs(CMD, SUMMARY, args, &opts)
+	if err := flags.ParseArgs(CMD, SUMMARY, args, &opts); err != nil {
-
+		var herr flags.HelpError
+		if errors.As(err, &herr) {
+			fmt.Println(herr)
+			return
+		}
+		log.Fatalf("um %s: %s", CMD, err)
+	}
 	l, err := last.GlobLast(last.GLOB)
 	if err != nil {
 		log.Fatalf("um %s: %v", CMD, err)

sort/sort.go

@@ -1,6 +1,7 @@
 package sort
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"os"
@@ -69,11 +70,18 @@ func write(file string, content string) {
 
 func Sort(args []string) {
 	opts := initOpts()
-	flags.ParseArgs(CMD, SUMMARY, args, &opts)
+	if err := flags.ParseArgs(CMD, SUMMARY, args, &opts); err != nil {
+		var herr flags.HelpError
+		if errors.As(err, &herr) {
+			fmt.Println(herr)
+			return
+		}
+		log.Fatalf("um %s: %s", CMD, err)
+	}
 	// BORK: by hand for now:
 	if !opts.Key.IsSet() {
-		flags.HelpRequired(CMD, opts.Key.Long)
+		fmt.Println(flags.HelpRequired(CMD, opts.Key.Long))
-		flags.Help(CMD, SUMMARY, opts)
+		return
 	}
 
 	sslice, err := pipe.FileListSplitMaybeStdin(opts.Source.Val)

tag/tag.go

@@ -1,6 +1,9 @@
 package tag
 
 import (
+	"errors"
+	"fmt"
+	"log"
 	"os"
 
 	"github.com/brtholomy/um/go/cmd"
@@ -33,7 +36,14 @@ func initOpts() options {
 
 func Tag(args []string) {
 	opts := initOpts()
-	flags.ParseArgs(CMD, SUMMARY, args, &opts)
+	if err := flags.ParseArgs(CMD, SUMMARY, args, &opts); err != nil {
+		var herr flags.HelpError
+		if errors.As(err, &herr) {
+			fmt.Println(herr)
+			return
+		}
+		log.Fatalf("um %s: %s", CMD, err)
+	}
 
 	queries := parseQuery(opts.Query.Val)
 	entries := entriesGlobOrStdin(last.GLOB)