From 1c0cf42f16fe18fcafa559cdece27e300c0ed725 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Jun 2022 10:03:33 -0500 Subject: [PATCH 1/6] feat: Reserve "-v" on root. Add "version" subcommand --- cli/root.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/cli/root.go b/cli/root.go index d2fc267221594..b9416e4fa79c6 100644 --- a/cli/root.go +++ b/cli/root.go @@ -90,6 +90,7 @@ func Root() *cobra.Command { users(), portForward(), workspaceAgent(), + versionCmd(), ) cmd.SetUsageTemplate(usageTemplate()) @@ -107,9 +108,32 @@ func Root() *cobra.Command { cmd.PersistentFlags().Bool(varNoOpen, false, "Block automatically opening URLs in the browser.") _ = cmd.PersistentFlags().MarkHidden(varNoOpen) + // Cobra automatically uses "-v" and "--version" for printing the version on + // the root cmd. If we decide to use "-v" as a verbose flag on the root, + // then that will be a behavior change. So we should just reserve "-v" + // and make users use "coder --version" or "coder version" + cmd.Flags().BoolP("placeholder", "v", false, "This flag is a placeholder to make the cobra version flag work appropriately") + _ = cmd.Flags().MarkHidden("placeholder") + return cmd } +// versionCmd comes from example in cobra issue. +// https://github.com/spf13/cobra/issues/724 +func versionCmd() *cobra.Command { + return &cobra.Command{ + Use: "version", + Short: "Version for coder", + Example: "coder version", + RunE: func(cmd *cobra.Command, args []string) error { + // Use "--version" behavior on root. + root := cmd.Root() + root.SetArgs([]string{"--version"}) + return root.Execute() + }, + } +} + // createClient returns a new client from the command context. // It reads from global configuration files if flags are not set. func createClient(cmd *cobra.Command) (*codersdk.Client, error) { From bec0e88e11e28c1d3f63c3a30af1aabc45412565 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Jun 2022 10:13:47 -0500 Subject: [PATCH 2/6] test: Add unit test for version cmd --- cli/root_test.go | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/cli/root_test.go b/cli/root_test.go index 28206cc46707f..f4f2b49e869f7 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -1,8 +1,11 @@ package cli_test import ( + "bytes" "testing" + "github.com/coder/coder/buildinfo" + "github.com/stretchr/testify/require" "github.com/coder/coder/cli" @@ -15,8 +18,43 @@ func TestRoot(t *testing.T) { cmd, _ := clitest.New(t, "delete") - cmd, err := cmd.ExecuteC() + err := cmd.Execute() errStr := cli.FormatCobraError(err, cmd) require.Contains(t, errStr, "Run 'coder delete --help' for usage.") }) + + // The "-v" short flag should return the usage output, not a version output + t.Run("TestVShortFlag", func(t *testing.T) { + t.Parallel() + + buf := new(bytes.Buffer) + cmd, _ := clitest.New(t, "-v") + cmd.SetOut(buf) + + err := cmd.Execute() + require.NoError(t, err) + // Check the usage string is the output when using "-v" + require.Contains(t, buf.String(), cmd.UsageString()) + require.Contains(t, buf.String(), cmd.Root().Long) + }) + + t.Run("TestVersion", func(t *testing.T) { + t.Parallel() + + bufFlag := new(bytes.Buffer) + cmd, _ := clitest.New(t, "--version") + cmd.SetOut(bufFlag) + err := cmd.Execute() + require.NoError(t, err) + + bufCmd := new(bytes.Buffer) + cmd, _ = clitest.New(t, "version") + cmd.SetOut(bufCmd) + err = cmd.Execute() + require.NoError(t, err) + + require.Equal(t, bufFlag.String(), bufCmd.String(), "cmd and flag identical output") + require.Contains(t, bufFlag.String(), buildinfo.Version(), "has version") + require.Contains(t, bufFlag.String(), buildinfo.ExternalURL(), "has url") + }) } From ae779c6276a5f305273343c3d5d4836d6a16c890 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Jun 2022 10:18:00 -0500 Subject: [PATCH 3/6] Fix test --- cli/root_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/root_test.go b/cli/root_test.go index f4f2b49e869f7..4ab0daaa4db66 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -18,7 +18,7 @@ func TestRoot(t *testing.T) { cmd, _ := clitest.New(t, "delete") - err := cmd.Execute() + cmd, err := cmd.ExecuteC() errStr := cli.FormatCobraError(err, cmd) require.Contains(t, errStr, "Run 'coder delete --help' for usage.") }) From b509594c34677867105483e423f2d406139eef3e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Jun 2022 13:51:04 -0500 Subject: [PATCH 4/6] Only support 'coder version' --- cli/root.go | 39 ++++++++++++--------------------------- cli/root_test.go | 31 +++++-------------------------- 2 files changed, 17 insertions(+), 53 deletions(-) diff --git a/cli/root.go b/cli/root.go index b9416e4fa79c6..158ccf961aa6b 100644 --- a/cli/root.go +++ b/cli/root.go @@ -53,7 +53,6 @@ func init() { func Root() *cobra.Command { cmd := &cobra.Command{ Use: "coder", - Version: buildinfo.Version(), SilenceErrors: true, SilenceUsage: true, Long: `Coder — A tool for provisioning self-hosted development environments. @@ -94,7 +93,6 @@ func Root() *cobra.Command { ) cmd.SetUsageTemplate(usageTemplate()) - cmd.SetVersionTemplate(versionTemplate()) cmd.PersistentFlags().String(varURL, "", "Specify the URL to your deployment.") cmd.PersistentFlags().String(varToken, "", "Specify an authentication token.") @@ -108,28 +106,26 @@ func Root() *cobra.Command { cmd.PersistentFlags().Bool(varNoOpen, false, "Block automatically opening URLs in the browser.") _ = cmd.PersistentFlags().MarkHidden(varNoOpen) - // Cobra automatically uses "-v" and "--version" for printing the version on - // the root cmd. If we decide to use "-v" as a verbose flag on the root, - // then that will be a behavior change. So we should just reserve "-v" - // and make users use "coder --version" or "coder version" - cmd.Flags().BoolP("placeholder", "v", false, "This flag is a placeholder to make the cobra version flag work appropriately") - _ = cmd.Flags().MarkHidden("placeholder") - return cmd } -// versionCmd comes from example in cobra issue. -// https://github.com/spf13/cobra/issues/724 +// versionCmd prints the coder version func versionCmd() *cobra.Command { return &cobra.Command{ Use: "version", - Short: "Version for coder", + Short: "Show coder version", Example: "coder version", RunE: func(cmd *cobra.Command, args []string) error { - // Use "--version" behavior on root. - root := cmd.Root() - root.SetArgs([]string{"--version"}) - return root.Execute() + var str strings.Builder + str.WriteString(fmt.Sprintf("Coder %s", buildinfo.Version())) + buildTime, valid := buildinfo.Time() + if valid { + str.WriteString(" " + buildTime.Format(time.UnixDate)) + } + str.WriteString("\r\n" + buildinfo.ExternalURL() + "\r\n") + + _, _ = fmt.Fprint(cmd.OutOrStdout(), str.String()) + return nil }, } } @@ -310,17 +306,6 @@ Use "{{.CommandPath}} [command] --help" for more information about a command. {{end}}` } -func versionTemplate() string { - template := `Coder {{printf "%s" .Version}}` - buildTime, valid := buildinfo.Time() - if valid { - template += " " + buildTime.Format(time.UnixDate) - } - template += "\r\n" + buildinfo.ExternalURL() - template += "\r\n" - return template -} - // FormatCobraError colorizes and adds "--help" docs to cobra commands. func FormatCobraError(err error, cmd *cobra.Command) string { helpErrMsg := fmt.Sprintf("Run '%s --help' for usage.", cmd.CommandPath()) diff --git a/cli/root_test.go b/cli/root_test.go index 4ab0daaa4db66..7aa3d0bf83e7d 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -23,38 +23,17 @@ func TestRoot(t *testing.T) { require.Contains(t, errStr, "Run 'coder delete --help' for usage.") }) - // The "-v" short flag should return the usage output, not a version output - t.Run("TestVShortFlag", func(t *testing.T) { + t.Run("TestVersion", func(t *testing.T) { t.Parallel() buf := new(bytes.Buffer) - cmd, _ := clitest.New(t, "-v") + cmd, _ := clitest.New(t, "version") cmd.SetOut(buf) - err := cmd.Execute() require.NoError(t, err) - // Check the usage string is the output when using "-v" - require.Contains(t, buf.String(), cmd.UsageString()) - require.Contains(t, buf.String(), cmd.Root().Long) - }) - - t.Run("TestVersion", func(t *testing.T) { - t.Parallel() - - bufFlag := new(bytes.Buffer) - cmd, _ := clitest.New(t, "--version") - cmd.SetOut(bufFlag) - err := cmd.Execute() - require.NoError(t, err) - - bufCmd := new(bytes.Buffer) - cmd, _ = clitest.New(t, "version") - cmd.SetOut(bufCmd) - err = cmd.Execute() - require.NoError(t, err) - require.Equal(t, bufFlag.String(), bufCmd.String(), "cmd and flag identical output") - require.Contains(t, bufFlag.String(), buildinfo.Version(), "has version") - require.Contains(t, bufFlag.String(), buildinfo.ExternalURL(), "has url") + output := buf.String() + require.Contains(t, output, buildinfo.Version(), "has version") + require.Contains(t, output, buildinfo.ExternalURL(), "has url") }) } From 273ad889f29b50da2f53eaed24521a4d1f375921 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Jun 2022 17:19:37 -0500 Subject: [PATCH 5/6] rename test --- cli/root_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/root_test.go b/cli/root_test.go index 7aa3d0bf83e7d..48298267a02db 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -23,7 +23,7 @@ func TestRoot(t *testing.T) { require.Contains(t, errStr, "Run 'coder delete --help' for usage.") }) - t.Run("TestVersion", func(t *testing.T) { + t.Run("Version", func(t *testing.T) { t.Parallel() buf := new(bytes.Buffer) From 5a6b0f284d33e0969c6fbfe361949b83deb20bbc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Jun 2022 17:30:18 -0500 Subject: [PATCH 6/6] Linter --- cli/root.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/root.go b/cli/root.go index 158ccf961aa6b..a112de28f5b28 100644 --- a/cli/root.go +++ b/cli/root.go @@ -117,12 +117,12 @@ func versionCmd() *cobra.Command { Example: "coder version", RunE: func(cmd *cobra.Command, args []string) error { var str strings.Builder - str.WriteString(fmt.Sprintf("Coder %s", buildinfo.Version())) + _, _ = str.WriteString(fmt.Sprintf("Coder %s", buildinfo.Version())) buildTime, valid := buildinfo.Time() if valid { - str.WriteString(" " + buildTime.Format(time.UnixDate)) + _, _ = str.WriteString(" " + buildTime.Format(time.UnixDate)) } - str.WriteString("\r\n" + buildinfo.ExternalURL() + "\r\n") + _, _ = str.WriteString("\r\n" + buildinfo.ExternalURL() + "\r\n") _, _ = fmt.Fprint(cmd.OutOrStdout(), str.String()) return nil