From eb5040e69ebebf68c9306ac2d8e33d5f46c6fbef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 6 Jan 2016 11:59:28 +0100 Subject: [PATCH] Treat write errors in markdown doc generation This code was already using io.Writer, but was completely ignoring write errors. The most worrying part is how GenMarkdownTreeCustom used an unnecessary buffer to then dump all of its contents on a file, and instead of returning an error on file creation/writing, it would just exit the entire program. --- doc/md_docs.go | 117 ++++++++++++++++++++++++++++---------------- doc/md_docs.md | 4 +- doc/md_docs_test.go | 8 ++- 3 files changed, 82 insertions(+), 47 deletions(-) diff --git a/doc/md_docs.go b/doc/md_docs.go index 21b48232..666e9495 100644 --- a/doc/md_docs.go +++ b/doc/md_docs.go @@ -14,7 +14,6 @@ package doc import ( - "bytes" "fmt" "io" "os" @@ -25,29 +24,38 @@ import ( "github.com/spf13/cobra" ) -func printOptions(out io.Writer, cmd *cobra.Command, name string) { +func printOptions(w io.Writer, cmd *cobra.Command, name string) error { flags := cmd.NonInheritedFlags() - flags.SetOutput(out) + flags.SetOutput(w) if flags.HasFlags() { - fmt.Fprintf(out, "### Options\n\n```\n") + if _, err := fmt.Fprintf(w, "### Options\n\n```\n"); err != nil { + return err + } flags.PrintDefaults() - fmt.Fprintf(out, "```\n\n") + if _, err := fmt.Fprintf(w, "```\n\n"); err != nil { + return err + } } parentFlags := cmd.InheritedFlags() - parentFlags.SetOutput(out) + parentFlags.SetOutput(w) if parentFlags.HasFlags() { - fmt.Fprintf(out, "### Options inherited from parent commands\n\n```\n") + if _, err := fmt.Fprintf(w, "### Options inherited from parent commands\n\n```\n"); err != nil { + return err + } parentFlags.PrintDefaults() - fmt.Fprintf(out, "```\n\n") + if _, err := fmt.Fprintf(w, "```\n\n"); err != nil { + return err + } } + return nil } -func GenMarkdown(cmd *cobra.Command, out io.Writer) { - GenMarkdownCustom(cmd, out, func(s string) string { return s }) +func GenMarkdown(cmd *cobra.Command, w io.Writer) error { + return GenMarkdownCustom(cmd, w, func(s string) string { return s }) } -func GenMarkdownCustom(cmd *cobra.Command, out io.Writer, linkHandler func(string) string) { +func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) string) error { name := cmd.CommandPath() short := cmd.Short @@ -56,29 +64,49 @@ func GenMarkdownCustom(cmd *cobra.Command, out io.Writer, linkHandler func(strin long = short } - fmt.Fprintf(out, "## %s\n\n", name) - fmt.Fprintf(out, "%s\n\n", short) - fmt.Fprintf(out, "### Synopsis\n\n") - fmt.Fprintf(out, "\n%s\n\n", long) + if _, err := fmt.Fprintf(w, "## %s\n\n", name); err != nil { + return err + } + if _, err := fmt.Fprintf(w, "%s\n\n", short); err != nil { + return err + } + if _, err := fmt.Fprintf(w, "### Synopsis\n\n"); err != nil { + return err + } + if _, err := fmt.Fprintf(w, "\n%s\n\n", long); err != nil { + return err + } if cmd.Runnable() { - fmt.Fprintf(out, "```\n%s\n```\n\n", cmd.UseLine()) + if _, err := fmt.Fprintf(w, "```\n%s\n```\n\n", cmd.UseLine()); err != nil { + return err + } } if len(cmd.Example) > 0 { - fmt.Fprintf(out, "### Examples\n\n") - fmt.Fprintf(out, "```\n%s\n```\n\n", cmd.Example) + if _, err := fmt.Fprintf(w, "### Examples\n\n"); err != nil { + return err + } + if _, err := fmt.Fprintf(w, "```\n%s\n```\n\n", cmd.Example); err != nil { + return err + } } - printOptions(out, cmd, name) + if err := printOptions(w, cmd, name); err != nil { + return err + } if hasSeeAlso(cmd) { - fmt.Fprintf(out, "### SEE ALSO\n") + if _, err := fmt.Fprintf(w, "### SEE ALSO\n"); err != nil { + return err + } if cmd.HasParent() { parent := cmd.Parent() pname := parent.CommandPath() link := pname + ".md" link = strings.Replace(link, " ", "_", -1) - fmt.Fprintf(out, "* [%s](%s)\t - %s\n", pname, linkHandler(link), parent.Short) + if _, err := fmt.Fprintf(w, "* [%s](%s)\t - %s\n", pname, linkHandler(link), parent.Short); err != nil { + return err + } cmd.VisitParents(func(c *cobra.Command) { if c.DisableAutoGenTag { cmd.DisableAutoGenTag = c.DisableAutoGenTag @@ -96,48 +124,51 @@ func GenMarkdownCustom(cmd *cobra.Command, out io.Writer, linkHandler func(strin cname := name + " " + child.Name() link := cname + ".md" link = strings.Replace(link, " ", "_", -1) - fmt.Fprintf(out, "* [%s](%s)\t - %s\n", cname, linkHandler(link), child.Short) + if _, err := fmt.Fprintf(w, "* [%s](%s)\t - %s\n", cname, linkHandler(link), child.Short); err != nil { + return err + } + } + if _, err := fmt.Fprintf(w, "\n"); err != nil { + return err } - fmt.Fprintf(out, "\n") } if !cmd.DisableAutoGenTag { - fmt.Fprintf(out, "###### Auto generated by spf13/cobra on %s\n", time.Now().Format("2-Jan-2006")) + if _, err := fmt.Fprintf(w, "###### Auto generated by spf13/cobra on %s\n", time.Now().Format("2-Jan-2006")); err != nil { + return err + } } + return nil } -func GenMarkdownTree(cmd *cobra.Command, dir string) { +func GenMarkdownTree(cmd *cobra.Command, dir string) error { identity := func(s string) string { return s } emptyStr := func(s string) string { return "" } - GenMarkdownTreeCustom(cmd, dir, emptyStr, identity) + return GenMarkdownTreeCustom(cmd, dir, emptyStr, identity) } -func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHandler func(string) string) { +func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHandler func(string) string) error { for _, c := range cmd.Commands() { if !c.IsAvailableCommand() || c.IsHelpCommand() { continue } - GenMarkdownTreeCustom(c, dir, filePrepender, linkHandler) + if err := GenMarkdownTreeCustom(c, dir, filePrepender, linkHandler); err != nil { + return err + } } - out := new(bytes.Buffer) - - GenMarkdownCustom(cmd, out, linkHandler) filename := cmd.CommandPath() filename = dir + strings.Replace(filename, " ", "_", -1) + ".md" - outFile, err := os.Create(filename) + f, err := os.Create(filename) if err != nil { - fmt.Println(err) - os.Exit(1) + return err } - defer outFile.Close() - _, err = outFile.WriteString(filePrepender(filename)) - if err != nil { - fmt.Println(err) - os.Exit(1) + defer f.Close() + + if _, err := io.WriteString(f, filePrepender(filename)); err != nil { + return err } - _, err = outFile.Write(out.Bytes()) - if err != nil { - fmt.Println(err) - os.Exit(1) + if err := GenMarkdownCustom(cmd, f, linkHandler); err != nil { + return err } + return nil } diff --git a/doc/md_docs.md b/doc/md_docs.md index 8c6a00ab..da35f92e 100644 --- a/doc/md_docs.md +++ b/doc/md_docs.md @@ -39,13 +39,13 @@ This will write the markdown doc for ONLY "cmd" into the out, buffer. Both `GenMarkdown` and `GenMarkdownTree` have alternate versions with callbacks to get some control of the output: ```go -func GenMarkdownTreeCustom(cmd *Command, dir string, filePrepender, linkHandler func(string) string) { +func GenMarkdownTreeCustom(cmd *Command, dir string, filePrepender, linkHandler func(string) string) error { //... } ``` ```go -func GenMarkdownCustom(cmd *Command, out *bytes.Buffer, linkHandler func(string) string) { +func GenMarkdownCustom(cmd *Command, out *bytes.Buffer, linkHandler func(string) string) error { //... } ``` diff --git a/doc/md_docs_test.go b/doc/md_docs_test.go index 4e9d6a92..86ee0293 100644 --- a/doc/md_docs_test.go +++ b/doc/md_docs_test.go @@ -21,7 +21,9 @@ func TestGenMdDoc(t *testing.T) { out := new(bytes.Buffer) // We generate on s subcommand so we have both subcommands and parents - GenMarkdown(cmdEcho, out) + if err := GenMarkdown(cmdEcho, out); err != nil { + t.Fatal(err) + } found := out.String() // Our description @@ -75,7 +77,9 @@ func TestGenMdNoTag(t *testing.T) { cmdRootWithRun.PersistentFlags().StringVarP(&flags2a, "rootflag", "r", "two", strtwoParentHelp) out := new(bytes.Buffer) - GenMarkdown(c, out) + if err := GenMarkdown(c, out); err != nil { + t.Fatal(err) + } found := out.String() unexpected := "Auto generated"