From f6060cc6b46939487955690d2c2eb304a85651df Mon Sep 17 00:00:00 2001 From: Ivan Klymenchenko Date: Fri, 10 Jun 2022 13:37:06 +0700 Subject: [PATCH] refactor: Refactor options.go --- main.go | 23 +++++++++++---------- options.go | 48 ++++++++++++++++++-------------------------- options_test.go | 53 ++++++++----------------------------------------- smug.go | 6 +++--- smug_test.go | 22 ++++++++++---------- 5 files changed, 53 insertions(+), 99 deletions(-) diff --git a/main.go b/main.go index a4d8441..02a6d2f 100644 --- a/main.go +++ b/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} diff --git a/options.go b/options.go index ec7f812..252d4c4 100644 --- a/options.go +++ b/options.go @@ -94,22 +94,22 @@ 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]) @@ -117,7 +117,7 @@ func ParseOptions(argv []string, helpRequested func()) (Options, error) { 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, diff --git a/options_test.go b/options_test.go index 545b3eb..054c70c 100644 --- a/options_test.go +++ b/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 } } diff --git a/smug.go b/smug.go index 3e6e6fc..0f87743 100644 --- a/smug.go +++ b/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() diff --git a/smug_test.go b/smug_test.go index f67f1c5..4eab1cf 100644 --- a/smug_test.go +++ b/smug_test.go @@ -10,7 +10,7 @@ import ( var testTable = map[string]struct { config Config - options Options + options *Options context Context startCommands []string stopCommands []string @@ -28,7 +28,7 @@ var testTable = map[string]struct { }, }, }, - Options{}, + &Options{}, Context{}, []string{ "tmux has-session -t ses:", @@ -58,7 +58,7 @@ var testTable = map[string]struct { }, }, }, - Options{ + &Options{ Detach: true, }, Context{}, @@ -104,7 +104,7 @@ var testTable = map[string]struct { "stop2 -d --foo=bar", }, }, - Options{}, + &Options{}, Context{}, []string{ "tmux has-session -t ses:", @@ -139,7 +139,7 @@ var testTable = map[string]struct { }, }, }, - Options{ + &Options{ Windows: []string{"win2"}, }, Context{}, @@ -164,7 +164,7 @@ var testTable = map[string]struct { {Name: "win1"}, }, }, - Options{}, + &Options{}, Context{}, []string{ "tmux has-session -t ses:", @@ -180,7 +180,7 @@ var testTable = map[string]struct { Session: "ses", Root: "root", }, - Options{Attach: false}, + &Options{Attach: false}, Context{InsideTmuxSession: true}, []string{ "tmux has-session -t ses:", @@ -201,7 +201,7 @@ var testTable = map[string]struct { {Name: "win1"}, }, }, - Options{Attach: true}, + &Options{Attach: true}, Context{InsideTmuxSession: true}, []string{ "tmux has-session -t ses:", @@ -220,7 +220,7 @@ var testTable = map[string]struct { {Name: "win1"}, }, }, - Options{ + &Options{ InsideCurrentSession: true, }, Context{InsideTmuxSession: true}, @@ -243,7 +243,7 @@ var testTable = map[string]struct { {Name: "win1"}, }, }, - Options{ + &Options{ InsideCurrentSession: true, }, Context{InsideTmuxSession: true}, @@ -348,7 +348,7 @@ func TestPrintCurrentSession(t *testing.T) { smug := Smug{tmux, commander} - actualConfig, err := smug.GetConfigFromSession(Options{Project: "test"}, Context{}) + actualConfig, err := smug.GetConfigFromSession(&Options{Project: "test"}, Context{}) if err != nil { t.Fatalf("error %v", err) }