From 5fa1003a36c00b0066d588b3dc3d183bb9d21c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 6 Jan 2016 11:49:16 +0100 Subject: [PATCH 1/4] Remove unused outs global var --- doc/cmd_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/cmd_test.go b/doc/cmd_test.go index d49d186..a4b5568 100644 --- a/doc/cmd_test.go +++ b/doc/cmd_test.go @@ -11,7 +11,7 @@ import ( ) var flagb1, flagb2, flagb3, flagbr, flagbp bool -var flags1, flags2a, flags2b, flags3, outs string +var flags1, flags2a, flags2b, flags3 string var flagi1, flagi2, flagi3, flagir int const strtwoParentHelp = "help message for parent flag strtwo" From ea06b29c101781006cb324289ef453102250bb25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 6 Jan 2016 11:49:26 +0100 Subject: [PATCH 2/4] Simplify GenMarkdownTreeCustom signature --- doc/md_docs.go | 2 +- doc/md_docs.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/md_docs.go b/doc/md_docs.go index 79bb687..21b4823 100644 --- a/doc/md_docs.go +++ b/doc/md_docs.go @@ -111,7 +111,7 @@ func GenMarkdownTree(cmd *cobra.Command, dir string) { GenMarkdownTreeCustom(cmd, dir, emptyStr, identity) } -func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender func(string) string, linkHandler func(string) string) { +func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHandler func(string) string) { for _, c := range cmd.Commands() { if !c.IsAvailableCommand() || c.IsHelpCommand() { continue diff --git a/doc/md_docs.md b/doc/md_docs.md index 41259d2..8c6a00a 100644 --- a/doc/md_docs.md +++ b/doc/md_docs.md @@ -39,7 +39,7 @@ 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 func(string) string, linkHandler func(string) string) { +func GenMarkdownTreeCustom(cmd *Command, dir string, filePrepender, linkHandler func(string) string) { //... } ``` 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 3/4] 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 21b4823..666e949 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 8c6a00a..da35f92 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 4e9d6a9..86ee029 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" From 5df1341f9327a2aa23f979ab4640898687c13850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 6 Jan 2016 12:21:23 +0100 Subject: [PATCH 4/4] Treat write errors in man doc generation Just like the last commit, but now for manpages. genMan still works with a buffer and returns []byte instead of working directly with an io.Writer. This is because, in turn, md2man takes byte slices instead of readers and writers. Wrapping genMan around a writer is unnecessary especially since it's not an exported function, and also because we'd still need a buffer to get the output bytes. --- doc/man_docs.go | 48 +++++++++++++++++++++----------------------- doc/man_docs_test.go | 8 ++++++-- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/doc/man_docs.go b/doc/man_docs.go index 71e020d..942b0aa 100644 --- a/doc/man_docs.go +++ b/doc/man_docs.go @@ -32,7 +32,7 @@ import ( // correctly if your command names have - in them. If you have `cmd` with two // subcmds, `sub` and `sub-third`. And `sub` has a subcommand called `third` // it is undefined which help output will be in the file `cmd-sub-third.1`. -func GenManTree(cmd *cobra.Command, header *GenManHeader, dir string) { +func GenManTree(cmd *cobra.Command, header *GenManHeader, dir string) error { if header == nil { header = &GenManHeader{} } @@ -40,31 +40,28 @@ func GenManTree(cmd *cobra.Command, header *GenManHeader, dir string) { if !c.IsAvailableCommand() || c.IsHelpCommand() { continue } - GenManTree(c, header, dir) + if err := GenManTree(c, header, dir); err != nil { + return err + } } - out := new(bytes.Buffer) - needToResetTitle := header.Title == "" - GenMan(cmd, header, out) + filename := cmd.CommandPath() + filename = dir + strings.Replace(filename, " ", "-", -1) + ".1" + f, err := os.Create(filename) + if err != nil { + return err + } + defer f.Close() + + if err := GenMan(cmd, header, f); err != nil { + return err + } if needToResetTitle { header.Title = "" } - - filename := cmd.CommandPath() - filename = dir + strings.Replace(filename, " ", "-", -1) + ".1" - outFile, err := os.Create(filename) - if err != nil { - fmt.Println(err) - os.Exit(1) - } - defer outFile.Close() - _, err = outFile.Write(out.Bytes()) - if err != nil { - fmt.Println(err) - os.Exit(1) - } + return nil } // GenManHeader is a lot like the .TH header at the start of man pages. These @@ -80,15 +77,16 @@ type GenManHeader struct { Manual string } -// GenMan will generate a man page for the given command in the out buffer. -// The header argument may be nil, however obviously out may not. -func GenMan(cmd *cobra.Command, header *GenManHeader, out io.Writer) { +// GenMan will generate a man page for the given command and write it to +// w. The header argument may be nil, however obviously w may not. +func GenMan(cmd *cobra.Command, header *GenManHeader, w io.Writer) error { if header == nil { header = &GenManHeader{} } - buf := genMan(cmd, header) - final := mangen.Render(buf) - out.Write(final) + b := genMan(cmd, header) + final := mangen.Render(b) + _, err := w.Write(final) + return err } func fillHeader(header *GenManHeader, name string) { diff --git a/doc/man_docs_test.go b/doc/man_docs_test.go index 4e8f706..3083125 100644 --- a/doc/man_docs_test.go +++ b/doc/man_docs_test.go @@ -29,7 +29,9 @@ func TestGenManDoc(t *testing.T) { Section: "2", } // We generate on a subcommand so we have both subcommands and parents - GenMan(cmdEcho, header, out) + if err := GenMan(cmdEcho, header, out); err != nil { + t.Fatal(err) + } found := out.String() // Make sure parent has - in CommandPath() in SEE ALSO: @@ -85,7 +87,9 @@ func TestGenManNoGenTag(t *testing.T) { Section: "2", } // We generate on a subcommand so we have both subcommands and parents - GenMan(cmdEcho, header, out) + if err := GenMan(cmdEcho, header, out); err != nil { + t.Fatal(err) + } found := out.String() unexpected := translate("#HISTORY")