refactor: Refactor options.go

main.go

@@ -47,13 +47,19 @@ Examples:
 	$ smug print > ~/.config/smug/blog.yml
 `, version, FileUsage, WindowsUsage, AttachUsage, InsideCurrentSessionUsage, DebugUsage, DetachUsage)
 
-func main() {
-	options, err := ParseOptions(os.Args[1:], func() {
-		fmt.Fprintf(os.Stdout, usage)
-		os.Exit(0)
-	})
+func newLogger(path string) *log.Logger {
+	logFile, err := os.Create(filepath.Join(path, "smug.log"))
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err.Error())
+		os.Exit(1)
+	}
+	return log.New(logFile, "", 0)
+}
 
+func main() {
+	options, err := ParseOptions(os.Args[1:])
 	if err == ErrHelp {
+		fmt.Fprintf(os.Stdout, usage)
 		os.Exit(0)
 	}
 
@@ -70,12 +76,7 @@ func main() {
 
 	var logger *log.Logger
 	if options.Debug {
-		logFile, err := os.Create(filepath.Join(userConfigDir, "smug.log"))
-		if err != nil {
-			fmt.Fprintln(os.Stderr, err.Error())
-			os.Exit(1)
-		}
-		logger = log.New(logFile, "", 0)
+		logger = newLogger(userConfigDir)
 	}
 
 	commander := DefaultCommander{logger}

options.go

@@ -94,30 +94,30 @@ const (
 	InsideCurrentSessionUsage = "Create all windows inside current session"
 )
 
-// Creates a new FlagSet.
-// Moved it to a variable to be able to override it in the tests.
-var NewFlagSet = func(cmd string) *pflag.FlagSet {
-	f := pflag.NewFlagSet(cmd, pflag.ContinueOnError)
-	return f
-}
-
-func ParseOptions(argv []string, helpRequested func()) (Options, error) {
-	if len(argv) == 0 {
-		helpRequested()
-		return Options{}, ErrHelp
+func parseUserSettings(args []string) map[string]string {
+	settings := make(map[string]string)
+	for _, kv := range args {
+		s := strings.Split(kv, "=")
+		if len(s) < 2 {
+			continue
+		}
+		settings[s[0]] = s[1]
 	}
 
-	if argv[0] == "--help" || argv[0] == "-h" {
-		helpRequested()
-		return Options{}, ErrHelp
+	return settings
+}
+
+func ParseOptions(argv []string) (*Options, error) {
+	if len(argv) == 0 || argv[0] == "--help" || argv[0] == "-h" {
+		return nil, ErrHelp
 	}
 
 	cmd, cmdErr := Commands.Resolve(argv[0])
 	if errors.Is(cmdErr, ErrCommandNotFound) {
 		cmd = Commands.FindByName(CommandStart)
 	}
 
-	flags := NewFlagSet(cmd.Name)
+	flags := pflag.NewFlagSet(cmd.Name, pflag.ContinueOnError)
 
 	config := flags.StringP("file", "f", "", FileUsage)
 	windows := flags.StringArrayP("windows", "w", []string{}, WindowsUsage)
@@ -128,11 +128,11 @@ func ParseOptions(argv []string, helpRequested func()) (Options, error) {
 
 	err := flags.Parse(argv)
 	if err == pflag.ErrHelp {
-		return Options{}, ErrHelp
+		return nil, ErrHelp
 	}
 
 	if err != nil {
-		return Options{}, err
+		return nil, err
 	}
 
 	var project string
@@ -151,19 +151,9 @@ func ParseOptions(argv []string, helpRequested func()) (Options, error) {
 		windows = &wl
 	}
 
-	settings := make(map[string]string)
-	userSettings := flags.Args()[1:]
-	if len(userSettings) > 0 {
-		for _, kv := range userSettings {
-			s := strings.Split(kv, "=")
-			if len(s) < 2 {
-				continue
-			}
-			settings[s[0]] = s[1]
-		}
-	}
+	settings := parseUserSettings(flags.Args()[1:])
 
-	return Options{
+	return &Options{
 		Project:              project,
 		Config:               *config,
 		Command:              cmd.Name,

options_test.go

@@ -4,15 +4,12 @@ import (
 	"errors"
 	"reflect"
 	"testing"
-
-	"github.com/spf13/pflag"
 )
 
 var usageTestTable = []struct {
-	argv      []string
-	opts      Options
-	err       error
-	helpCalls int
+	argv []string
+	opts Options
+	err  error
 }{
 	{
 		[]string{"start", "smug"},
@@ -27,7 +24,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "smug", "-w", "foo"},
@@ -42,7 +38,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "smug:foo,bar"},
@@ -57,7 +52,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "smug", "--attach", "--debug", "--detach"},
@@ -72,7 +66,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "smug", "-ad"},
@@ -87,7 +80,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "-f", "test.yml"},
@@ -102,7 +94,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "-f", "test.yml", "-w", "win1", "-w", "win2"},
@@ -117,7 +108,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "project", "a=b", "x=y"},
@@ -135,7 +125,6 @@ var usageTestTable = []struct {
 			},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "-f", "test.yml", "a=b", "x=y"},
@@ -153,7 +142,6 @@ var usageTestTable = []struct {
 			},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "-f", "test.yml", "-w", "win1", "-w", "win2", "a=b", "x=y"},
@@ -171,13 +159,11 @@ var usageTestTable = []struct {
 			},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"start", "--help"},
 		Options{},
 		ErrHelp,
-		1,
 	},
 	{
 		[]string{"test"},
@@ -188,7 +174,6 @@ var usageTestTable = []struct {
 			Settings: map[string]string{},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{"test", "-w", "win1", "-w", "win2", "a=b", "x=y"},
@@ -199,55 +184,33 @@ var usageTestTable = []struct {
 			Settings: map[string]string{"a": "b", "x": "y"},
 		},
 		nil,
-		0,
 	},
 	{
 		[]string{},
 		Options{},
 		ErrHelp,
-		1,
 	},
 	{
 		[]string{"--help"},
 		Options{},
 		ErrHelp,
-		1,
 	},
 	{
 		[]string{"start", "--test"},
 		Options{},
 		errors.New("unknown flag: --test"),
-		0,
 	},
 }
 
 func TestParseOptions(t *testing.T) {
-	helpCalls := 0
-	helpRequested := func() {
-		helpCalls++
-	}
-
-	NewFlagSet = func(cmd string) *pflag.FlagSet {
-		flagSet := pflag.NewFlagSet(cmd, pflag.ContinueOnError)
-		flagSet.Usage = helpRequested
-		return flagSet
-	}
-
 	for _, v := range usageTestTable {
-		opts, err := ParseOptions(v.argv, helpRequested)
-
-		if !reflect.DeepEqual(v.opts, opts) {
-			t.Errorf("expected struct %v, got %v", v.opts, opts)
-		}
-
-		if helpCalls != v.helpCalls {
-			t.Errorf("expected to get %d help calls, got %d", v.helpCalls, helpCalls)
+		opts, err := ParseOptions(v.argv)
+		if v.err != nil && err != nil && err.Error() != v.err.Error() {
+			t.Errorf("expected error %v, got %v", v.err, err)
 		}
 
-		if v.err != nil && err.Error() != v.err.Error() {
-			t.Errorf("expected to get error %v, got %v", v.err, err)
+		if opts != nil && !reflect.DeepEqual(v.opts, *opts) {
+			t.Errorf("expected struct %v, got %v", v.opts, opts)
 		}
-
-		helpCalls = 0
 	}
 }

smug.go

@@ -77,7 +77,7 @@ func (smug Smug) switchOrAttach(target string, attach bool, insideTmuxSession bo
 	return nil
 }
 
-func (smug Smug) Stop(config Config, options Options, context Context) error {
+func (smug Smug) Stop(config Config, options *Options, context Context) error {
 	windows := options.Windows
 	if len(windows) == 0 {
 		sessionRoot := ExpandPath(config.Root)
@@ -100,7 +100,7 @@ func (smug Smug) Stop(config Config, options Options, context Context) error {
 	return nil
 }
 
-func (smug Smug) Start(config Config, options Options, context Context) error {
+func (smug Smug) Start(config Config, options *Options, context Context) error {
 	var sessionName string
 	var err error
 
@@ -221,7 +221,7 @@ func (smug Smug) Start(config Config, options Options, context Context) error {
 	return nil
 }
 
-func (smug Smug) GetConfigFromSession(options Options, context Context) (Config, error) {
+func (smug Smug) GetConfigFromSession(options *Options, context Context) (Config, error) {
 	config := Config{}
 
 	tmuxSession, err := smug.tmux.SessionName()