From 9554e522af8e06faeac42c793a96bb96d7bc9eb2 Mon Sep 17 00:00:00 2001 From: Jeff Hubbard Date: Mon, 28 Mar 2022 14:24:57 -0700 Subject: [PATCH] Change the way we parse list-devcontainers response --- internal/codespaces/api/api.go | 10 +++++++-- pkg/cmd/codespace/common.go | 2 +- pkg/cmd/codespace/create.go | 36 ++++++++++++++++++++++---------- pkg/cmd/codespace/create_test.go | 10 ++++----- pkg/cmd/codespace/mock_api.go | 6 +++--- testdevcontainer | 6 ------ utils/utils.go | 9 ++++++++ 7 files changed, 51 insertions(+), 28 deletions(-) delete mode 100644 testdevcontainer diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index d0e61a0b1..0765525ec 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -762,9 +762,14 @@ func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error { return nil } +type DevContainerEntry struct { + Path string `json:"path"` + Name string `json:"name,omitempty"` +} + // ListDevContainers returns a list of valid devcontainer.json files for the repo. Pass a negative limit to request all pages from // the API until all devcontainer.json files have been fetched. -func (a *API) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []string, err error) { +func (a *API) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []DevContainerEntry, err error) { perPage := 100 if limit > 0 && limit < 100 { perPage = limit @@ -792,8 +797,9 @@ func (a *API) ListDevContainers(ctx context.Context, repoID int, branch string, } var response struct { - Devcontainers []string `json:"devcontainers"` + Devcontainers []DevContainerEntry `json:"devcontainers"` } + dec := json.NewDecoder(resp.Body) if err := dec.Decode(&response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index d9c941cf7..1bd04ca56 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -74,7 +74,7 @@ type apiClient interface { GetCodespaceRegionLocation(ctx context.Context) (string, error) GetCodespacesMachines(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) - ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []string, err error) + ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []api.DevContainerEntry, err error) GetCodespaceRepoSuggestions(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) } diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index d95bfba1e..1a5560b8d 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -58,6 +58,8 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { locationCh := getLocation(ctx, vscsLocation, a.apiClient) + DEFAULT_DEVCONTAINER_DEFINITIONS := []string{".devcontainer.json", ".devcontainer/devcontainer.json"} + userInputs := struct { Repository string Branch string @@ -113,23 +115,35 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { // now that we have repo+branch, we can list available devcontainer.json files (if any) if len(opts.devContainerPath) < 1 { a.StartProgressIndicatorWithLabel("Fetching devcontainer.json files") - devContainerPaths, err := a.apiClient.ListDevContainers(ctx, repository.ID, branch, 100) + devcontainers, err := a.apiClient.ListDevContainers(ctx, repository.ID, branch, 100) if err != nil { return fmt.Errorf("error getting devcontainer.json paths: %w", err) } a.StopProgressIndicator() - if len(devContainerPaths) > 0 { - devContainerPathQuestion := &survey.Question{ - Name: "devContainerPath", - Prompt: &survey.Select{ - Message: "Devcontainer definition file:", - Options: append([]string{"default"}, devContainerPaths...), - }, - } + if len(devcontainers) > 0 { - if err := ask([]*survey.Question{devContainerPathQuestion}, &devContainerPath); err != nil { - return fmt.Errorf("failed to prompt: %w", err) + // if there is only one devcontainer.json file and it is one of the default paths we can auto-select it + if len(devcontainers) == 1 && utils.StringInSlice(devcontainers[0].Path, DEFAULT_DEVCONTAINER_DEFINITIONS) { + devContainerPath = devcontainers[0].Path + } else { + promptOptions := []string{"default"} + + for _, devcontainer := range devcontainers { + promptOptions = append(promptOptions, devcontainer.Path) + } + + devContainerPathQuestion := &survey.Question{ + Name: "devContainerPath", + Prompt: &survey.Select{ + Message: "Devcontainer definition file:", + Options: promptOptions, + }, + } + + if err := ask([]*survey.Question{devContainerPathQuestion}, &devContainerPath); err != nil { + return fmt.Errorf("failed to prompt: %w", err) + } } } diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index eff850dda..c1b2174f5 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -86,8 +86,8 @@ func TestApp_Create(t *testing.T) { DefaultBranch: "main", }, nil }, - ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]string, error) { - return []string{}, nil + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + return []api.DevContainerEntry{}, nil }, GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) { return []*api.Machine{ @@ -139,7 +139,7 @@ func TestApp_Create(t *testing.T) { DefaultBranch: "main", }, nil }, - ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]string, error) { + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { return nil, fmt.Errorf("some error") }, }, @@ -167,8 +167,8 @@ func TestApp_Create(t *testing.T) { DefaultBranch: "main", }, nil }, - ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]string, error) { - return []string{}, nil + ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + return []api.DevContainerEntry{}, nil }, GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) { return []*api.Machine{ diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index bccb90566..897b39f1d 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -52,7 +52,7 @@ import ( // ListCodespacesFunc: func(ctx context.Context, limit int) ([]*api.Codespace, error) { // panic("mock out the ListCodespaces method") // }, -// ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]string, error) { +// ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { // panic("mock out the ListDevContainers method") // }, // StartCodespaceFunc: func(ctx context.Context, name string) error { @@ -105,7 +105,7 @@ type apiClientMock struct { ListCodespacesFunc func(ctx context.Context, limit int) ([]*api.Codespace, error) // ListDevContainersFunc mocks the ListDevContainers method. - ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]string, error) + ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) // StartCodespaceFunc mocks the StartCodespace method. StartCodespaceFunc func(ctx context.Context, name string) error @@ -687,7 +687,7 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { } // ListDevContainers calls ListDevContainersFunc. -func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) ([]string, error) { +func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { if mock.ListDevContainersFunc == nil { panic("apiClientMock.ListDevContainersFunc: method is nil but apiClient.ListDevContainers was just called") } diff --git a/testdevcontainer b/testdevcontainer deleted file mode 100644 index 1168d0af0..000000000 --- a/testdevcontainer +++ /dev/null @@ -1,6 +0,0 @@ -{ - "repository_id": 392831291, - "location": "WestUs2", - "devcontainer_path": ".devcontainer/foo ' \" bar/devcontainer.json", - "vscs_target": "development" -} diff --git a/utils/utils.go b/utils/utils.go index cd423bf65..18ae33143 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -83,3 +83,12 @@ func DisplayURL(urlStr string) string { func ValidURL(urlStr string) bool { return len(urlStr) < 8192 } + +func StringInSlice(a string, slice []string) bool { + for _, b := range slice { + if b == a { + return true + } + } + return false +}