From fd6609f8c47c6acb8737036b8f2911a034a4eb16 Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Fri, 9 Nov 2018 10:40:14 -0500 Subject: [PATCH 1/8] Remove use of custom errorf from CmdUpdate. --- claat/cmd/update.go | 9 +++++++-- claat/main.go | 6 ++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/claat/cmd/update.go b/claat/cmd/update.go index 8569cfe12..b03d9c7fc 100644 --- a/claat/cmd/update.go +++ b/claat/cmd/update.go @@ -41,7 +41,8 @@ type CmdUpdateOptions struct { } // CmdUpdate is the "claat update ..." subcommand. -func CmdUpdate(opts CmdUpdateOptions) { +// It returns true if the command succeeded, false otherwise. +func CmdUpdate(opts CmdUpdateOptions) bool { roots := flag.Args() if len(roots) == 0 { roots = []string{"."} @@ -69,14 +70,18 @@ func CmdUpdate(opts CmdUpdateOptions) { ch <- &result{d, meta, err} }(d) } + + errs := []error{} for range dirs { res := <-ch if res.err != nil { - errorf(reportErr, res.dir, res.err) + errs = append(errs, res.err) + log.Printf(reportErr, res.dir, res.err) } else { log.Printf(reportOk, res.meta.ID) } } + return len(errs) == 0 } // updateCodelab reads metadata from a dir/codelab.json file, diff --git a/claat/main.go b/claat/main.go index 5ecf98534..ad4032c67 100644 --- a/claat/main.go +++ b/claat/main.go @@ -78,11 +78,13 @@ func main() { case "build": cmd.CmdBuild() case "update": - cmd.CmdUpdate(cmd.CmdUpdateOptions{ + if ok := cmd.CmdUpdate(cmd.CmdUpdateOptions{ AuthToken: *authToken, GlobalGA: *globalGA, Prefix: *prefix, - }) + }); !ok { + os.Exit(1) + } case "help": usage() case "version": From 4e4e82cbfbf92751b190090ed9681d40fb4e81c4 Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Fri, 9 Nov 2018 10:45:53 -0500 Subject: [PATCH 2/8] Remove use of custom error from CmdExport. --- claat/cmd/export.go | 8 ++++++-- claat/main.go | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/claat/cmd/export.go b/claat/cmd/export.go index 942b4ebb3..3a640de63 100644 --- a/claat/cmd/export.go +++ b/claat/cmd/export.go @@ -45,7 +45,8 @@ type CmdExportOptions struct { } // CmdExport is the "claat export ..." subcommand. -func CmdExport(opts CmdExportOptions) { +// It returns true if the command succeeded, false otherwise. +func CmdExport(opts CmdExportOptions) bool { if flag.NArg() == 0 { log.Fatalf("Need at least one source. Try '-h' for options.") } @@ -62,14 +63,17 @@ func CmdExport(opts CmdExportOptions) { ch <- &result{src, meta, err} }(src) } + errs := []error{} for range args { res := <-ch if res.err != nil { - errorf(reportErr, res.src, res.err) + errs = append(errs, res.err) + log.Printf(reportErr, res.src, res.err) } else if !isStdout(opts.Output) { log.Printf(reportOk, res.meta.ID) } } + return len(errs) == 0 } // exportCodelab fetches codelab src from either local disk or remote, diff --git a/claat/main.go b/claat/main.go index ad4032c67..1f720475c 100644 --- a/claat/main.go +++ b/claat/main.go @@ -65,14 +65,16 @@ func main() { switch os.Args[1] { case "export": - cmd.CmdExport(cmd.CmdExportOptions{ + if ok := cmd.CmdExport(cmd.CmdExportOptions{ AuthToken: *authToken, Expenv: *expenv, GlobalGA: *globalGA, Output: *output, Prefix: *prefix, Tmplout: *tmplout, - }) + }); !ok { + os.Exit(1) + } case "serve": cmd.CmdServe(*addr) case "build": From b67d222c3f10d5cdfc99c3165ec6e966c039ed22 Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Fri, 9 Nov 2018 10:47:06 -0500 Subject: [PATCH 3/8] Remove custom errorf from util.go. --- claat/cmd/util.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/claat/cmd/util.go b/claat/cmd/util.go index 2afa09f25..69af9b851 100644 --- a/claat/cmd/util.go +++ b/claat/cmd/util.go @@ -21,7 +21,6 @@ package cmd import ( "encoding/json" - "log" "sync" // allow parsers to register themselves @@ -54,14 +53,6 @@ func isStdout(filename string) bool { return filename == stdout } -// errorf calls log.Printf with fmt and args, and sets non-zero exit code. -func errorf(format string, args ...interface{}) { - log.Printf(format, args...) - exitMu.Lock() - Exit = 1 - exitMu.Unlock() -} - // ParseExtraVars parses extra template variables from command line. // extra is any additional arguments to pass to format templates. Should be formatted as JSON objects of string:string KV pairs. func ParseExtraVars(extra string) map[string]string { From 6d6179f852e3b8339b7524969ff8d48b19892a76 Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Fri, 9 Nov 2018 11:02:28 -0500 Subject: [PATCH 4/8] Remove unnecessary constants from util.go. --- claat/cmd/util.go | 7 +------ claat/main.go | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/claat/cmd/util.go b/claat/cmd/util.go index 69af9b851..ab8bc6802 100644 --- a/claat/cmd/util.go +++ b/claat/cmd/util.go @@ -21,7 +21,6 @@ package cmd import ( "encoding/json" - "sync" // allow parsers to register themselves _ "github.com/googlecodelabs/tools/claat/parser/gdoc" @@ -42,11 +41,7 @@ const ( reportOk = "ok\t%s" ) -var ( - Exit int // program exit code - exitMu sync.Mutex // guards exit - ExtraVars map[string]string // Extra template variables passed on the command line. -) +var ExtraVars map[string]string // Extra template variables passed on the command line. // isStdout reports whether filename is stdout. func isStdout(filename string) bool { diff --git a/claat/main.go b/claat/main.go index 1f720475c..a57f98177 100644 --- a/claat/main.go +++ b/claat/main.go @@ -94,7 +94,6 @@ func main() { default: log.Fatalf("Unknown subcommand. Try '-h' for options.") } - os.Exit(cmd.Exit) } // usage prints usageText and program arguments to stderr. From 15994cdb3ef6adba9d49f584ea5a39875f1a9ea9 Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Fri, 9 Nov 2018 11:10:16 -0500 Subject: [PATCH 5/8] Remove custom errorf from ParseExtraVars. --- claat/cmd/util.go | 10 ++++++---- claat/main.go | 7 ++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/claat/cmd/util.go b/claat/cmd/util.go index ab8bc6802..60443c031 100644 --- a/claat/cmd/util.go +++ b/claat/cmd/util.go @@ -21,6 +21,7 @@ package cmd import ( "encoding/json" + "log" // allow parsers to register themselves _ "github.com/googlecodelabs/tools/claat/parser/gdoc" @@ -50,15 +51,16 @@ func isStdout(filename string) bool { // ParseExtraVars parses extra template variables from command line. // extra is any additional arguments to pass to format templates. Should be formatted as JSON objects of string:string KV pairs. -func ParseExtraVars(extra string) map[string]string { +func ParseExtraVars(extra string) (map[string]string, error) { vars := make(map[string]string) if extra == "" { - return vars + return vars, nil } b := []byte(extra) err := json.Unmarshal(b, &vars) if err != nil { - errorf("Error parsing additional template data: %v", err) + log.Printf("Error parsing additional template data: %v", err) + return nil, err } - return vars + return vars, nil } diff --git a/claat/main.go b/claat/main.go index a57f98177..3245d4ec7 100644 --- a/claat/main.go +++ b/claat/main.go @@ -61,7 +61,12 @@ func main() { flag.Usage = usage flag.CommandLine.Parse(os.Args[2:]) - cmd.ExtraVars = cmd.ParseExtraVars(*extra) + + var err error + cmd.ExtraVars, err = cmd.ParseExtraVars(*extra) + if err != nil { + os.Exit(1) + } switch os.Args[1] { case "export": From abc04689fd62699e71563248b80667356f9003d4 Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Thu, 15 Nov 2018 17:16:48 -0500 Subject: [PATCH 6/8] Change information flow model so command functions send error codes up. The caller (main) is responsible for invoking os.Exit. --- claat/cmd/build.go | 5 ++++- claat/cmd/export.go | 10 +++++----- claat/cmd/serve.go | 4 +++- claat/cmd/update.go | 10 +++++----- claat/main.go | 19 +++++++++---------- 5 files changed, 26 insertions(+), 22 deletions(-) diff --git a/claat/cmd/build.go b/claat/cmd/build.go index 01b53284f..c1a8344f8 100644 --- a/claat/cmd/build.go +++ b/claat/cmd/build.go @@ -29,7 +29,9 @@ import ( "sync" ) -func CmdBuild() { +// CmdBuild is the "claat build ..." subcommand. +// It returns a process exit code. +func CmdBuild() int { const depsDir = "bower_components" var codelabElem = []byte(` @@ -46,6 +48,7 @@ func CmdBuild() { if err := writeFile(filepath.Join("elements", "codelab.html"), codelabElem); err != nil { log.Fatalf(err.Error()) } + return 0 } func writeFile(name string, content []byte) error { diff --git a/claat/cmd/export.go b/claat/cmd/export.go index 3a640de63..90ade5bf1 100644 --- a/claat/cmd/export.go +++ b/claat/cmd/export.go @@ -45,8 +45,9 @@ type CmdExportOptions struct { } // CmdExport is the "claat export ..." subcommand. -// It returns true if the command succeeded, false otherwise. -func CmdExport(opts CmdExportOptions) bool { +// It returns a process exit code. +func CmdExport(opts CmdExportOptions) int { + exitCode := 0 if flag.NArg() == 0 { log.Fatalf("Need at least one source. Try '-h' for options.") } @@ -63,17 +64,16 @@ func CmdExport(opts CmdExportOptions) bool { ch <- &result{src, meta, err} }(src) } - errs := []error{} for range args { res := <-ch if res.err != nil { - errs = append(errs, res.err) + exitCode = 1 log.Printf(reportErr, res.src, res.err) } else if !isStdout(opts.Output) { log.Printf(reportOk, res.meta.ID) } } - return len(errs) == 0 + return exitCode } // exportCodelab fetches codelab src from either local disk or remote, diff --git a/claat/cmd/serve.go b/claat/cmd/serve.go index cba2cad7a..5b3ff60d4 100644 --- a/claat/cmd/serve.go +++ b/claat/cmd/serve.go @@ -23,7 +23,8 @@ import ( // CmdServe is the "claat serve ..." subcommand. // addr is the hostname and port to bind the web server to. -func CmdServe(addr string) { +// It returns a process exit code. +func CmdServe(addr string) int { CmdBuild() http.Handle("/", http.FileServer(http.Dir("."))) log.Printf("Serving codelabs on %s, opening browser tab now...", addr) @@ -33,6 +34,7 @@ func CmdServe(addr string) { }() openBrowser("http://" + addr) log.Fatalf("claat serve: %v", <-ch) + return 0 } // openBrowser tries to open the URL in a browser. diff --git a/claat/cmd/update.go b/claat/cmd/update.go index b03d9c7fc..1d699a5c6 100644 --- a/claat/cmd/update.go +++ b/claat/cmd/update.go @@ -41,8 +41,8 @@ type CmdUpdateOptions struct { } // CmdUpdate is the "claat update ..." subcommand. -// It returns true if the command succeeded, false otherwise. -func CmdUpdate(opts CmdUpdateOptions) bool { +// It returns a process exit code. +func CmdUpdate(opts CmdUpdateOptions) int { roots := flag.Args() if len(roots) == 0 { roots = []string{"."} @@ -71,17 +71,17 @@ func CmdUpdate(opts CmdUpdateOptions) bool { }(d) } - errs := []error{} + exitCode := 0 for range dirs { res := <-ch if res.err != nil { - errs = append(errs, res.err) + exitCode = 1 log.Printf(reportErr, res.dir, res.err) } else { log.Printf(reportOk, res.meta.ID) } } - return len(errs) == 0 + return exitCode } // updateCodelab reads metadata from a dir/codelab.json file, diff --git a/claat/main.go b/claat/main.go index 3245d4ec7..d83cd1fab 100644 --- a/claat/main.go +++ b/claat/main.go @@ -68,30 +68,27 @@ func main() { os.Exit(1) } + exitCode := 0 switch os.Args[1] { case "export": - if ok := cmd.CmdExport(cmd.CmdExportOptions{ + exitCode = cmd.CmdExport(cmd.CmdExportOptions{ AuthToken: *authToken, Expenv: *expenv, GlobalGA: *globalGA, Output: *output, Prefix: *prefix, Tmplout: *tmplout, - }); !ok { - os.Exit(1) - } + }) case "serve": - cmd.CmdServe(*addr) + exitCode = cmd.CmdServe(*addr) case "build": - cmd.CmdBuild() + exitCode = cmd.CmdBuild() case "update": - if ok := cmd.CmdUpdate(cmd.CmdUpdateOptions{ + exitCode = cmd.CmdUpdate(cmd.CmdUpdateOptions{ AuthToken: *authToken, GlobalGA: *globalGA, Prefix: *prefix, - }); !ok { - os.Exit(1) - } + }) case "help": usage() case "version": @@ -99,6 +96,8 @@ func main() { default: log.Fatalf("Unknown subcommand. Try '-h' for options.") } + + os.Exit(exitCode) } // usage prints usageText and program arguments to stderr. From 80ce6526477b0bbced673f26202760077a091546 Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Mon, 19 Nov 2018 23:29:21 +0000 Subject: [PATCH 7/8] Switch from zero-init to declaration for exit codes. No semantic impact; this is purely stylistic. --- claat/cmd/export.go | 2 +- claat/cmd/update.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/claat/cmd/export.go b/claat/cmd/export.go index 90ade5bf1..43e5f2123 100644 --- a/claat/cmd/export.go +++ b/claat/cmd/export.go @@ -47,7 +47,7 @@ type CmdExportOptions struct { // CmdExport is the "claat export ..." subcommand. // It returns a process exit code. func CmdExport(opts CmdExportOptions) int { - exitCode := 0 + var exitCode int if flag.NArg() == 0 { log.Fatalf("Need at least one source. Try '-h' for options.") } diff --git a/claat/cmd/update.go b/claat/cmd/update.go index 1d699a5c6..d07349148 100644 --- a/claat/cmd/update.go +++ b/claat/cmd/update.go @@ -71,7 +71,7 @@ func CmdUpdate(opts CmdUpdateOptions) int { }(d) } - exitCode := 0 + var exitCode int for range dirs { res := <-ch if res.err != nil { From 0c4910cd8c9e0cfc2576506beb0c93a99fad3f0e Mon Sep 17 00:00:00 2001 From: Cassie Recher Date: Tue, 20 Nov 2018 15:27:19 +0000 Subject: [PATCH 8/8] Move ParseExtrVars to main.go. It was only being used in main, so it didn't make much sense to keep in utils. --- claat/cmd/export.go | 9 ++++++--- claat/cmd/update.go | 4 +++- claat/cmd/util.go | 20 -------------------- claat/main.go | 22 ++++++++++++++++++++-- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/claat/cmd/export.go b/claat/cmd/export.go index 43e5f2123..7afa484f8 100644 --- a/claat/cmd/export.go +++ b/claat/cmd/export.go @@ -34,6 +34,8 @@ type CmdExportOptions struct { AuthToken string // Expenv is the codelab environment to export to. Expenv string + // ExtraVars is extra template variables. + ExtraVars map[string]string // GlobalGA is the global Google Analytics account to use. GlobalGA string // Output is the output directory, or "-" for stdout. @@ -120,12 +122,13 @@ func exportCodelab(src string, opts CmdExportOptions) (*types.Meta, error) { } } // write codelab and its metadata to disk - return meta, writeCodelab(dir, clab.Codelab, ctx) + return meta, writeCodelab(dir, clab.Codelab, opts.ExtraVars, ctx) } // writeCodelab stores codelab main content in ctx.Format and its metadata // in JSON format on disk. -func writeCodelab(dir string, clab *types.Codelab, ctx *types.Context) error { +// extraVars is extra variables to pass into the template context. +func writeCodelab(dir string, clab *types.Codelab, extraVars map[string]string, ctx *types.Context) error { // output to stdout does not include metadata if !isStdout(dir) { // make sure codelab dir exists @@ -153,7 +156,7 @@ func writeCodelab(dir string, clab *types.Codelab, ctx *types.Context) error { GlobalGA: ctx.MainGA, Meta: &clab.Meta, Steps: clab.Steps, - Extra: ExtraVars, + Extra: extraVars, }} if ctx.Format != "offline" { w := os.Stdout diff --git a/claat/cmd/update.go b/claat/cmd/update.go index d07349148..de061c738 100644 --- a/claat/cmd/update.go +++ b/claat/cmd/update.go @@ -34,6 +34,8 @@ import ( type CmdUpdateOptions struct { // AuthToken is the token to use for the Drive API. AuthToken string + // ExtraVars is extra template variables. + ExtraVars map[string]string // GlobalGA is the global Google Analytics account to use. GlobalGA string // Prefix is a URL prefix to prepend when using HTML format. @@ -127,7 +129,7 @@ func updateCodelab(dir string, opts CmdUpdateOptions) (*types.Meta, error) { } // write codelab and its metadata - if err := writeCodelab(newdir, clab.Codelab, &meta.Context); err != nil { + if err := writeCodelab(newdir, clab.Codelab, opts.ExtraVars, &meta.Context); err != nil { return nil, err } diff --git a/claat/cmd/util.go b/claat/cmd/util.go index 60443c031..e65ab8c28 100644 --- a/claat/cmd/util.go +++ b/claat/cmd/util.go @@ -20,8 +20,6 @@ package cmd import ( - "encoding/json" - "log" // allow parsers to register themselves _ "github.com/googlecodelabs/tools/claat/parser/gdoc" @@ -42,25 +40,7 @@ const ( reportOk = "ok\t%s" ) -var ExtraVars map[string]string // Extra template variables passed on the command line. - // isStdout reports whether filename is stdout. func isStdout(filename string) bool { return filename == stdout } - -// ParseExtraVars parses extra template variables from command line. -// extra is any additional arguments to pass to format templates. Should be formatted as JSON objects of string:string KV pairs. -func ParseExtraVars(extra string) (map[string]string, error) { - vars := make(map[string]string) - if extra == "" { - return vars, nil - } - b := []byte(extra) - err := json.Unmarshal(b, &vars) - if err != nil { - log.Printf("Error parsing additional template data: %v", err) - return nil, err - } - return vars, nil -} diff --git a/claat/main.go b/claat/main.go index d83cd1fab..b5991ef47 100644 --- a/claat/main.go +++ b/claat/main.go @@ -20,6 +20,7 @@ package main import ( + "encoding/json" "flag" "fmt" "log" @@ -62,8 +63,7 @@ func main() { flag.Usage = usage flag.CommandLine.Parse(os.Args[2:]) - var err error - cmd.ExtraVars, err = cmd.ParseExtraVars(*extra) + extraVars, err := ParseExtraVars(*extra) if err != nil { os.Exit(1) } @@ -74,6 +74,7 @@ func main() { exitCode = cmd.CmdExport(cmd.CmdExportOptions{ AuthToken: *authToken, Expenv: *expenv, + ExtraVars: extraVars, GlobalGA: *globalGA, Output: *output, Prefix: *prefix, @@ -86,6 +87,7 @@ func main() { case "update": exitCode = cmd.CmdUpdate(cmd.CmdUpdateOptions{ AuthToken: *authToken, + ExtraVars: extraVars, GlobalGA: *globalGA, Prefix: *prefix, }) @@ -100,6 +102,22 @@ func main() { os.Exit(exitCode) } +// ParseExtraVars parses extra template variables from command line. +// extra is any additional arguments to pass to format templates. Should be formatted as JSON objects of string:string KV pairs. +func ParseExtraVars(extra string) (map[string]string, error) { + vars := make(map[string]string) + if extra == "" { + return vars, nil + } + b := []byte(extra) + err := json.Unmarshal(b, &vars) + if err != nil { + log.Printf("Error parsing additional template data: %v", err) + return nil, err + } + return vars, nil +} + // usage prints usageText and program arguments to stderr. func usage() { fmt.Fprint(os.Stderr, usageText)