Skip to content

Commit

Permalink
refactor: Make output for cmd help, adhoc data more consistent
Browse files Browse the repository at this point in the history
Usage funcs in pkg cmd would write some stuff to stdout, some to stderr.
Make this consistent. Going with stdout for usage funcs to make it
easier to capture help in a file.

Specifying outputs in the library code helps make the Info subcommand
cleaner. If you've selected the json format output, you're piping to jq
and there's an error, then invalid json is written to stdout. So, write
that adhoc stuff to stderr.
  • Loading branch information
rafaelespinoza committed Jan 17, 2022
1 parent 8d6ada0 commit 28476fb
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 30 deletions.
8 changes: 4 additions & 4 deletions godfish.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func runMigration(driver Driver, pathToFile string, mig internal.Migration) (err
if mig.Indirection().Value == internal.DirReverse {
gerund = "rolling back"
}
fmt.Printf("%s version %q ... ", gerund, mig.Version().String())
fmt.Fprintf(os.Stderr, "%s version %q ... ", gerund, mig.Version().String())

if err = driver.Execute(string(data)); err != nil {
err = &runMigrationError{
Expand All @@ -204,7 +204,7 @@ func runMigration(driver Driver, pathToFile string, mig internal.Migration) (err
mig.Version().String(),
)
if err == nil {
fmt.Println("ok")
fmt.Fprintln(os.Stderr, "ok")
}
return
}
Expand Down Expand Up @@ -252,7 +252,7 @@ func choosePrinter(format string, w io.Writer) (out internal.InfoPrinter) {
func Init(pathToFile string) (err error) {
_, err = os.Stat(pathToFile)
if err == nil {
fmt.Printf("config file %q already present\n", pathToFile)
fmt.Fprintf(os.Stderr, "config file %q already present\n", pathToFile)
return nil
}
if !os.IsNotExist(err) {
Expand Down Expand Up @@ -363,7 +363,7 @@ func (m *migrationFinder) available() (out []internal.Migration, err error) {
for _, fn := range filenames {
mig, ierr := internal.ParseMigration(internal.Filename(fn))
if errors.Is(ierr, internal.ErrDataInvalid) {
fmt.Println(ierr)
fmt.Fprintln(os.Stderr, ierr)
continue
} else if ierr != nil {
err = ierr
Expand Down
10 changes: 8 additions & 2 deletions internal/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func New(driver godfish.Driver) Root {
},
}

rootFlags := flag.NewFlagSet("godfish", flag.ExitOnError)
rootFlags := newFlagSet("godfish")
rootFlags.Usage = func() {
fmt.Fprintf(rootFlags.Output(), `Usage:
Expand Down Expand Up @@ -124,9 +124,15 @@ Examples:
}
}

func newFlagSet(name string) (out *flag.FlagSet) {
out = flag.NewFlagSet(name, flag.ExitOnError)
out.SetOutput(os.Stdout)
return
}

// printFlagDefaults calls PrintDefaults on f. It helps make help message
// formatting more consistent.
func printFlagDefaults(f *flag.FlagSet) {
fmt.Printf("\n%s flags:\n\n", f.Name())
fmt.Fprintf(f.Output(), "\n%s flags:\n\n", f.Name())
f.PrintDefaults()
}
4 changes: 2 additions & 2 deletions internal/cmd/create-migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func makeCreateMigration(subcmdName string) alf.Directive {

// Other subcommands scope the flagset within the Setup func. However, this
// one is scoped up here to check if some flags were specified at runtime.
flags := flag.NewFlagSet(subcmdName, flag.ExitOnError)
flags := newFlagSet(subcmdName)

return &alf.Command{
Description: "generate migration files",
Expand Down Expand Up @@ -48,7 +48,7 @@ func makeCreateMigration(subcmdName string) alf.Directive {
"customize the directional part of the filename for reverse migration",
)
flags.Usage = func() {
fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags]
fmt.Fprintf(flags.Output(), `Usage: %s [godfish-flags] %s [%s-flags]
Generate migration files: one meant for the "forward" direction,
another meant for "reverse". Optionally create a migration in the forward
Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func makeInfo(name string) alf.Directive {
return &alf.Command{
Description: "output applied migrations, migrations to apply",
Setup: func(p flag.FlagSet) *flag.FlagSet {
flags := flag.NewFlagSet(name, flag.ExitOnError)
flags := newFlagSet(name)
flags.StringVar(
&direction,
"direction",
Expand All @@ -38,7 +38,7 @@ func makeInfo(name string) alf.Directive {
fmt.Sprintf("timestamp of migration, format: %s", internal.TimeFormat),
)
flags.Usage = func() {
fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags]
fmt.Fprintf(flags.Output(), `Usage: %s [godfish-flags] %s [%s-flags]
List applied migrations, preview migrations to apply.
Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ func makeInit(name string) alf.Directive {
return &alf.Command{
Description: "create godfish configuration file",
Setup: func(p flag.FlagSet) *flag.FlagSet {
flags := flag.NewFlagSet(name, flag.ExitOnError)
flags := newFlagSet(name)
flags.StringVar(
&conf,
"conf",
".godfish.json",
"path to godfish config file",
)
flags.Usage = func() {
fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags]
fmt.Fprintf(flags.Output(), `Usage: %s [godfish-flags] %s [%s-flags]
Creates a configuration file, unless it already exists.
`,
Expand Down
12 changes: 6 additions & 6 deletions internal/cmd/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ func makeMigrate(name string) alf.Directive {
return &alf.Command{
Description: "execute migration(s) in the forward direction",
Setup: func(p flag.FlagSet) *flag.FlagSet {
flags := flag.NewFlagSet(name, flag.ExitOnError)
flags := newFlagSet(name)
flags.StringVar(
&version,
"version",
"",
fmt.Sprintf("timestamp of migration, format: %s", internal.TimeFormat),
)
flags.Usage = func() {
fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags]
fmt.Fprintf(flags.Output(), `Usage: %s [godfish-flags] %s [%s-flags]
Execute migration(s) in the forward direction. If the "version" is left
unspecified, then all available migrations are executed. Otherwise,
Expand Down Expand Up @@ -57,9 +57,9 @@ func makeRemigrate(name string) alf.Directive {
return &alf.Command{
Description: "rollback and then re-apply the last migration",
Setup: func(p flag.FlagSet) *flag.FlagSet {
flags := flag.NewFlagSet(name, flag.ExitOnError)
flags := newFlagSet(name)
flags.Usage = func() {
fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags]
fmt.Fprintf(flags.Output(), `Usage: %s [godfish-flags] %s [%s-flags]
Execute the last migration in reverse (rollback) and then execute the same
one forward. This could be useful for development.
Expand Down Expand Up @@ -89,15 +89,15 @@ func makeRollback(name string) alf.Directive {
return &alf.Command{
Description: "execute migration(s) in the reverse direction",
Setup: func(p flag.FlagSet) *flag.FlagSet {
flags := flag.NewFlagSet(name, flag.ExitOnError)
flags := newFlagSet(name)
flags.StringVar(
&version,
"version",
"",
fmt.Sprintf("timestamp of migration, format: %s", internal.TimeFormat),
)
flags.Usage = func() {
fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags]
fmt.Fprintf(flags.Output(), `Usage: %s [godfish-flags] %s [%s-flags]
Execute migration(s) in the reverse direction. If the "version" is left
unspecified, then only the first available migration is executed. Otherwise,
Expand Down
22 changes: 13 additions & 9 deletions internal/cmd/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"flag"
"fmt"
"os"

"github.com/rafaelespinoza/alf"
)
Expand All @@ -26,10 +27,10 @@ func makeVersion(name string) alf.Directive {
return &alf.Command{
Description: "show metadata about the build",
Setup: func(p flag.FlagSet) *flag.FlagSet {
flags := flag.NewFlagSet(name, flag.ExitOnError)
flags := newFlagSet(name)
flags.BoolVar(&formatJSON, "json", false, "format output as JSON")
flags.Usage = func() {
fmt.Printf(`Usage: %s [flags]
fmt.Fprintf(flags.Output(), `Usage: %s [flags]
Prints some versioning info to stdout. Pass the -json flag to get JSON.
`,
Expand All @@ -40,13 +41,16 @@ func makeVersion(name string) alf.Directive {
return flags
},
Run: func(_ context.Context) error {
// Calling fmt.Print* also writes to stdout, but want to be explicit
// about where this subcommand output goes.
w := os.Stdout
if !formatJSON {
fmt.Printf("BranchName: %s\n", versionBranchName)
fmt.Printf("BuildTime: %s\n", versionBuildTime)
fmt.Printf("Driver: %s\n", versionDriver)
fmt.Printf("CommitHash: %s\n", versionCommitHash)
fmt.Printf("GoVersion: %s\n", versionGoVersion)
fmt.Printf("Tag: %s\n", versionTag)
fmt.Fprintf(w, "BranchName: %s\n", versionBranchName)
fmt.Fprintf(w, "BuildTime: %s\n", versionBuildTime)
fmt.Fprintf(w, "Driver: %s\n", versionDriver)
fmt.Fprintf(w, "CommitHash: %s\n", versionCommitHash)
fmt.Fprintf(w, "GoVersion: %s\n", versionGoVersion)
fmt.Fprintf(w, "Tag: %s\n", versionTag)
return nil
}
out, err := json.Marshal(
Expand All @@ -62,7 +66,7 @@ func makeVersion(name string) alf.Directive {
if err != nil {
return err
}
fmt.Printf("%s\n", out)
fmt.Fprintf(w, "%s\n", out)
return nil
},
}
Expand Down
6 changes: 3 additions & 3 deletions internal/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,18 @@ func (m *MigrationParams) GenerateFiles() (err error) {
if forwardFile, err = newMigrationFile(m.Forward, m.Dirpath); err != nil {
return
}
fmt.Println("created forward file:", forwardFile.Name())
fmt.Fprintln(os.Stderr, "created forward file:", forwardFile.Name())
defer forwardFile.Close()

if !m.Reversible {
fmt.Println("migration marked irreversible, did not create reverse file")
fmt.Fprintln(os.Stderr, "migration marked irreversible, did not create reverse file")
return
}

if reverseFile, err = newMigrationFile(m.Reverse, m.Dirpath); err != nil {
return
}
fmt.Println("created reverse file:", reverseFile.Name())
fmt.Fprintln(os.Stderr, "created reverse file:", reverseFile.Name())
defer reverseFile.Close()
return
}
Expand Down

0 comments on commit 28476fb

Please sign in to comment.