From 9dbf267e54fd679563474e442d0956c444f5f328 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 3 Sep 2021 12:33:47 -0400 Subject: [PATCH 1/3] codespace flag, deprecate argument --- cmd/ghcs/ports.go | 93 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 29 deletions(-) diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index dd8f70131..4ef460b51 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -18,31 +18,25 @@ import ( "github.com/spf13/cobra" ) -// portOptions represents the options accepted by the ports command. -type portsOptions struct { - // CodespaceName is the name of the codespace, optional. - codespaceName string - - // AsJSON dictates whether the command returns a json output or not, optional. - asJSON bool -} - // newPortsCmd returns a Cobra "ports" command that displays a table of available ports, // according to the specified flags. func newPortsCmd() *cobra.Command { - opts := &portsOptions{} + var ( + codespace string + asJSON bool + ) portsCmd := &cobra.Command{ Use: "ports", Short: "List ports in a codespace", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return ports(opts) + return ports(codespace, asJSON) }, } - portsCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "The `name` of the codespace to use") - portsCmd.Flags().BoolVar(&opts.asJSON, "json", false, "Output as JSON") + portsCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") + portsCmd.Flags().BoolVar(&asJSON, "json", false, "Output as JSON") portsCmd.AddCommand(newPortsPublicCmd()) portsCmd.AddCommand(newPortsPrivateCmd()) @@ -55,17 +49,17 @@ func init() { rootCmd.AddCommand(newPortsCmd()) } -func ports(opts *portsOptions) error { +func ports(codespaceName string, asJSON bool) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() - log := output.NewLogger(os.Stdout, os.Stderr, opts.asJSON) + log := output.NewLogger(os.Stdout, os.Stderr, asJSON) user, err := apiClient.GetUser(ctx) if err != nil { return fmt.Errorf("error getting user: %v", err) } - codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, opts.codespaceName) + codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { if err == codespaces.ErrNoCodespaces { return err @@ -92,7 +86,7 @@ func ports(opts *portsOptions) error { _, _ = log.Errorf("Failed to get port names: %v\n", devContainerResult.err.Error()) } - table := output.NewTable(os.Stdout, opts.asJSON) + table := output.NewTable(os.Stdout, asJSON) table.SetHeader([]string{"Label", "Port", "Public", "Browse URL"}) for _, port := range ports { sourcePort := strconv.Itoa(port.SourcePort) @@ -161,28 +155,54 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod // newPortsPublicCmd returns a Cobra "ports public" subcommand, which makes a given port public. func newPortsPublicCmd() *cobra.Command { - return &cobra.Command{ - Use: "public ", + var codespace string + + newPortsPublicCmd := &cobra.Command{ + Use: "public ", Short: "Mark port as public", - Args: cobra.ExactArgs(2), + Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { log := output.NewLogger(os.Stdout, os.Stderr, false) - return updatePortVisibility(log, args[0], args[1], true) + + port := args[0] + if len(args) > 1 { + log.Println(" argument is deprecated. Use --codespace instead.") + codespace, port = args[0], args[1] + } + + return updatePortVisibility(log, codespace, port, true) }, } + + newPortsPublicCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") + + return newPortsPublicCmd } // newPortsPrivateCmd returns a Cobra "ports private" subcommand, which makes a given port private. func newPortsPrivateCmd() *cobra.Command { - return &cobra.Command{ - Use: "private ", + var codespace string + + newPortsPrivateCmd := &cobra.Command{ + Use: "private ", Short: "Mark port as private", - Args: cobra.ExactArgs(2), + Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { log := output.NewLogger(os.Stdout, os.Stderr, false) - return updatePortVisibility(log, args[0], args[1], false) + + port := args[0] + if len(args) > 1 { + log.Println(" argument is deprecated. Use --codespace instead.") + codespace, port = args[0], args[1] + } + + return updatePortVisibility(log, codespace, port, false) }, } + + newPortsPrivateCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") + + return newPortsPrivateCmd } func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, public bool) error { @@ -230,15 +250,30 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, // NewPortsForwardCmd returns a Cobra "ports forward" subcommand, which forwards a set of // port pairs from the codespace to localhost. func newPortsForwardCmd() *cobra.Command { - return &cobra.Command{ - Use: "forward :", + var codespace string + + newPortsForwardCmd := &cobra.Command{ + Use: "forward :...", Short: "Forward ports", - Args: cobra.MinimumNArgs(2), + Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { log := output.NewLogger(os.Stdout, os.Stderr, false) - return forwardPorts(log, args[0], args[1:]) + + ports := args[0:] + if len(args) > 1 && !strings.Contains(args[0], ":") { + // assume this is a codespace name + log.Println(" argument is deprecated. Use --codespace instead.") + codespace = args[0] + ports = args[1:] + } + + return forwardPorts(log, codespace, ports) }, } + + newPortsForwardCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") + + return newPortsForwardCmd } func forwardPorts(log *output.Logger, codespaceName string, ports []string) error { From 230bf640c5f020ec866596c5fbf53011c355b54d Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Thu, 9 Sep 2021 11:06:18 -0400 Subject: [PATCH 2/3] global flag, choose codespace when empty --- cmd/ghcs/ports.go | 64 +++++++++++++++++-------------- internal/codespaces/codespaces.go | 2 + 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 2fab4254d..2bb3e6917 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -37,7 +37,7 @@ func newPortsCmd() *cobra.Command { }, } - portsCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") + portsCmd.PersistentFlags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") portsCmd.Flags().BoolVar(&asJSON, "json", false, "Output as JSON") portsCmd.AddCommand(newPortsPublicCmd()) @@ -157,18 +157,24 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod // newPortsPublicCmd returns a Cobra "ports public" subcommand, which makes a given port public. func newPortsPublicCmd() *cobra.Command { - var codespace string - newPortsPublicCmd := &cobra.Command{ Use: "public ", Short: "Mark port as public", Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + codespace, err := cmd.Flags().GetString("codespace") + if err != nil { + // should only happen if flag is not defined + // or if the flag is not of string type + // since it's a persistent flag that we control it should never happen + return fmt.Errorf("get codespace flag: %v", err) + } + log := output.NewLogger(os.Stdout, os.Stderr, false) port := args[0] if len(args) > 1 { - log.Println(" argument is deprecated. Use --codespace instead.") + log.Errorln(" argument is deprecated. Use --codespace instead.") codespace, port = args[0], args[1] } @@ -176,25 +182,29 @@ func newPortsPublicCmd() *cobra.Command { }, } - newPortsPublicCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") - return newPortsPublicCmd } // newPortsPrivateCmd returns a Cobra "ports private" subcommand, which makes a given port private. func newPortsPrivateCmd() *cobra.Command { - var codespace string - newPortsPrivateCmd := &cobra.Command{ Use: "private ", Short: "Mark port as private", Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + codespace, err := cmd.Flags().GetString("codespace") + if err != nil { + // should only happen if flag is not defined + // or if the flag is not of string type + // since it's a persistent flag that we control it should never happen + return fmt.Errorf("get codespace flag: %v", err) + } + log := output.NewLogger(os.Stdout, os.Stderr, false) port := args[0] if len(args) > 1 { - log.Println(" argument is deprecated. Use --codespace instead.") + log.Errorln(" argument is deprecated. Use --codespace instead.") codespace, port = args[0], args[1] } @@ -202,8 +212,6 @@ func newPortsPrivateCmd() *cobra.Command { }, } - newPortsPrivateCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") - return newPortsPrivateCmd } @@ -216,13 +224,11 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, return fmt.Errorf("error getting user: %v", err) } - token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) - if err != nil { - return fmt.Errorf("error getting codespace token: %v", err) - } - - codespace, err := apiClient.GetCodespace(ctx, token, user.Login, codespaceName) + codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { + if err == codespaces.ErrNoCodespaces { + return err + } return fmt.Errorf("error getting codespace: %v", err) } @@ -252,19 +258,25 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, // NewPortsForwardCmd returns a Cobra "ports forward" subcommand, which forwards a set of // port pairs from the codespace to localhost. func newPortsForwardCmd() *cobra.Command { - var codespace string - newPortsForwardCmd := &cobra.Command{ Use: "forward :...", Short: "Forward ports", Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + codespace, err := cmd.Flags().GetString("codespace") + if err != nil { + // should only happen if flag is not defined + // or if the flag is not of string type + // since it's a persistent flag that we control it should never happen + return fmt.Errorf("get codespace flag: %v", err) + } + log := output.NewLogger(os.Stdout, os.Stderr, false) ports := args[0:] if len(args) > 1 && !strings.Contains(args[0], ":") { // assume this is a codespace name - log.Println(" argument is deprecated. Use --codespace instead.") + log.Errorln(" argument is deprecated. Use --codespace instead.") codespace = args[0] ports = args[1:] } @@ -273,8 +285,6 @@ func newPortsForwardCmd() *cobra.Command { }, } - newPortsForwardCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") - return newPortsForwardCmd } @@ -292,13 +302,11 @@ func forwardPorts(log *output.Logger, codespaceName string, ports []string) erro return fmt.Errorf("error getting user: %v", err) } - token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) - if err != nil { - return fmt.Errorf("error getting codespace token: %v", err) - } - - codespace, err := apiClient.GetCodespace(ctx, token, user.Login, codespaceName) + codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { + if err == codespaces.ErrNoCodespaces { + return err + } return fmt.Errorf("error getting codespace: %v", err) } diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index b5fa4a583..fd04b303e 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -124,6 +124,8 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use return lsclient.JoinWorkspace(ctx) } +// GetOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty. +// It then fetches the codespace token and the codespace record. func GetOrChooseCodespace(ctx context.Context, apiClient *api.API, user *api.User, codespaceName string) (codespace *api.Codespace, token string, err error) { if codespaceName == "" { codespace, err = ChooseCodespace(ctx, apiClient, user) From 9b9e533cb9697c63bac18062f1fc38ab07505d06 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 10 Sep 2021 13:32:30 -0400 Subject: [PATCH 3/3] add comment for special handling --- cmd/ghcs/ports.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 8b73626fa..4b190b36f 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -63,6 +63,7 @@ func ports(codespaceName string, asJSON bool) error { codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { + // TODO(josebalius): remove special handling of this error here and it other places if err == errNoCodespaces { return err }