Add --repo filter to more gh codespaces commands (#6669)

* Add --repo flag to commands

* Example of using common args

* Add -r to stop

* Add validation to the add helper

* Skip prompt for single option

* Migrate (mostly) everything to addGetOrChooseCodespaceCommandArgs

* Fix typo in logsCmd

* Use R instead of r

* Update a couple -r usages

* Refactor to codespace_selector

* Clean up selector, apply it in a couple examples

* Use apiClient instead in constructor

* Restore addDeprecatedRepoShorthand

* Finish implementing the commands

* Update existing tests to use the selector

* Add tests for selector

* linter

* Catch case where there's no conflict

* Make the flag persistent for ports

* Add support to stop
This commit is contained in:
Caleb Brose 2023-02-22 17:16:36 -06:00 committed by GitHub
parent aa1fe64cb6
commit 57c73e8239
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 432 additions and 155 deletions

View file

@ -10,7 +10,7 @@ import (
func newCodeCmd(app *App) *cobra.Command {
var (
codespace string
selector *CodespaceSelector
useInsiders bool
useWeb bool
)
@ -20,11 +20,12 @@ func newCodeCmd(app *App) *cobra.Command {
Short: "Open a codespace in Visual Studio Code",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.VSCode(cmd.Context(), codespace, useInsiders, useWeb)
return app.VSCode(cmd.Context(), selector, useInsiders, useWeb)
},
}
codeCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(codeCmd, app.apiClient)
codeCmd.Flags().BoolVar(&useInsiders, "insiders", false, "Use the insiders version of Visual Studio Code")
codeCmd.Flags().BoolVarP(&useWeb, "web", "w", false, "Use the web version of Visual Studio Code")
@ -32,8 +33,8 @@ func newCodeCmd(app *App) *cobra.Command {
}
// VSCode opens a codespace in the local VS VSCode application.
func (a *App) VSCode(ctx context.Context, codespaceName string, useInsiders bool, useWeb bool) error {
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
func (a *App) VSCode(ctx context.Context, selector *CodespaceSelector, useInsiders bool, useWeb bool) error {
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

View file

@ -69,7 +69,9 @@ func TestApp_VSCode(t *testing.T) {
apiClient: testCodeApiMock(),
io: ios,
}
if err := a.VSCode(context.Background(), tt.args.codespaceName, tt.args.useInsiders, tt.args.useWeb); (err != nil) != tt.wantErr {
selector := &CodespaceSelector{api: a.apiClient, codespaceName: tt.args.codespaceName}
if err := a.VSCode(context.Background(), selector, tt.args.useInsiders, tt.args.useWeb); (err != nil) != tt.wantErr {
t.Errorf("App.VSCode() error = %v, wantErr %v", err, tt.wantErr)
}
b.Verify(t, tt.wantURL)
@ -85,8 +87,9 @@ func TestApp_VSCode(t *testing.T) {
func TestPendingOperationDisallowsCode(t *testing.T) {
app := testingCodeApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.VSCode(context.Background(), "disabledCodespace", false, false); err != nil {
if err := app.VSCode(context.Background(), selector, false, false); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

View file

@ -0,0 +1,123 @@
package codespace
import (
"context"
"errors"
"fmt"
"strings"
"github.com/cli/cli/v2/internal/codespaces/api"
"github.com/spf13/cobra"
)
type CodespaceSelector struct {
api apiClient
repoName string
codespaceName string
}
var errNoFilteredCodespaces = errors.New("you have no codespaces meeting the filter criteria")
// AddCodespaceSelector adds persistent flags for selecting a codespace to the given command and returns a CodespaceSelector which applies them
func AddCodespaceSelector(cmd *cobra.Command, api apiClient) *CodespaceSelector {
cs := &CodespaceSelector{api: api}
cmd.PersistentFlags().StringVarP(&cs.codespaceName, "codespace", "c", "", "Name of the codespace")
cmd.PersistentFlags().StringVarP(&cs.repoName, "repo", "R", "", "Filter codespace selection by repository name (user/repo)")
cmd.MarkFlagsMutuallyExclusive("codespace", "repo")
return cs
}
func (cs *CodespaceSelector) Select(ctx context.Context) (codespace *api.Codespace, err error) {
if cs.codespaceName != "" {
codespace, err = cs.api.GetCodespace(ctx, cs.codespaceName, true)
if err != nil {
return nil, fmt.Errorf("getting full codespace details: %w", err)
}
} else {
codespaces, err := cs.fetchCodespaces(ctx)
if err != nil {
return nil, err
}
codespace, err = cs.chooseCodespace(ctx, codespaces)
if err != nil {
return nil, err
}
}
if codespace.PendingOperation {
return nil, fmt.Errorf(
"codespace is disabled while it has a pending operation: %s",
codespace.PendingOperationDisabledReason,
)
}
return codespace, nil
}
func (cs *CodespaceSelector) SelectName(ctx context.Context) (string, error) {
if cs.codespaceName != "" {
return cs.codespaceName, nil
}
codespaces, err := cs.fetchCodespaces(ctx)
if err != nil {
return "", err
}
codespace, err := cs.chooseCodespace(ctx, codespaces)
if err != nil {
return "", err
}
return codespace.Name, nil
}
func (cs *CodespaceSelector) fetchCodespaces(ctx context.Context) (codespaces []*api.Codespace, err error) {
codespaces, err = cs.api.ListCodespaces(ctx, api.ListCodespacesOptions{})
if err != nil {
return nil, fmt.Errorf("error getting codespaces: %w", err)
}
if len(codespaces) == 0 {
return nil, errNoCodespaces
}
// Note that repo filtering done here can also be done in api.ListCodespaces.
// We do it here instead so that we can differentiate no codespaces in general vs. none after filtering.
if cs.repoName != "" {
var filteredCodespaces []*api.Codespace
for _, c := range codespaces {
if !strings.EqualFold(c.Repository.FullName, cs.repoName) {
continue
}
filteredCodespaces = append(filteredCodespaces, c)
}
codespaces = filteredCodespaces
}
if len(codespaces) == 0 {
return nil, errNoFilteredCodespaces
}
return codespaces, err
}
func (cs *CodespaceSelector) chooseCodespace(ctx context.Context, codespaces []*api.Codespace) (codespace *api.Codespace, err error) {
skipPromptForSingleOption := cs.repoName != ""
codespace, err = chooseCodespaceFromList(ctx, codespaces, false, skipPromptForSingleOption)
if err != nil {
if err == errNoCodespaces {
return nil, err
}
return nil, fmt.Errorf("choosing codespace: %w", err)
}
return codespace, nil
}

View file

@ -0,0 +1,137 @@
package codespace
import (
"context"
"fmt"
"testing"
"github.com/cli/cli/v2/internal/codespaces/api"
)
func TestSelectWithCodespaceName(t *testing.T) {
wantName := "mock-codespace"
api := &apiClientMock{
GetCodespaceFunc: func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) {
if name != wantName {
t.Errorf("incorrect name: want %s, got %s", wantName, name)
}
return &api.Codespace{}, nil
},
}
cs := &CodespaceSelector{api: api, codespaceName: wantName}
_, err := cs.Select(context.Background())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}
func TestSelectNameWithCodespaceName(t *testing.T) {
wantName := "mock-codespace"
cs := &CodespaceSelector{codespaceName: wantName}
name, err := cs.SelectName(context.Background())
if name != wantName {
t.Errorf("incorrect name: want %s, got %s", wantName, name)
}
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}
func TestFetchCodespaces(t *testing.T) {
var (
repoA1 = &api.Codespace{Name: "1", Repository: api.Repository{FullName: "mock/A"}}
repoA2 = &api.Codespace{Name: "2", Repository: api.Repository{FullName: "mock/A"}}
repoB1 = &api.Codespace{Name: "1", Repository: api.Repository{FullName: "mock/B"}}
)
tests := []struct {
tName string
apiCodespaces []*api.Codespace
repoName string
wantCodespaces []*api.Codespace
wantErr error
}{
// Empty case
{
"empty", nil, "", nil, errNoCodespaces,
},
// Tests with no filtering
{
"no filtering, single codespace",
[]*api.Codespace{repoA1},
"",
[]*api.Codespace{repoA1},
nil,
},
{
"no filtering, multiple codespaces",
[]*api.Codespace{repoA1, repoA2, repoB1},
"",
[]*api.Codespace{repoA1, repoA2, repoB1},
nil,
},
// Test repo filtering
{
"repo filtering, single codespace",
[]*api.Codespace{repoA1},
"mock/A",
[]*api.Codespace{repoA1},
nil,
},
{
"repo filtering, multiple codespaces",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/A",
[]*api.Codespace{repoA1, repoA2},
nil,
},
{
"repo filtering, multiple codespaces 2",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/B",
[]*api.Codespace{repoB1},
nil,
},
{
"repo filtering, no matches",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/C",
nil,
errNoFilteredCodespaces,
},
}
for _, tt := range tests {
t.Run(tt.tName, func(t *testing.T) {
api := &apiClientMock{
ListCodespacesFunc: func(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error) {
return tt.apiCodespaces, nil
},
}
cs := &CodespaceSelector{api: api, repoName: tt.repoName}
codespaces, err := cs.fetchCodespaces(context.Background())
if err != tt.wantErr {
t.Errorf("expected error to be %v, got %v", tt.wantErr, err)
}
if fmt.Sprintf("%v", tt.wantCodespaces) != fmt.Sprintf("%v", codespaces) {
t.Errorf("expected codespaces to be %v, got %v", tt.wantCodespaces, codespaces)
}
})
}
}

View file

@ -102,21 +102,17 @@ type apiClient interface {
var errNoCodespaces = errors.New("you have no codespaces")
func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, error) {
codespaces, err := apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{})
if err != nil {
return nil, fmt.Errorf("error getting codespaces: %w", err)
}
return chooseCodespaceFromList(ctx, codespaces, false)
}
// chooseCodespaceFromList returns the codespace that the user has interactively selected from the list, or
// an error if there are no codespaces.
func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace, includeOwner bool) (*api.Codespace, error) {
func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace, includeOwner bool, skipPromptForSingleOption bool) (*api.Codespace, error) {
if len(codespaces) == 0 {
return nil, errNoCodespaces
}
if skipPromptForSingleOption && len(codespaces) == 1 {
return codespaces[0], nil
}
sortedCodespaces := codespaces
sort.Slice(sortedCodespaces, func(i, j int) bool {
return sortedCodespaces[i].CreatedAt > sortedCodespaces[j].CreatedAt
@ -154,35 +150,6 @@ func formatCodespacesForSelect(codespaces []*api.Codespace, includeOwner bool) [
return names
}
// getOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty.
// It then fetches the codespace record with full connection details.
// TODO(josebalius): accept a progress indicator or *App and show progress when fetching.
func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceName string) (codespace *api.Codespace, err error) {
if codespaceName == "" {
codespace, err = chooseCodespace(ctx, apiClient)
if err != nil {
if err == errNoCodespaces {
return nil, err
}
return nil, fmt.Errorf("choosing codespace: %w", err)
}
} else {
codespace, err = apiClient.GetCodespace(ctx, codespaceName, true)
if err != nil {
return nil, fmt.Errorf("getting full codespace details: %w", err)
}
}
if codespace.PendingOperation {
return nil, fmt.Errorf(
"codespace is disabled while it has a pending operation: %s",
codespace.PendingOperationDisabledReason,
)
}
return codespace, nil
}
func safeClose(closer io.Closer, err *error) {
if closeErr := closer.Close(); *err == nil {
*err = closeErr
@ -289,5 +256,9 @@ func addDeprecatedRepoShorthand(cmd *cobra.Command, target *string) error {
return fmt.Errorf("error marking `-r` shorthand as deprecated: %w", err)
}
if cmd.Flag("codespace") != nil {
cmd.MarkFlagsMutuallyExclusive("codespace", "repo-deprecated")
}
return nil
}

View file

@ -41,6 +41,8 @@ func newDeleteCmd(app *App) *cobra.Command {
prompter: &surveyPrompter{},
}
var selector *CodespaceSelector
deleteCmd := &cobra.Command{
Use: "delete",
Short: "Delete codespaces",
@ -54,6 +56,11 @@ func newDeleteCmd(app *App) *cobra.Command {
`),
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
// TODO: ideally we would use the selector directly, but the logic here is too intertwined with other flags to do so elegantly
// After the admin subcommand is added (see https://github.com/cli/cli/pull/6944#issuecomment-1419553639) we can revisit this.
opts.codespaceName = selector.codespaceName
opts.repoFilter = selector.repoName
if opts.deleteAll && opts.repoFilter != "" {
return cmdutil.FlagErrorf("both `--all` and `--repo` is not supported")
}
@ -64,13 +71,12 @@ func newDeleteCmd(app *App) *cobra.Command {
},
}
deleteCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace")
deleteCmd.Flags().BoolVar(&opts.deleteAll, "all", false, "Delete all codespaces")
deleteCmd.Flags().StringVarP(&opts.repoFilter, "repo", "R", "", "Delete codespaces for a `repository`")
if err := addDeprecatedRepoShorthand(deleteCmd, &opts.repoFilter); err != nil {
selector = AddCodespaceSelector(deleteCmd, app.apiClient)
if err := addDeprecatedRepoShorthand(deleteCmd, &selector.repoName); err != nil {
fmt.Fprintf(app.io.ErrOut, "%v\n", err)
}
deleteCmd.Flags().BoolVar(&opts.deleteAll, "all", false, "Delete all codespaces")
deleteCmd.Flags().BoolVarP(&opts.skipConfirm, "force", "f", false, "Skip confirmation for codespaces that contain unsaved changes")
deleteCmd.Flags().Uint16Var(&opts.keepDays, "days", 0, "Delete codespaces older than `N` days")
deleteCmd.Flags().StringVarP(&opts.orgName, "org", "o", "", "The `login` handle of the organization (admin-only)")
@ -100,7 +106,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) {
if !opts.deleteAll && opts.repoFilter == "" {
includeUsername := opts.orgName != ""
c, err := chooseCodespaceFromList(ctx, codespaces, includeUsername)
c, err := chooseCodespaceFromList(ctx, codespaces, includeUsername, false)
if err != nil {
return fmt.Errorf("error choosing codespace: %w", err)
}

View file

@ -2,6 +2,7 @@ package codespace
import (
"context"
"errors"
"fmt"
"github.com/cli/cli/v2/internal/codespaces/api"
@ -10,9 +11,9 @@ import (
)
type editOptions struct {
codespaceName string
displayName string
machine string
selector *CodespaceSelector
displayName string
machine string
}
func newEditCmd(app *App) *cobra.Command {
@ -31,7 +32,7 @@ func newEditCmd(app *App) *cobra.Command {
},
}
editCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(editCmd, app.apiClient)
editCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", "Set the display name")
editCmd.Flags().StringVar(&opts.displayName, "displayName", "", "display name")
if err := editCmd.Flags().MarkDeprecated("displayName", "use `--display-name` instead"); err != nil {
@ -44,21 +45,17 @@ func newEditCmd(app *App) *cobra.Command {
// Edits a codespace
func (a *App) Edit(ctx context.Context, opts editOptions) error {
codespaceName := opts.codespaceName
if codespaceName == "" {
selectedCodespace, err := chooseCodespace(ctx, a.apiClient)
if err != nil {
if err == errNoCodespaces {
return err
}
return fmt.Errorf("error choosing codespace: %w", err)
codespaceName, err := opts.selector.SelectName(ctx)
if err != nil {
// TODO: is there a cleaner way to do this?
if errors.Is(err, errNoCodespaces) || errors.Is(err, errNoFilteredCodespaces) {
return err
}
codespaceName = selectedCodespace.Name
return fmt.Errorf("error choosing codespace: %w", err)
}
a.StartProgressIndicatorWithLabel("Editing codespace")
_, err := a.apiClient.EditCodespace(ctx, codespaceName, &api.EditCodespaceParams{
_, err = a.apiClient.EditCodespace(ctx, codespaceName, &api.EditCodespaceParams{
DisplayName: opts.displayName,
Machine: opts.machine,
})

View file

@ -23,9 +23,9 @@ func TestEdit(t *testing.T) {
{
name: "edit codespace display name",
opts: editOptions{
codespaceName: "hubot",
displayName: "hubot-changed",
machine: "",
selector: &CodespaceSelector{codespaceName: "hubot"},
displayName: "hubot-changed",
machine: "",
},
wantEdits: &api.EditCodespaceParams{
DisplayName: "hubot-changed",
@ -54,9 +54,9 @@ func TestEdit(t *testing.T) {
{
name: "edit codespace machine",
opts: editOptions{
codespaceName: "hubot",
displayName: "",
machine: "machine",
selector: &CodespaceSelector{codespaceName: "hubot"},
displayName: "",
machine: "machine",
},
wantEdits: &api.EditCodespaceParams{
Machine: "machine",
@ -92,6 +92,11 @@ func TestEdit(t *testing.T) {
var err error
if tt.cliArgs == nil {
if tt.opts.selector == nil {
t.Fatalf("selector must be set in opts if cliArgs are not provided")
}
tt.opts.selector.api = apiMock
err = a.Edit(context.Background(), tt.opts)
} else {
cmd := newEditCmd(a)

View file

@ -13,28 +13,28 @@ import (
)
func newJupyterCmd(app *App) *cobra.Command {
var codespace string
var selector *CodespaceSelector
jupyterCmd := &cobra.Command{
Use: "jupyter",
Short: "Open a codespace in JupyterLab",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Jupyter(cmd.Context(), codespace)
return app.Jupyter(cmd.Context(), selector)
},
}
jupyterCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(jupyterCmd, app.apiClient)
return jupyterCmd
}
func (a *App) Jupyter(ctx context.Context, codespaceName string) (err error) {
func (a *App) Jupyter(ctx context.Context, selector *CodespaceSelector) (err error) {
// Ensure all child tasks (e.g. port forwarding) terminate before return.
ctx, cancel := context.WithCancel(ctx)
defer cancel()
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

View file

@ -12,8 +12,8 @@ import (
func newLogsCmd(app *App) *cobra.Command {
var (
codespace string
follow bool
selector *CodespaceSelector
follow bool
)
logsCmd := &cobra.Command{
@ -21,22 +21,23 @@ func newLogsCmd(app *App) *cobra.Command {
Short: "Access codespace logs",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Logs(cmd.Context(), codespace, follow)
return app.Logs(cmd.Context(), selector, follow)
},
}
logsCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(logsCmd, app.apiClient)
logsCmd.Flags().BoolVarP(&follow, "follow", "f", false, "Tail and follow the logs")
return logsCmd
}
func (a *App) Logs(ctx context.Context, codespaceName string, follow bool) (err error) {
func (a *App) Logs(ctx context.Context, selector *CodespaceSelector, follow bool) (err error) {
// Ensure all child tasks (port forwarding, remote exec) terminate before return.
ctx, cancel := context.WithCancel(ctx)
defer cancel()
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

View file

@ -10,8 +10,9 @@ import (
func TestPendingOperationDisallowsLogs(t *testing.T) {
app := testingLogsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.Logs(context.Background(), "disabledCodespace", false); err != nil {
if err := app.Logs(context.Background(), selector, false); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

View file

@ -29,30 +29,33 @@ const (
// newPortsCmd returns a Cobra "ports" command that displays a table of available ports,
// according to the specified flags.
func newPortsCmd(app *App) *cobra.Command {
var codespace string
var exporter cmdutil.Exporter
var (
selector *CodespaceSelector
exporter cmdutil.Exporter
)
portsCmd := &cobra.Command{
Use: "ports",
Short: "List ports in a codespace",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.ListPorts(cmd.Context(), codespace, exporter)
return app.ListPorts(cmd.Context(), selector, exporter)
},
}
portsCmd.PersistentFlags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(portsCmd, app.apiClient)
cmdutil.AddJSONFlags(portsCmd, &exporter, portFields)
portsCmd.AddCommand(newPortsForwardCmd(app))
portsCmd.AddCommand(newPortsVisibilityCmd(app))
portsCmd.AddCommand(newPortsForwardCmd(app, selector))
portsCmd.AddCommand(newPortsVisibilityCmd(app, selector))
return portsCmd
}
// ListPorts lists known ports in a codespace.
func (a *App) ListPorts(ctx context.Context, codespaceName string, exporter cmdutil.Exporter) (err error) {
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
func (a *App) ListPorts(ctx context.Context, selector *CodespaceSelector, exporter cmdutil.Exporter) (err error) {
codespace, err := selector.Select(ctx)
if err != nil {
return err
}
@ -218,21 +221,14 @@ func getDevContainer(ctx context.Context, apiClient apiClient, codespace *api.Co
return ch
}
func newPortsVisibilityCmd(app *App) *cobra.Command {
func newPortsVisibilityCmd(app *App, selector *CodespaceSelector) *cobra.Command {
return &cobra.Command{
Use: "visibility <port>:{public|private|org}...",
Short: "Change the visibility of the forwarded port",
Example: "gh codespace ports visibility 80:org 3000:private 8000: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: %w", err)
}
return app.UpdatePortVisibility(cmd.Context(), codespace, args)
return app.UpdatePortVisibility(cmd.Context(), selector, args)
},
}
}
@ -261,13 +257,13 @@ func (e *ErrUpdatingPortVisibility) Unwrap() error {
var errUpdatePortVisibilityForbidden = errors.New("organization admin has forbidden this privacy setting")
func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName string, args []string) (err error) {
func (a *App) UpdatePortVisibility(ctx context.Context, selector *CodespaceSelector, args []string) (err error) {
ports, err := a.parsePortVisibilities(args)
if err != nil {
return fmt.Errorf("error parsing port arguments: %w", err)
}
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}
@ -347,32 +343,24 @@ func (a *App) parsePortVisibilities(args []string) ([]portVisibility, error) {
// NewPortsForwardCmd returns a Cobra "ports forward" subcommand, which forwards a set of
// port pairs from the codespace to localhost.
func newPortsForwardCmd(app *App) *cobra.Command {
func newPortsForwardCmd(app *App, selector *CodespaceSelector) *cobra.Command {
return &cobra.Command{
Use: "forward <remote-port>:<local-port>...",
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: %w", err)
}
return app.ForwardPorts(cmd.Context(), codespace, args)
return app.ForwardPorts(cmd.Context(), selector, args)
},
}
}
func (a *App) ForwardPorts(ctx context.Context, codespaceName string, ports []string) (err error) {
func (a *App) ForwardPorts(ctx context.Context, selector *CodespaceSelector, ports []string) (err error) {
portPairs, err := getPortPairs(ports)
if err != nil {
return fmt.Errorf("get port pairs: %w", err)
}
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

View file

@ -207,13 +207,16 @@ func runUpdateVisibilityTest(t *testing.T, portVisibilities []portVisibility, ev
portArgs = append(portArgs, fmt.Sprintf("%d:%s", pv.number, pv.visibility))
}
return a.UpdatePortVisibility(ctx, "codespace-name", portArgs)
selector := &CodespaceSelector{api: a.apiClient, codespaceName: "codespace-name"}
return a.UpdatePortVisibility(ctx, selector, portArgs)
}
func TestPendingOperationDisallowsListPorts(t *testing.T) {
app := testingPortsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.ListPorts(context.Background(), "disabledCodespace", nil); err != nil {
if err := app.ListPorts(context.Background(), selector, nil); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}
@ -224,8 +227,9 @@ func TestPendingOperationDisallowsListPorts(t *testing.T) {
func TestPendingOperationDisallowsUpdatePortVisability(t *testing.T) {
app := testingPortsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.UpdatePortVisibility(context.Background(), "disabledCodespace", nil); err != nil {
if err := app.UpdatePortVisibility(context.Background(), selector, nil); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}
@ -236,8 +240,9 @@ func TestPendingOperationDisallowsUpdatePortVisability(t *testing.T) {
func TestPendingOperationDisallowsForwardPorts(t *testing.T) {
app := testingPortsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.ForwardPorts(context.Background(), "disabledCodespace", nil); err != nil {
if err := app.ForwardPorts(context.Background(), selector, nil); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

View file

@ -10,8 +10,10 @@ import (
)
func newRebuildCmd(app *App) *cobra.Command {
var codespace string
var fullRebuild bool
var (
selector *CodespaceSelector
fullRebuild bool
)
rebuildCmd := &cobra.Command{
Use: "rebuild",
@ -21,21 +23,22 @@ preserved. Your codespace will be rebuilt using your working directory's
dev container. A full rebuild also removes cached Docker images.`,
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Rebuild(cmd.Context(), codespace, fullRebuild)
return app.Rebuild(cmd.Context(), selector, fullRebuild)
},
}
rebuildCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "name of the codespace")
selector = AddCodespaceSelector(rebuildCmd, app.apiClient)
rebuildCmd.Flags().BoolVar(&fullRebuild, "full", false, "perform a full rebuild")
return rebuildCmd
}
func (a *App) Rebuild(ctx context.Context, codespaceName string, full bool) (err error) {
func (a *App) Rebuild(ctx context.Context, selector *CodespaceSelector, full bool) (err error) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

View file

@ -14,8 +14,9 @@ func TestAlreadyRebuildingCodespace(t *testing.T) {
State: api.CodespaceStateRebuilding,
}
app := testingRebuildApp(*rebuildingCodespace)
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "rebuildingCodespace"}
err := app.Rebuild(context.Background(), "rebuildingCodespace", false)
err := app.Rebuild(context.Background(), selector, false)
if err != nil {
t.Errorf("rebuilding a codespace that was already rebuilding: %v", err)
}

View file

@ -10,10 +10,13 @@ import (
type selectOptions struct {
filePath string
selector *CodespaceSelector
}
func newSelectCmd(app *App) *cobra.Command {
opts := selectOptions{}
var (
opts selectOptions
)
selectCmd := &cobra.Command{
Use: "select",
@ -21,19 +24,39 @@ func newSelectCmd(app *App) *cobra.Command {
Hidden: true,
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Select(cmd.Context(), "", opts)
return app.Select(cmd.Context(), opts)
},
}
opts.selector = AddCodespaceSelector(selectCmd, app.apiClient)
selectCmd.Flags().StringVarP(&opts.filePath, "file", "f", "", "Output file path")
return selectCmd
}
// Hidden codespace select command allows to reuse existing codespace selection
// dialog by external GH CLI extensions. By default, print selected codespace name
// to stdout. Pass file argument to save result into a file instead.
func (a *App) Select(ctx context.Context, name string, opts selectOptions) (err error) {
codespace, err := getOrChooseCodespace(ctx, a.apiClient, name)
// Hidden codespace `select` command allows to reuse existing codespace selection
// dialog by external GH CLI extensions. By default output selected codespace name
// into `stdout`. Pass `--file`(`-f`) flag along with a file path to output selected
// codespace name into a file instead.
//
// ## Examples
//
// With `stdout` output:
//
// ```shell
//
// gh codespace select
//
// ```
//
// With `into-a-file` output:
//
// ```shell
//
// gh codespace select --file /tmp/selected_codespace.txt
//
// ```
func (a *App) Select(ctx context.Context, opts selectOptions) (err error) {
codespace, err := opts.selector.Select(ctx)
if err != nil {
return err
}

View file

@ -50,6 +50,7 @@ func TestApp_Select(t *testing.T) {
a := NewApp(ios, nil, testSelectApiMock(), nil, nil)
opts := selectOptions{}
if tt.outputToFile {
file, err := os.CreateTemp("", "codespace-selection-test")
if err != nil {
@ -61,7 +62,9 @@ func TestApp_Select(t *testing.T) {
opts = selectOptions{filePath: file.Name()}
}
if err := a.Select(context.Background(), tt.arg, opts); (err != nil) != tt.wantErr {
opts.selector = &CodespaceSelector{api: a.apiClient, codespaceName: tt.arg}
if err := a.Select(context.Background(), opts); (err != nil) != tt.wantErr {
t.Errorf("App.Select() error = %v, wantErr %v", err, tt.wantErr)
}

View file

@ -36,7 +36,7 @@ const automaticPrivateKeyName = "codespaces.auto"
var errKeyFileNotFound = errors.New("SSH key file does not exist")
type sshOptions struct {
codespace string
selector *CodespaceSelector
profile string
serverPort int
debug bool
@ -87,7 +87,7 @@ func newSSHCmd(app *App) *cobra.Command {
`),
PreRunE: func(c *cobra.Command, args []string) error {
if opts.stdio {
if opts.codespace == "" {
if opts.selector.codespaceName == "" {
return errors.New("`--stdio` requires explicit `--codespace`")
}
if opts.config {
@ -122,7 +122,7 @@ func newSSHCmd(app *App) *cobra.Command {
sshCmd.Flags().StringVarP(&opts.profile, "profile", "", "", "Name of the SSH profile to use")
sshCmd.Flags().IntVarP(&opts.serverPort, "server-port", "", 0, "SSH server port number (0 => pick unused)")
sshCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(sshCmd, app.apiClient)
sshCmd.Flags().BoolVarP(&opts.debug, "debug", "d", false, "Log debug data to a file")
sshCmd.Flags().StringVarP(&opts.debugFile, "debug-file", "", "", "Path of the file log to")
sshCmd.Flags().BoolVarP(&opts.config, "config", "", false, "Write OpenSSH configuration to stdout")
@ -160,7 +160,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
args = append([]string{"-i", keyPair.PrivateKeyPath}, args...)
}
codespace, err := getOrChooseCodespace(ctx, a.apiClient, opts.codespace)
codespace, err := opts.selector.Select(ctx)
if err != nil {
return err
}
@ -471,13 +471,13 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
defer cancel()
var csList []*api.Codespace
if opts.codespace == "" {
if opts.selector.codespaceName == "" {
a.StartProgressIndicatorWithLabel("Fetching codespaces")
csList, err = a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{})
a.StopProgressIndicator()
} else {
var codespace *api.Codespace
codespace, err = getOrChooseCodespace(ctx, a.apiClient, opts.codespace)
codespace, err = opts.selector.Select(ctx)
csList = []*api.Codespace{codespace}
}
if err != nil {
@ -494,7 +494,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
var wg sync.WaitGroup
var status error
for _, cs := range csList {
if cs.State != "Available" && opts.codespace == "" {
if cs.State != "Available" && opts.selector.codespaceName == "" {
fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State)
status = cmdutil.SilentError
continue
@ -656,7 +656,7 @@ func newCpCmd(app *App) *cobra.Command {
// We don't expose all sshOptions.
cpCmd.Flags().BoolVarP(&opts.recursive, "recursive", "r", false, "Recursively copy directories")
cpCmd.Flags().BoolVarP(&opts.expand, "expand", "e", false, "Expand remote file names on remote shell")
cpCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(cpCmd, app.apiClient)
cpCmd.Flags().StringVarP(&opts.profile, "profile", "p", "", "Name of the SSH profile to use")
return cpCmd
}

View file

@ -15,8 +15,9 @@ import (
func TestPendingOperationDisallowsSSH(t *testing.T) {
app := testingSSHApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.SSH(context.Background(), []string{}, sshOptions{codespace: "disabledCodespace"}); err != nil {
if err := app.SSH(context.Background(), []string{}, sshOptions{selector: selector}); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

View file

@ -11,9 +11,9 @@ import (
)
type stopOptions struct {
codespaceName string
orgName string
userName string
selector *CodespaceSelector
orgName string
userName string
}
func newStopCmd(app *App) *cobra.Command {
@ -24,13 +24,13 @@ func newStopCmd(app *App) *cobra.Command {
Short: "Stop a running codespace",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
if opts.orgName != "" && opts.codespaceName != "" && opts.userName == "" {
if opts.orgName != "" && opts.selector.codespaceName != "" && opts.userName == "" {
return cmdutil.FlagErrorf("using `--org` with `--codespace` requires `--user`")
}
return app.StopCodespace(cmd.Context(), opts)
},
}
stopCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(stopCmd, app.apiClient)
stopCmd.Flags().StringVarP(&opts.orgName, "org", "o", "", "The `login` handle of the organization (admin-only)")
stopCmd.Flags().StringVarP(&opts.userName, "user", "u", "", "The `username` to stop codespace for (used with --org)")
@ -38,12 +38,19 @@ func newStopCmd(app *App) *cobra.Command {
}
func (a *App) StopCodespace(ctx context.Context, opts *stopOptions) error {
codespaceName := opts.codespaceName
ownerName := opts.userName
var (
codespaceName = opts.selector.codespaceName
repoName = opts.selector.repoName
ownerName = opts.userName
)
if codespaceName == "" {
a.StartProgressIndicatorWithLabel("Fetching codespaces")
codespaces, err := a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{OrgName: opts.orgName, UserName: ownerName})
codespaces, err := a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{
RepoName: repoName,
OrgName: opts.orgName,
UserName: ownerName,
})
a.StopProgressIndicator()
if err != nil {
return fmt.Errorf("failed to list codespaces: %w", err)
@ -61,7 +68,8 @@ func (a *App) StopCodespace(ctx context.Context, opts *stopOptions) error {
}
includeOwner := opts.orgName != ""
codespace, err := chooseCodespaceFromList(ctx, runningCodespaces, includeOwner)
skipPromptForSingleOption := repoName != ""
codespace, err := chooseCodespaceFromList(ctx, runningCodespaces, includeOwner, skipPromptForSingleOption)
if err != nil {
return fmt.Errorf("failed to choose codespace: %w", err)
}

View file

@ -22,7 +22,7 @@ func TestApp_StopCodespace(t *testing.T) {
{
name: "Stop a codespace I own",
opts: &stopOptions{
codespaceName: "test-codespace",
selector: &CodespaceSelector{codespaceName: "test-codespace"},
},
fields: fields{
apiClient: &apiClientMock{
@ -52,9 +52,9 @@ func TestApp_StopCodespace(t *testing.T) {
{
name: "Stop a codespace as an org admin",
opts: &stopOptions{
codespaceName: "test-codespace",
orgName: "test-org",
userName: "test-user",
selector: &CodespaceSelector{codespaceName: "test-codespace"},
orgName: "test-org",
userName: "test-user",
},
fields: fields{
apiClient: &apiClientMock{