From 975bd7c08a459c2f26ffd6dc2998c05793710f2a Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Tue, 5 Oct 2021 11:23:12 -0400 Subject: [PATCH 1/8] Change choose codespace prompt - Repo + branch is favored - Codespace name is included to disambiguate between two codespaces - Move Codespace model out of out API into its own package - Update call sites and group behavior under codespace.Codespace --- internal/codespaces/api/api.go | 60 +++----------- internal/codespaces/api/api_test.go | 6 +- internal/codespaces/codespace/codespace.go | 76 ++++++++++++++++++ internal/codespaces/codespaces.go | 38 ++++----- internal/codespaces/states.go | 11 +-- pkg/cmd/codespace/common.go | 61 +++++++++++---- pkg/cmd/codespace/create.go | 5 +- pkg/cmd/codespace/delete.go | 20 +++-- pkg/cmd/codespace/delete_test.go | 29 +++---- pkg/cmd/codespace/list.go | 11 +-- pkg/cmd/codespace/mock_api.go | 91 +++++++++++----------- pkg/cmd/codespace/ports.go | 4 +- 12 files changed, 237 insertions(+), 175 deletions(-) create mode 100644 internal/codespaces/codespace/codespace.go diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 7ab3867aa..42caa02ee 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -38,6 +38,7 @@ import ( "strings" "time" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/opentracing/opentracing-go" ) @@ -140,46 +141,7 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error return &response, nil } -type Codespace struct { - Name string `json:"name"` - GUID string `json:"guid"` - CreatedAt string `json:"created_at"` - LastUsedAt string `json:"last_used_at"` - Branch string `json:"branch"` - RepositoryName string `json:"repository_name"` - RepositoryNWO string `json:"repository_nwo"` - OwnerLogin string `json:"owner_login"` - Environment CodespaceEnvironment `json:"environment"` -} - -type CodespaceEnvironment struct { - State string `json:"state"` - Connection CodespaceEnvironmentConnection `json:"connection"` - GitStatus CodespaceEnvironmentGitStatus `json:"gitStatus"` -} - -type CodespaceEnvironmentGitStatus struct { - Ahead int `json:"ahead"` - Behind int `json:"behind"` - Branch string `json:"branch"` - Commit string `json:"commit"` - HasUnpushedChanges bool `json:"hasUnpushedChanges"` - HasUncommitedChanges bool `json:"hasUncommitedChanges"` -} - -const ( - CodespaceEnvironmentStateAvailable = "Available" -) - -type CodespaceEnvironmentConnection struct { - SessionID string `json:"sessionId"` - SessionToken string `json:"sessionToken"` - RelayEndpoint string `json:"relayEndpoint"` - RelaySAS string `json:"relaySas"` - HostPublicKeys []string `json:"hostPublicKeys"` -} - -func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { +func (a *API) ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error) { req, err := http.NewRequest( http.MethodGet, a.githubAPI+"/user/codespaces", nil, ) @@ -204,7 +166,7 @@ func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { } var response struct { - Codespaces []*Codespace `json:"codespaces"` + Codespaces []*codespace.Codespace `json:"codespaces"` } if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) @@ -267,10 +229,10 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s return response.RepositoryToken, nil } -func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) (*Codespace, error) { +func (a *API) GetCodespace(ctx context.Context, token, owner, codespaceName string) (*codespace.Codespace, error) { req, err := http.NewRequest( http.MethodGet, - a.githubAPI+"/vscs_internal/user/"+owner+"/codespaces/"+codespace, + a.githubAPI+"/vscs_internal/user/"+owner+"/codespaces/"+codespaceName, nil, ) if err != nil { @@ -294,7 +256,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) return nil, jsonErrorResponse(b) } - var response Codespace + var response codespace.Codespace if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) } @@ -302,7 +264,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) return &response, nil } -func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codespace) error { +func (a *API) StartCodespace(ctx context.Context, token string, codespace *codespace.Codespace) error { req, err := http.NewRequest( http.MethodPost, a.githubAPI+"/vscs_internal/proxy/environments/"+codespace.GUID+"/start", @@ -429,7 +391,7 @@ type CreateCodespaceParams struct { // CreateCodespace creates a codespace with the given parameters and returns a non-nil error if it // fails to create. -func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams) (*Codespace, error) { +func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams) (*codespace.Codespace, error) { codespace, err := a.startCreate(ctx, params.RepositoryID, params.Machine, params.Branch, params.Location) if err != errProvisioningInProgress { return codespace, err @@ -482,7 +444,7 @@ var errProvisioningInProgress = errors.New("provisioning in progress") // It may return success or an error, or errProvisioningInProgress indicating that the operation // did not complete before the GitHub API's time limit for RPCs (10s), in which case the caller // must poll the server to learn the outcome. -func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, location string) (*Codespace, error) { +func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, location string) (*codespace.Codespace, error) { requestBody, err := json.Marshal(startCreateRequest{repoID, branch, location, machine}) if err != nil { return nil, fmt.Errorf("error marshaling request: %w", err) @@ -512,7 +474,7 @@ func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, loca return nil, errProvisioningInProgress // RPC finished before result of creation known } - var response Codespace + var response codespace.Codespace if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) } @@ -554,7 +516,7 @@ type getCodespaceRepositoryContentsResponse struct { Content string `json:"content"` } -func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Codespace, path string) ([]byte, error) { +func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *codespace.Codespace, path string) ([]byte, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+codespace.RepositoryNWO+"/contents/"+path, nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) diff --git a/internal/codespaces/api/api_test.go b/internal/codespaces/api/api_test.go index 8bbe1c8a9..8ca4b5d5f 100644 --- a/internal/codespaces/api/api_test.go +++ b/internal/codespaces/api/api_test.go @@ -7,10 +7,12 @@ import ( "net/http" "net/http/httptest" "testing" + + "github.com/cli/cli/v2/internal/codespaces/codespace" ) func TestListCodespaces(t *testing.T) { - codespaces := []*Codespace{ + codespaces := []*codespace.Codespace{ { Name: "testcodespace", CreatedAt: "2021-08-09T10:10:24+02:00", @@ -19,7 +21,7 @@ func TestListCodespaces(t *testing.T) { } svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { response := struct { - Codespaces []*Codespace `json:"codespaces"` + Codespaces []*codespace.Codespace `json:"codespaces"` }{ Codespaces: codespaces, } diff --git a/internal/codespaces/codespace/codespace.go b/internal/codespaces/codespace/codespace.go new file mode 100644 index 000000000..69188ea4b --- /dev/null +++ b/internal/codespaces/codespace/codespace.go @@ -0,0 +1,76 @@ +package codespace + +import "fmt" + +type Codespace struct { + Name string `json:"name"` + CreatedAt string `json:"created_at"` + LastUsedAt string `json:"last_used_at"` + GUID string `json:"guid"` + Branch string `json:"branch"` + RepositoryName string `json:"repository_name"` + RepositoryNWO string `json:"repository_nwo"` + OwnerLogin string `json:"owner_login"` + Environment Environment `json:"environment"` +} + +// DisplayName returns the repository nwo and branch. +// If includeName is true, the name of the codespace is included. +// If includeGitStatus is true, the branch will include a star if +// the codespace has unsaved changes. +func (c *Codespace) DisplayName(includeName, includeGitStatus bool) string { + branch := c.Branch + if includeGitStatus { + branch = c.BranchWithGitStatus() + } + + if includeName { + return fmt.Sprintf( + "%s: %s [%s]", c.RepositoryNWO, branch, c.Name, + ) + } + return c.RepositoryNWO + ": " + branch +} + +// BranchWithGitStatus returns the branch with a star +// if the branch is currently being worked on. +func (c *Codespace) BranchWithGitStatus() string { + if c.HasUnsavedChanges() { + return c.Branch + "*" + } + + return c.Branch +} + +// HasUnsavedChanges returns whether the environment has +// unsaved changes. +func (c *Codespace) HasUnsavedChanges() bool { + return c.Environment.GitStatus.HasUncommitedChanges || c.Environment.GitStatus.HasUnpushedChanges +} + +type Environment struct { + State string `json:"state"` + Connection EnvironmentConnection `json:"connection"` + GitStatus EnvironmentGitStatus `json:"gitStatus"` +} + +type EnvironmentGitStatus struct { + Ahead int `json:"ahead"` + Behind int `json:"behind"` + Branch string `json:"branch"` + Commit string `json:"commit"` + HasUnpushedChanges bool `json:"hasUnpushedChanges"` + HasUncommitedChanges bool `json:"hasUncommitedChanges"` +} + +const ( + EnvironmentStateAvailable = "Available" +) + +type EnvironmentConnection struct { + SessionID string `json:"sessionId"` + SessionToken string `json:"sessionToken"` + RelayEndpoint string `json:"relayEndpoint"` + RelaySAS string `json:"relaySas"` + HostPublicKeys []string `json:"hostPublicKeys"` +} diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 654504205..0d3702d2f 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -6,7 +6,7 @@ import ( "fmt" "time" - "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/liveshare" ) @@ -15,33 +15,33 @@ type logger interface { Println(v ...interface{}) (int, error) } -func connectionReady(codespace *api.Codespace) bool { - return codespace.Environment.Connection.SessionID != "" && - codespace.Environment.Connection.SessionToken != "" && - codespace.Environment.Connection.RelayEndpoint != "" && - codespace.Environment.Connection.RelaySAS != "" && - codespace.Environment.State == api.CodespaceEnvironmentStateAvailable +func connectionReady(cs *codespace.Codespace) bool { + return cs.Environment.Connection.SessionID != "" && + cs.Environment.Connection.SessionToken != "" && + cs.Environment.Connection.RelayEndpoint != "" && + cs.Environment.Connection.RelaySAS != "" && + cs.Environment.State == codespace.EnvironmentStateAvailable } type apiClient interface { - GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) + GetCodespace(ctx context.Context, token, user, name string) (*codespace.Codespace, error) GetCodespaceToken(ctx context.Context, user, codespace string) (string, error) - StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespace(ctx context.Context, token string, codespace *codespace.Codespace) error } // ConnectToLiveshare waits for a Codespace to become running, // and connects to it using a Live Share session. -func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, userLogin, token string, codespace *api.Codespace) (*liveshare.Session, error) { +func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, userLogin, token string, cs *codespace.Codespace) (*liveshare.Session, error) { var startedCodespace bool - if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { + if cs.Environment.State != codespace.EnvironmentStateAvailable { startedCodespace = true log.Print("Starting your codespace...") - if err := apiClient.StartCodespace(ctx, token, codespace); err != nil { + if err := apiClient.StartCodespace(ctx, token, cs); err != nil { return nil, fmt.Errorf("error starting codespace: %w", err) } } - for retries := 0; !connectionReady(codespace); retries++ { + for retries := 0; !connectionReady(cs); retries++ { if retries > 1 { if retries%2 == 0 { log.Print(".") @@ -55,7 +55,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, us } var err error - codespace, err = apiClient.GetCodespace(ctx, token, userLogin, codespace.Name) + cs, err = apiClient.GetCodespace(ctx, token, userLogin, cs.Name) if err != nil { return nil, fmt.Errorf("error getting codespace: %w", err) } @@ -68,10 +68,10 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, us log.Println("Connecting to your codespace...") return liveshare.Connect(ctx, liveshare.Options{ - SessionID: codespace.Environment.Connection.SessionID, - SessionToken: codespace.Environment.Connection.SessionToken, - RelaySAS: codespace.Environment.Connection.RelaySAS, - RelayEndpoint: codespace.Environment.Connection.RelayEndpoint, - HostPublicKeys: codespace.Environment.Connection.HostPublicKeys, + SessionID: cs.Environment.Connection.SessionID, + SessionToken: cs.Environment.Connection.SessionToken, + RelaySAS: cs.Environment.Connection.RelaySAS, + RelayEndpoint: cs.Environment.Connection.RelayEndpoint, + HostPublicKeys: cs.Environment.Connection.HostPublicKeys, }) } diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index b9d12a796..e37b625e3 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -10,6 +10,7 @@ import ( "time" "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/liveshare" ) @@ -36,13 +37,13 @@ type PostCreateState struct { // PollPostCreateStates watches for state changes in a codespace, // and calls the supplied poller for each batch of state changes. // It runs until it encounters an error, including cancellation of the context. -func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, user *api.User, codespace *api.Codespace, poller func([]PostCreateState)) (err error) { - token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespace.Name) +func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, user *api.User, c *codespace.Codespace, poller func([]PostCreateState)) (err error) { + token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) if err != nil { return fmt.Errorf("getting codespace token: %w", err) } - session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) + session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, c) if err != nil { return fmt.Errorf("connect to Live Share: %w", err) } @@ -83,7 +84,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, return fmt.Errorf("connection failed: %w", err) case <-t.C: - states, err := getPostCreateOutput(ctx, localPort, codespace, sshUser) + states, err := getPostCreateOutput(ctx, localPort, sshUser) if err != nil { return fmt.Errorf("get post create output: %w", err) } @@ -93,7 +94,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, } } -func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace, user string) ([]PostCreateState, error) { +func getPostCreateOutput(ctx context.Context, tunnelPort int, user string) ([]PostCreateState, error) { cmd, err := NewRemoteCommand( ctx, tunnelPort, fmt.Sprintf("%s@localhost", user), "cat /workspaces/.codespaces/shared/postCreateOutput.json", diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index ecce47b2a..20142c030 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -9,10 +9,12 @@ import ( "io" "os" "sort" + "strings" "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/terminal" "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/cmd/codespace/output" "github.com/spf13/cobra" "golang.org/x/term" @@ -34,21 +36,21 @@ func NewApp(logger *output.Logger, apiClient apiClient) *App { type apiClient interface { GetUser(ctx context.Context) (*api.User, error) GetCodespaceToken(ctx context.Context, user, name string) (string, error) - GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) - ListCodespaces(ctx context.Context) ([]*api.Codespace, error) + GetCodespace(ctx context.Context, token, user, name string) (*codespace.Codespace, error) + ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error) DeleteCodespace(ctx context.Context, user, name string) error - StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error - CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) + StartCodespace(ctx context.Context, token string, codespace *codespace.Codespace) error + CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) GetCodespaceRegionLocation(ctx context.Context) (string, error) GetCodespacesSKUs(ctx context.Context, user *api.User, repository *api.Repository, branch, location string) ([]*api.SKU, error) - GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) + GetCodespaceRepositoryContents(ctx context.Context, codespace *codespace.Codespace, path string) ([]byte, error) } var errNoCodespaces = errors.New("you have no codespaces") -func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, error) { +func chooseCodespace(ctx context.Context, apiClient apiClient) (*codespace.Codespace, error) { codespaces, err := apiClient.ListCodespaces(ctx) if err != nil { return nil, fmt.Errorf("error getting codespaces: %w", err) @@ -56,7 +58,9 @@ func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, return chooseCodespaceFromList(ctx, codespaces) } -func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) (*api.Codespace, error) { +// chooseCodespaceFromList returns the selected codespace from the list, +// or an error if there are no codespaces. +func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codespace) (*codespace.Codespace, error) { if len(codespaces) == 0 { return nil, errNoCodespaces } @@ -65,14 +69,38 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) ( return codespaces[i].CreatedAt > codespaces[j].CreatedAt }) - codespacesByName := make(map[string]*api.Codespace) - codespacesNames := make([]string, 0, len(codespaces)) - for _, codespace := range codespaces { - codespacesByName[codespace.Name] = codespace - codespacesNames = append(codespacesNames, codespace.Name) + type codespaceWithIndex struct { + cs *codespace.Codespace + idx int } - sshSurvey := []*survey.Question{ + codespacesByName := make(map[string]codespaceWithIndex) + codespacesNames := make([]string, 0, len(codespaces)) + for _, cs := range codespaces { + csName := cs.DisplayName(false, false) + displayNameWithGitStatus := cs.DisplayName(false, true) + + if seenCodespace, ok := codespacesByName[csName]; ok { + // there is an existing codespace on the repo and branch + // we need to disambiguate by adding the codespace name + // to the existing entry and the one we are adding now + fullDisplayName := seenCodespace.cs.DisplayName(true, false) + fullDisplayNameWithGitStatus := seenCodespace.cs.DisplayName(true, true) + + codespacesByName[fullDisplayName] = codespaceWithIndex{seenCodespace.cs, seenCodespace.idx} + codespacesNames[seenCodespace.idx] = fullDisplayNameWithGitStatus + delete(codespacesByName, csName) // delete the existing map entry with old name + + // update this codespace names to include the name to disambiguate + csName = cs.DisplayName(true, false) + displayNameWithGitStatus = cs.DisplayName(true, true) + } + + codespacesByName[csName] = codespaceWithIndex{cs, len(codespacesNames)} + codespacesNames = append(codespacesNames, displayNameWithGitStatus) + } + + csSurvey := []*survey.Question{ { Name: "codespace", Prompt: &survey.Select{ @@ -87,17 +115,18 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) ( var answers struct { Codespace string } - if err := ask(sshSurvey, &answers); err != nil { + if err := ask(csSurvey, &answers); err != nil { return nil, fmt.Errorf("error getting answers: %w", err) } - codespace := codespacesByName[answers.Codespace] + selectedCodespace := strings.Replace(answers.Codespace, "*", "", -1) + codespace := codespacesByName[selectedCodespace].cs return codespace, nil } // 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 apiClient, user *api.User, codespaceName string) (codespace *api.Codespace, token string, err error) { +func getOrChooseCodespace(ctx context.Context, apiClient apiClient, user *api.User, codespaceName string) (codespace *codespace.Codespace, token string, err error) { if codespaceName == "" { codespace, err = chooseCodespace(ctx, apiClient) if err != nil { diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 68f6d5a86..c0648f4f0 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -10,6 +10,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/internal/codespaces" "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/cmd/codespace/output" "github.com/fatih/camelcase" "github.com/spf13/cobra" @@ -108,7 +109,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { // showStatus polls the codespace for a list of post create states and their status. It will keep polling // until all states have finished. Once all states have finished, we poll once more to check if any new // states have been introduced and stop polling otherwise. -func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, user *api.User, codespace *api.Codespace) error { +func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, user *api.User, cs *codespace.Codespace) error { var lastState codespaces.PostCreateState var breakNextState bool @@ -157,7 +158,7 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, us } } - err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller) + err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, cs, poller) if err != nil { if errors.Is(err, context.Canceled) && breakNextState { return nil // we cancelled the context to stop polling, we can ignore the error diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index 8ea821dc9..a51f26d6b 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -8,7 +8,7 @@ import ( "time" "github.com/AlecAivazis/survey/v2" - "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" ) @@ -64,7 +64,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) error { return fmt.Errorf("error getting user: %w", err) } - var codespaces []*api.Codespace + var codespaces []*codespace.Codespace nameFilter := opts.codespaceName if nameFilter == "" { codespaces, err = a.apiClient.ListCodespaces(ctx) @@ -86,15 +86,15 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) error { return fmt.Errorf("error getting codespace token: %w", err) } - codespace, err := a.apiClient.GetCodespace(ctx, token, user.Login, nameFilter) + cs, err := a.apiClient.GetCodespace(ctx, token, user.Login, nameFilter) if err != nil { return fmt.Errorf("error fetching codespace information: %w", err) } - codespaces = []*api.Codespace{codespace} + codespaces = []*codespace.Codespace{cs} } - codespacesToDelete := make([]*api.Codespace, 0, len(codespaces)) + codespacesToDelete := make([]*codespace.Codespace, 0, len(codespaces)) lastUpdatedCutoffTime := opts.now().AddDate(0, 0, -int(opts.keepDays)) for _, c := range codespaces { if nameFilter != "" && c.Name != nameFilter { @@ -146,16 +146,14 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) error { return nil } -func confirmDeletion(p prompter, codespace *api.Codespace, isInteractive bool) (bool, error) { - gs := codespace.Environment.GitStatus - hasUnsavedChanges := gs.HasUncommitedChanges || gs.HasUnpushedChanges - if !hasUnsavedChanges { +func confirmDeletion(p prompter, cs *codespace.Codespace, isInteractive bool) (bool, error) { + if !cs.HasUnsavedChanges() { return true, nil } if !isInteractive { - return false, fmt.Errorf("codespace %s has unsaved changes (use --force to override)", codespace.Name) + return false, fmt.Errorf("codespace %s has unsaved changes (use --force to override)", cs.Name) } - return p.Confirm(fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", codespace.Name)) + return p.Confirm(fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", cs.Name)) } type surveyPrompter struct{} diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index bf16f82f0..dd17d3f40 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -12,6 +12,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/cmd/codespace/output" ) @@ -25,7 +26,7 @@ func TestDelete(t *testing.T) { tests := []struct { name string opts deleteOptions - codespaces []*api.Codespace + codespaces []*codespace.Codespace confirms map[string]bool deleteErr error wantErr bool @@ -38,7 +39,7 @@ func TestDelete(t *testing.T) { opts: deleteOptions{ codespaceName: "hubot-robawt-abc", }, - codespaces: []*api.Codespace{ + codespaces: []*codespace.Codespace{ { Name: "hubot-robawt-abc", }, @@ -50,7 +51,7 @@ func TestDelete(t *testing.T) { opts: deleteOptions{ repoFilter: "monalisa/spoon-knife", }, - codespaces: []*api.Codespace{ + codespaces: []*codespace.Codespace{ { Name: "monalisa-spoonknife-123", RepositoryNWO: "monalisa/Spoon-Knife", @@ -72,7 +73,7 @@ func TestDelete(t *testing.T) { deleteAll: true, keepDays: 3, }, - codespaces: []*api.Codespace{ + codespaces: []*codespace.Codespace{ { Name: "monalisa-spoonknife-123", LastUsedAt: daysAgo(1), @@ -93,7 +94,7 @@ func TestDelete(t *testing.T) { opts: deleteOptions{ deleteAll: true, }, - codespaces: []*api.Codespace{ + codespaces: []*codespace.Codespace{ { Name: "monalisa-spoonknife-123", }, @@ -116,27 +117,27 @@ func TestDelete(t *testing.T) { deleteAll: true, skipConfirm: false, }, - codespaces: []*api.Codespace{ + codespaces: []*codespace.Codespace{ { Name: "monalisa-spoonknife-123", - Environment: api.CodespaceEnvironment{ - GitStatus: api.CodespaceEnvironmentGitStatus{ + Environment: codespace.Environment{ + GitStatus: codespace.EnvironmentGitStatus{ HasUnpushedChanges: true, }, }, }, { Name: "hubot-robawt-abc", - Environment: api.CodespaceEnvironment{ - GitStatus: api.CodespaceEnvironmentGitStatus{ + Environment: codespace.Environment{ + GitStatus: codespace.EnvironmentGitStatus{ HasUncommitedChanges: true, }, }, }, { Name: "monalisa-spoonknife-c4f3", - Environment: api.CodespaceEnvironment{ - GitStatus: api.CodespaceEnvironmentGitStatus{ + Environment: codespace.Environment{ + GitStatus: codespace.EnvironmentGitStatus{ HasUnpushedChanges: false, HasUncommitedChanges: false, }, @@ -167,7 +168,7 @@ func TestDelete(t *testing.T) { }, } if tt.opts.codespaceName == "" { - apiMock.ListCodespacesFunc = func(_ context.Context) ([]*api.Codespace, error) { + apiMock.ListCodespacesFunc = func(_ context.Context) ([]*codespace.Codespace, error) { return tt.codespaces, nil } } else { @@ -177,7 +178,7 @@ func TestDelete(t *testing.T) { } return "CS_TOKEN", nil } - apiMock.GetCodespaceFunc = func(_ context.Context, token, userLogin, name string) (*api.Codespace, error) { + apiMock.GetCodespaceFunc = func(_ context.Context, token, userLogin, name string) (*codespace.Codespace, error) { if userLogin != user.Login { return nil, fmt.Errorf("unexpected user %q", userLogin) } diff --git a/pkg/cmd/codespace/list.go b/pkg/cmd/codespace/list.go index a6a4b9a1b..48b9eb2b1 100644 --- a/pkg/cmd/codespace/list.go +++ b/pkg/cmd/codespace/list.go @@ -5,7 +5,6 @@ import ( "fmt" "os" - "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/pkg/cmd/codespace/output" "github.com/spf13/cobra" ) @@ -39,7 +38,7 @@ func (a *App) List(ctx context.Context, asJSON bool) error { table.Append([]string{ codespace.Name, codespace.RepositoryNWO, - codespace.Branch + dirtyStar(codespace.Environment.GitStatus), + codespace.BranchWithGitStatus(), codespace.Environment.State, codespace.CreatedAt, }) @@ -48,11 +47,3 @@ func (a *App) List(ctx context.Context, asJSON bool) error { table.Render() return nil } - -func dirtyStar(status api.CodespaceEnvironmentGitStatus) string { - if status.HasUncommitedChanges || status.HasUnpushedChanges { - return "*" - } - - return "" -} diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 669083a32..51da96f7a 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -8,6 +8,7 @@ import ( "sync" "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" ) // apiClientMock is a mock implementation of apiClient. @@ -19,19 +20,19 @@ import ( // AuthorizedKeysFunc: func(ctx context.Context, user string) ([]byte, error) { // panic("mock out the AuthorizedKeys method") // }, -// CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { +// CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) { // panic("mock out the CreateCodespace method") // }, // DeleteCodespaceFunc: func(ctx context.Context, user string, name string) error { // panic("mock out the DeleteCodespace method") // }, -// GetCodespaceFunc: func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) { +// GetCodespaceFunc: func(ctx context.Context, token string, user string, name string) (*codespace.Codespace, error) { // panic("mock out the GetCodespace method") // }, // GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { // panic("mock out the GetCodespaceRegionLocation method") // }, -// GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) { +// GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespaceMoqParam *codespace.Codespace, path string) ([]byte, error) { // panic("mock out the GetCodespaceRepositoryContents method") // }, // GetCodespaceTokenFunc: func(ctx context.Context, user string, name string) (string, error) { @@ -46,10 +47,10 @@ import ( // GetUserFunc: func(ctx context.Context) (*api.User, error) { // panic("mock out the GetUser method") // }, -// ListCodespacesFunc: func(ctx context.Context) ([]*api.Codespace, error) { +// ListCodespacesFunc: func(ctx context.Context) ([]*codespace.Codespace, error) { // panic("mock out the ListCodespaces method") // }, -// StartCodespaceFunc: func(ctx context.Context, token string, codespace *api.Codespace) error { +// StartCodespaceFunc: func(ctx context.Context, token string, codespaceMoqParam *codespace.Codespace) error { // panic("mock out the StartCodespace method") // }, // } @@ -63,19 +64,19 @@ type apiClientMock struct { AuthorizedKeysFunc func(ctx context.Context, user string) ([]byte, error) // CreateCodespaceFunc mocks the CreateCodespace method. - CreateCodespaceFunc func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) + CreateCodespaceFunc func(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) // DeleteCodespaceFunc mocks the DeleteCodespace method. DeleteCodespaceFunc func(ctx context.Context, user string, name string) error // GetCodespaceFunc mocks the GetCodespace method. - GetCodespaceFunc func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) + GetCodespaceFunc func(ctx context.Context, token string, user string, name string) (*codespace.Codespace, error) // GetCodespaceRegionLocationFunc mocks the GetCodespaceRegionLocation method. GetCodespaceRegionLocationFunc func(ctx context.Context) (string, error) // GetCodespaceRepositoryContentsFunc mocks the GetCodespaceRepositoryContents method. - GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) + GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespaceMoqParam *codespace.Codespace, path string) ([]byte, error) // GetCodespaceTokenFunc mocks the GetCodespaceToken method. GetCodespaceTokenFunc func(ctx context.Context, user string, name string) (string, error) @@ -90,10 +91,10 @@ type apiClientMock struct { GetUserFunc func(ctx context.Context) (*api.User, error) // ListCodespacesFunc mocks the ListCodespaces method. - ListCodespacesFunc func(ctx context.Context) ([]*api.Codespace, error) + ListCodespacesFunc func(ctx context.Context) ([]*codespace.Codespace, error) // StartCodespaceFunc mocks the StartCodespace method. - StartCodespaceFunc func(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespaceFunc func(ctx context.Context, token string, codespaceMoqParam *codespace.Codespace) error // calls tracks calls to the methods. calls struct { @@ -140,8 +141,8 @@ type apiClientMock struct { GetCodespaceRepositoryContents []struct { // Ctx is the ctx argument value. Ctx context.Context - // Codespace is the codespace argument value. - Codespace *api.Codespace + // CodespaceMoqParam is the codespaceMoqParam argument value. + CodespaceMoqParam *codespace.Codespace // Path is the path argument value. Path string } @@ -190,8 +191,8 @@ type apiClientMock struct { Ctx context.Context // Token is the token argument value. Token string - // Codespace is the codespace argument value. - Codespace *api.Codespace + // CodespaceMoqParam is the codespaceMoqParam argument value. + CodespaceMoqParam *codespace.Codespace } } lockAuthorizedKeys sync.RWMutex @@ -244,7 +245,7 @@ func (mock *apiClientMock) AuthorizedKeysCalls() []struct { } // CreateCodespace calls CreateCodespaceFunc. -func (mock *apiClientMock) CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { +func (mock *apiClientMock) CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) { if mock.CreateCodespaceFunc == nil { panic("apiClientMock.CreateCodespaceFunc: method is nil but apiClient.CreateCodespace was just called") } @@ -318,7 +319,7 @@ func (mock *apiClientMock) DeleteCodespaceCalls() []struct { } // GetCodespace calls GetCodespaceFunc. -func (mock *apiClientMock) GetCodespace(ctx context.Context, token string, user string, name string) (*api.Codespace, error) { +func (mock *apiClientMock) GetCodespace(ctx context.Context, token string, user string, name string) (*codespace.Codespace, error) { if mock.GetCodespaceFunc == nil { panic("apiClientMock.GetCodespaceFunc: method is nil but apiClient.GetCodespace was just called") } @@ -392,37 +393,37 @@ func (mock *apiClientMock) GetCodespaceRegionLocationCalls() []struct { } // GetCodespaceRepositoryContents calls GetCodespaceRepositoryContentsFunc. -func (mock *apiClientMock) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) { +func (mock *apiClientMock) GetCodespaceRepositoryContents(ctx context.Context, codespaceMoqParam *codespace.Codespace, path string) ([]byte, error) { if mock.GetCodespaceRepositoryContentsFunc == nil { panic("apiClientMock.GetCodespaceRepositoryContentsFunc: method is nil but apiClient.GetCodespaceRepositoryContents was just called") } callInfo := struct { - Ctx context.Context - Codespace *api.Codespace - Path string + Ctx context.Context + CodespaceMoqParam *codespace.Codespace + Path string }{ - Ctx: ctx, - Codespace: codespace, - Path: path, + Ctx: ctx, + CodespaceMoqParam: codespaceMoqParam, + Path: path, } mock.lockGetCodespaceRepositoryContents.Lock() mock.calls.GetCodespaceRepositoryContents = append(mock.calls.GetCodespaceRepositoryContents, callInfo) mock.lockGetCodespaceRepositoryContents.Unlock() - return mock.GetCodespaceRepositoryContentsFunc(ctx, codespace, path) + return mock.GetCodespaceRepositoryContentsFunc(ctx, codespaceMoqParam, path) } // GetCodespaceRepositoryContentsCalls gets all the calls that were made to GetCodespaceRepositoryContents. // Check the length with: // len(mockedapiClient.GetCodespaceRepositoryContentsCalls()) func (mock *apiClientMock) GetCodespaceRepositoryContentsCalls() []struct { - Ctx context.Context - Codespace *api.Codespace - Path string + Ctx context.Context + CodespaceMoqParam *codespace.Codespace + Path string } { var calls []struct { - Ctx context.Context - Codespace *api.Codespace - Path string + Ctx context.Context + CodespaceMoqParam *codespace.Codespace + Path string } mock.lockGetCodespaceRepositoryContents.RLock() calls = mock.calls.GetCodespaceRepositoryContents @@ -583,7 +584,7 @@ func (mock *apiClientMock) GetUserCalls() []struct { } // ListCodespaces calls ListCodespacesFunc. -func (mock *apiClientMock) ListCodespaces(ctx context.Context) ([]*api.Codespace, error) { +func (mock *apiClientMock) ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error) { if mock.ListCodespacesFunc == nil { panic("apiClientMock.ListCodespacesFunc: method is nil but apiClient.ListCodespaces was just called") } @@ -614,37 +615,37 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { } // StartCodespace calls StartCodespaceFunc. -func (mock *apiClientMock) StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error { +func (mock *apiClientMock) StartCodespace(ctx context.Context, token string, codespaceMoqParam *codespace.Codespace) error { if mock.StartCodespaceFunc == nil { panic("apiClientMock.StartCodespaceFunc: method is nil but apiClient.StartCodespace was just called") } callInfo := struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Token string + CodespaceMoqParam *codespace.Codespace }{ - Ctx: ctx, - Token: token, - Codespace: codespace, + Ctx: ctx, + Token: token, + CodespaceMoqParam: codespaceMoqParam, } mock.lockStartCodespace.Lock() mock.calls.StartCodespace = append(mock.calls.StartCodespace, callInfo) mock.lockStartCodespace.Unlock() - return mock.StartCodespaceFunc(ctx, token, codespace) + return mock.StartCodespaceFunc(ctx, token, codespaceMoqParam) } // StartCodespaceCalls gets all the calls that were made to StartCodespace. // Check the length with: // len(mockedapiClient.StartCodespaceCalls()) func (mock *apiClientMock) StartCodespaceCalls() []struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Token string + CodespaceMoqParam *codespace.Codespace } { var calls []struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Token string + CodespaceMoqParam *codespace.Codespace } mock.lockStartCodespace.RLock() calls = mock.calls.StartCodespace diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go index dee726bc4..90f4db635 100644 --- a/pkg/cmd/codespace/ports.go +++ b/pkg/cmd/codespace/ports.go @@ -12,7 +12,7 @@ import ( "strings" "github.com/cli/cli/v2/internal/codespaces" - "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/cmd/codespace/output" "github.com/cli/cli/v2/pkg/liveshare" "github.com/muhammadmuzzammil1998/jsonc" @@ -119,7 +119,7 @@ type portAttribute struct { Label string `json:"label"` } -func getDevContainer(ctx context.Context, apiClient apiClient, codespace *api.Codespace) <-chan devContainerResult { +func getDevContainer(ctx context.Context, apiClient apiClient, codespace *codespace.Codespace) <-chan devContainerResult { ch := make(chan devContainerResult, 1) go func() { contents, err := apiClient.GetCodespaceRepositoryContents(ctx, codespace, ".devcontainer/devcontainer.json") From 1971292175011112e461240f64ff93131e288c24 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Tue, 5 Oct 2021 13:24:47 -0400 Subject: [PATCH 2/8] Fixes bug there are +2 codespaces with a conflict - Tracks conflicting name going forward for other records - Moves the git status dirty star into a constant so we can reference it --- internal/codespaces/codespace/codespace.go | 5 +++- pkg/cmd/codespace/common.go | 31 +++++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/internal/codespaces/codespace/codespace.go b/internal/codespaces/codespace/codespace.go index 69188ea4b..66f4a2c3f 100644 --- a/internal/codespaces/codespace/codespace.go +++ b/internal/codespaces/codespace/codespace.go @@ -32,11 +32,14 @@ func (c *Codespace) DisplayName(includeName, includeGitStatus bool) string { return c.RepositoryNWO + ": " + branch } +// GitStatusDirty represents an unsaved changes status. +const GitStatusDirty = "*" + // BranchWithGitStatus returns the branch with a star // if the branch is currently being worked on. func (c *Codespace) BranchWithGitStatus() string { if c.HasUnsavedChanges() { - return c.Branch + "*" + return c.Branch + GitStatusDirty } return c.Branch diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index 1d96bbce4..05d7e615e 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -74,22 +74,30 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codesp idx int } + namesWithConflict := make(map[string]bool) codespacesByName := make(map[string]codespaceWithIndex) codespacesNames := make([]string, 0, len(codespaces)) for _, cs := range codespaces { csName := cs.DisplayName(false, false) displayNameWithGitStatus := cs.DisplayName(false, true) - if seenCodespace, ok := codespacesByName[csName]; ok { - // there is an existing codespace on the repo and branch - // we need to disambiguate by adding the codespace name - // to the existing entry and the one we are adding now - fullDisplayName := seenCodespace.cs.DisplayName(true, false) - fullDisplayNameWithGitStatus := seenCodespace.cs.DisplayName(true, true) + _, hasExistingConflict := namesWithConflict[csName] + if seenCodespace, ok := codespacesByName[csName]; ok || hasExistingConflict { + // There is an existing codespace on the repo and branch. + // We need to disambiguate by adding the codespace name + // to the existing entry and the one we are processing now. + if !hasExistingConflict { + fullDisplayName := seenCodespace.cs.DisplayName(true, false) + fullDisplayNameWithGitStatus := seenCodespace.cs.DisplayName(true, true) - codespacesByName[fullDisplayName] = codespaceWithIndex{seenCodespace.cs, seenCodespace.idx} - codespacesNames[seenCodespace.idx] = fullDisplayNameWithGitStatus - delete(codespacesByName, csName) // delete the existing map entry with old name + codespacesByName[fullDisplayName] = codespaceWithIndex{seenCodespace.cs, seenCodespace.idx} + codespacesNames[seenCodespace.idx] = fullDisplayNameWithGitStatus + delete(codespacesByName, csName) // delete the existing map entry with old name + + // All other codespaces with the same name should update + // to their specific name, this tracks conflicting names going forward + namesWithConflict[csName] = true + } // update this codespace names to include the name to disambiguate csName = cs.DisplayName(true, false) @@ -119,7 +127,10 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codesp return nil, fmt.Errorf("error getting answers: %w", err) } - selectedCodespace := strings.Replace(answers.Codespace, "*", "", -1) + // Codespaces are indexed without the git status included as compared + // to how it is displayed in the prompt, so the git status symbol needs + // cleaning up in case it is included. + selectedCodespace := strings.Replace(answers.Codespace, codespace.GitStatusDirty, "", -1) codespace := codespacesByName[selectedCodespace].cs return codespace, nil } From 7fe8357d40883f2d6294197bb53b428320fcb401 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Tue, 5 Oct 2021 13:30:17 -0400 Subject: [PATCH 3/8] Better short name --- internal/codespaces/api/api.go | 6 +++--- internal/codespaces/states.go | 6 +++--- pkg/cmd/codespace/ports.go | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index da155b9ab..72d5cf338 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -524,14 +524,14 @@ type getCodespaceRepositoryContentsResponse struct { Content string `json:"content"` } -func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *codespace.Codespace, path string) ([]byte, error) { - req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+codespace.RepositoryNWO+"/contents/"+path, nil) +func (a *API) GetCodespaceRepositoryContents(ctx context.Context, cs *codespace.Codespace, path string) ([]byte, error) { + req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+cs.RepositoryNWO+"/contents/"+path, nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } q := req.URL.Query() - q.Add("ref", codespace.Branch) + q.Add("ref", cs.Branch) req.URL.RawQuery = q.Encode() a.setHeaders(req) diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index e37b625e3..14c2cc2ea 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -37,13 +37,13 @@ type PostCreateState struct { // PollPostCreateStates watches for state changes in a codespace, // and calls the supplied poller for each batch of state changes. // It runs until it encounters an error, including cancellation of the context. -func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, user *api.User, c *codespace.Codespace, poller func([]PostCreateState)) (err error) { - token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) +func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, user *api.User, cs *codespace.Codespace, poller func([]PostCreateState)) (err error) { + token, err := apiClient.GetCodespaceToken(ctx, user.Login, cs.Name) if err != nil { return fmt.Errorf("getting codespace token: %w", err) } - session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, c) + session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, cs) if err != nil { return fmt.Errorf("connect to Live Share: %w", err) } diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go index a535efd12..3c7b2255e 100644 --- a/pkg/cmd/codespace/ports.go +++ b/pkg/cmd/codespace/ports.go @@ -119,10 +119,10 @@ type portAttribute struct { Label string `json:"label"` } -func getDevContainer(ctx context.Context, apiClient apiClient, codespace *codespace.Codespace) <-chan devContainerResult { +func getDevContainer(ctx context.Context, apiClient apiClient, cs *codespace.Codespace) <-chan devContainerResult { ch := make(chan devContainerResult, 1) go func() { - contents, err := apiClient.GetCodespaceRepositoryContents(ctx, codespace, ".devcontainer/devcontainer.json") + contents, err := apiClient.GetCodespaceRepositoryContents(ctx, cs, ".devcontainer/devcontainer.json") if err != nil { ch <- devContainerResult{nil, fmt.Errorf("error getting content: %w", err)} return From 3a2864363035720e9c1da0371abab2f4c4847176 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 11:44:26 -0400 Subject: [PATCH 4/8] Keep codespace struct in API for now - Use a private codespace structure in the cmd pkg to encapsulate common behavior --- internal/codespaces/api/api.go | 61 +++++++++++++--- internal/codespaces/api/api_test.go | 6 +- internal/codespaces/codespace/codespace.go | 82 ---------------------- internal/codespaces/codespaces.go | 12 ++-- internal/codespaces/states.go | 4 +- pkg/cmd/codespace/common.go | 78 +++++++++++++++----- pkg/cmd/codespace/create.go | 3 +- pkg/cmd/codespace/delete.go | 13 ++-- pkg/cmd/codespace/delete_test.go | 29 ++++---- pkg/cmd/codespace/list.go | 13 ++-- pkg/cmd/codespace/mock_api.go | 55 +++++++-------- pkg/cmd/codespace/ports.go | 4 +- 12 files changed, 178 insertions(+), 182 deletions(-) delete mode 100644 internal/codespaces/codespace/codespace.go diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 0fd87fda2..44ec720e9 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -37,7 +37,6 @@ import ( "strings" "time" - "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/opentracing/opentracing-go" ) @@ -147,7 +146,49 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error return &response, nil } -func (a *API) ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error) { +type Codespace struct { + Name string `json:"name"` + CreatedAt string `json:"created_at"` + LastUsedAt string `json:"last_used_at"` + State string `json:"state"` + GUID string `json:"guid"` + Branch string `json:"branch"` + RepositoryName string `json:"repository_name"` + RepositoryNWO string `json:"repository_nwo"` + OwnerLogin string `json:"owner_login"` + Environment CodespaceEnvironment `json:"environment"` +} + +const CodespaceStateProvisioned = "provisioned" + +type CodespaceEnvironment struct { + State string `json:"state"` + Connection CodespaceEnvironmentConnection `json:"connection"` + GitStatus CodespaceEnvironmentGitStatus `json:"gitStatus"` +} + +type CodespaceEnvironmentGitStatus struct { + Ahead int `json:"ahead"` + Behind int `json:"behind"` + Branch string `json:"branch"` + Commit string `json:"commit"` + HasUnpushedChanges bool `json:"hasUnpushedChanges"` + HasUncommitedChanges bool `json:"hasUncommitedChanges"` +} + +const ( + CodespaceEnvironmentStateAvailable = "Available" +) + +type CodespaceEnvironmentConnection struct { + SessionID string `json:"sessionId"` + SessionToken string `json:"sessionToken"` + RelayEndpoint string `json:"relayEndpoint"` + RelaySAS string `json:"relaySas"` + HostPublicKeys []string `json:"hostPublicKeys"` +} + +func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { req, err := http.NewRequest( http.MethodGet, a.githubAPI+"/user/codespaces", nil, ) @@ -172,7 +213,7 @@ func (a *API) ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error } var response struct { - Codespaces []*codespace.Codespace `json:"codespaces"` + Codespaces []*Codespace `json:"codespaces"` } if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) @@ -183,7 +224,7 @@ func (a *API) ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error // GetCodespace returns the user codespace based on the provided name. // If the codespace is not found, an error is returned. // If includeConnection is true, it will return the connection information for the codespace. -func (a *API) GetCodespace(ctx context.Context, codespaceName string, includeConnection bool) (*codespace.Codespace, error) { +func (a *API) GetCodespace(ctx context.Context, codespaceName string, includeConnection bool) (*Codespace, error) { req, err := http.NewRequest( http.MethodGet, a.githubAPI+"/user/codespaces/"+codespaceName, @@ -216,7 +257,7 @@ func (a *API) GetCodespace(ctx context.Context, codespaceName string, includeCon return nil, jsonErrorResponse(b) } - var response codespace.Codespace + var response Codespace if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) } @@ -354,7 +395,7 @@ type CreateCodespaceParams struct { // CreateCodespace creates a codespace with the given parameters and returns a non-nil error if it // fails to create. -func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams) (*codespace.Codespace, error) { +func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams) (*Codespace, error) { cs, err := a.startCreate(ctx, params.RepositoryID, params.Machine, params.Branch, params.Location) if err != errProvisioningInProgress { return nil, err @@ -380,7 +421,7 @@ func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams } // we continue to poll until the codespace shows as provisioned - if cs.State != codespace.StateProvisioned { + if cs.State != CodespaceStateProvisioned { continue } @@ -402,7 +443,7 @@ var errProvisioningInProgress = errors.New("provisioning in progress") // It may return success or an error, or errProvisioningInProgress indicating that the operation // did not complete before the GitHub API's time limit for RPCs (10s), in which case the caller // must poll the server to learn the outcome. -func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, location string) (*codespace.Codespace, error) { +func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, location string) (*Codespace, error) { requestBody, err := json.Marshal(startCreateRequest{repoID, branch, location, machine}) if err != nil { return nil, fmt.Errorf("error marshaling request: %w", err) @@ -432,7 +473,7 @@ func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, loca return nil, errProvisioningInProgress // RPC finished before result of creation known } - var response codespace.Codespace + var response Codespace if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) } @@ -469,7 +510,7 @@ type getCodespaceRepositoryContentsResponse struct { Content string `json:"content"` } -func (a *API) GetCodespaceRepositoryContents(ctx context.Context, cs *codespace.Codespace, path string) ([]byte, error) { +func (a *API) GetCodespaceRepositoryContents(ctx context.Context, cs *Codespace, path string) ([]byte, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+cs.RepositoryNWO+"/contents/"+path, nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) diff --git a/internal/codespaces/api/api_test.go b/internal/codespaces/api/api_test.go index 8ca4b5d5f..8bbe1c8a9 100644 --- a/internal/codespaces/api/api_test.go +++ b/internal/codespaces/api/api_test.go @@ -7,12 +7,10 @@ import ( "net/http" "net/http/httptest" "testing" - - "github.com/cli/cli/v2/internal/codespaces/codespace" ) func TestListCodespaces(t *testing.T) { - codespaces := []*codespace.Codespace{ + codespaces := []*Codespace{ { Name: "testcodespace", CreatedAt: "2021-08-09T10:10:24+02:00", @@ -21,7 +19,7 @@ func TestListCodespaces(t *testing.T) { } svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { response := struct { - Codespaces []*codespace.Codespace `json:"codespaces"` + Codespaces []*Codespace `json:"codespaces"` }{ Codespaces: codespaces, } diff --git a/internal/codespaces/codespace/codespace.go b/internal/codespaces/codespace/codespace.go deleted file mode 100644 index 4fb8916d6..000000000 --- a/internal/codespaces/codespace/codespace.go +++ /dev/null @@ -1,82 +0,0 @@ -package codespace - -import "fmt" - -type Codespace struct { - Name string `json:"name"` - CreatedAt string `json:"created_at"` - LastUsedAt string `json:"last_used_at"` - State string `json:"state"` - GUID string `json:"guid"` - Branch string `json:"branch"` - RepositoryName string `json:"repository_name"` - RepositoryNWO string `json:"repository_nwo"` - OwnerLogin string `json:"owner_login"` - Environment Environment `json:"environment"` -} - -const StateProvisioned = "provisioned" - -// DisplayName returns the repository nwo and branch. -// If includeName is true, the name of the codespace is included. -// If includeGitStatus is true, the branch will include a star if -// the codespace has unsaved changes. -func (c *Codespace) DisplayName(includeName, includeGitStatus bool) string { - branch := c.Branch - if includeGitStatus { - branch = c.BranchWithGitStatus() - } - - if includeName { - return fmt.Sprintf( - "%s: %s [%s]", c.RepositoryNWO, branch, c.Name, - ) - } - return c.RepositoryNWO + ": " + branch -} - -// GitStatusDirty represents an unsaved changes status. -const GitStatusDirty = "*" - -// BranchWithGitStatus returns the branch with a star -// if the branch is currently being worked on. -func (c *Codespace) BranchWithGitStatus() string { - if c.HasUnsavedChanges() { - return c.Branch + GitStatusDirty - } - - return c.Branch -} - -// HasUnsavedChanges returns whether the environment has -// unsaved changes. -func (c *Codespace) HasUnsavedChanges() bool { - return c.Environment.GitStatus.HasUncommitedChanges || c.Environment.GitStatus.HasUnpushedChanges -} - -type Environment struct { - State string `json:"state"` - Connection EnvironmentConnection `json:"connection"` - GitStatus EnvironmentGitStatus `json:"gitStatus"` -} - -type EnvironmentGitStatus struct { - Ahead int `json:"ahead"` - Behind int `json:"behind"` - Branch string `json:"branch"` - Commit string `json:"commit"` - HasUnpushedChanges bool `json:"hasUnpushedChanges"` - HasUncommitedChanges bool `json:"hasUncommitedChanges"` -} - -const ( - EnvironmentStateAvailable = "Available" -) - -type EnvironmentConnection struct { - SessionID string `json:"sessionId"` - SessionToken string `json:"sessionToken"` - RelayEndpoint string `json:"relayEndpoint"` - RelaySAS string `json:"relaySas"` - HostPublicKeys []string `json:"hostPublicKeys"` -} diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 6b8358778..15499c738 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -6,7 +6,7 @@ import ( "fmt" "time" - "github.com/cli/cli/v2/internal/codespaces/codespace" + "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/pkg/liveshare" ) @@ -15,24 +15,24 @@ type logger interface { Println(v ...interface{}) (int, error) } -func connectionReady(cs *codespace.Codespace) bool { +func connectionReady(cs *api.Codespace) bool { return cs.Environment.Connection.SessionID != "" && cs.Environment.Connection.SessionToken != "" && cs.Environment.Connection.RelayEndpoint != "" && cs.Environment.Connection.RelaySAS != "" && - cs.Environment.State == codespace.EnvironmentStateAvailable + cs.Environment.State == api.CodespaceEnvironmentStateAvailable } type apiClient interface { - GetCodespace(ctx context.Context, name string, includeConnection bool) (*codespace.Codespace, error) + GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) StartCodespace(ctx context.Context, name string) error } // ConnectToLiveshare waits for a Codespace to become running, // and connects to it using a Live Share session. -func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, cs *codespace.Codespace) (*liveshare.Session, error) { +func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, cs *api.Codespace) (*liveshare.Session, error) { var startedCodespace bool - if cs.Environment.State != codespace.EnvironmentStateAvailable { + if cs.Environment.State != api.CodespaceEnvironmentStateAvailable { startedCodespace = true log.Print("Starting your codespace...") if err := apiClient.StartCodespace(ctx, cs.Name); err != nil { diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index dfae33f09..9a1204f4a 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -9,7 +9,7 @@ import ( "strings" "time" - "github.com/cli/cli/v2/internal/codespaces/codespace" + "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/pkg/liveshare" ) @@ -36,7 +36,7 @@ type PostCreateState struct { // PollPostCreateStates watches for state changes in a codespace, // and calls the supplied poller for each batch of state changes. // It runs until it encounters an error, including cancellation of the context. -func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, cs *codespace.Codespace, poller func([]PostCreateState)) (err error) { +func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, cs *api.Codespace, poller func([]PostCreateState)) (err error) { session, err := ConnectToLiveshare(ctx, log, apiClient, cs) if err != nil { return fmt.Errorf("connect to Live Share: %w", err) diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index f7e77c2fa..5ced6f50a 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -14,7 +14,6 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/terminal" "github.com/cli/cli/v2/internal/codespaces/api" - "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/cmd/codespace/output" "github.com/spf13/cobra" "golang.org/x/term" @@ -35,21 +34,21 @@ func NewApp(logger *output.Logger, apiClient apiClient) *App { //go:generate moq -fmt goimports -rm -skip-ensure -out mock_api.go . apiClient type apiClient interface { GetUser(ctx context.Context) (*api.User, error) - GetCodespace(ctx context.Context, name string, includeConnection bool) (*codespace.Codespace, error) - ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error) + GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) + ListCodespaces(ctx context.Context) ([]*api.Codespace, error) DeleteCodespace(ctx context.Context, name string) error StartCodespace(ctx context.Context, name string) error - CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) + CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) GetCodespaceRegionLocation(ctx context.Context) (string, error) GetCodespacesMachines(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) - GetCodespaceRepositoryContents(ctx context.Context, codespace *codespace.Codespace, path string) ([]byte, error) + GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) } var errNoCodespaces = errors.New("you have no codespaces") -func chooseCodespace(ctx context.Context, apiClient apiClient) (*codespace.Codespace, error) { +func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, error) { codespaces, err := apiClient.ListCodespaces(ctx) if err != nil { return nil, fmt.Errorf("error getting codespaces: %w", err) @@ -59,7 +58,7 @@ func chooseCodespace(ctx context.Context, apiClient apiClient) (*codespace.Codes // chooseCodespaceFromList returns the selected codespace from the list, // or an error if there are no codespaces. -func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codespace) (*codespace.Codespace, error) { +func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) (*api.Codespace, error) { if len(codespaces) == 0 { return nil, errNoCodespaces } @@ -69,16 +68,17 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codesp }) type codespaceWithIndex struct { - cs *codespace.Codespace + cs codespace idx int } namesWithConflict := make(map[string]bool) codespacesByName := make(map[string]codespaceWithIndex) codespacesNames := make([]string, 0, len(codespaces)) - for _, cs := range codespaces { - csName := cs.DisplayName(false, false) - displayNameWithGitStatus := cs.DisplayName(false, true) + for _, apiCodespace := range codespaces { + cs := codespace{apiCodespace} + csName := cs.displayName(false, false) + displayNameWithGitStatus := cs.displayName(false, true) _, hasExistingConflict := namesWithConflict[csName] if seenCodespace, ok := codespacesByName[csName]; ok || hasExistingConflict { @@ -86,8 +86,8 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codesp // We need to disambiguate by adding the codespace name // to the existing entry and the one we are processing now. if !hasExistingConflict { - fullDisplayName := seenCodespace.cs.DisplayName(true, false) - fullDisplayNameWithGitStatus := seenCodespace.cs.DisplayName(true, true) + fullDisplayName := seenCodespace.cs.displayName(true, false) + fullDisplayNameWithGitStatus := seenCodespace.cs.displayName(true, true) codespacesByName[fullDisplayName] = codespaceWithIndex{seenCodespace.cs, seenCodespace.idx} codespacesNames[seenCodespace.idx] = fullDisplayNameWithGitStatus @@ -99,8 +99,8 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codesp } // update this codespace names to include the name to disambiguate - csName = cs.DisplayName(true, false) - displayNameWithGitStatus = cs.DisplayName(true, true) + csName = cs.displayName(true, false) + displayNameWithGitStatus = cs.displayName(true, true) } codespacesByName[csName] = codespaceWithIndex{cs, len(codespacesNames)} @@ -129,14 +129,13 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*codespace.Codesp // Codespaces are indexed without the git status included as compared // to how it is displayed in the prompt, so the git status symbol needs // cleaning up in case it is included. - selectedCodespace := strings.Replace(answers.Codespace, codespace.GitStatusDirty, "", -1) - codespace := codespacesByName[selectedCodespace].cs - return codespace, nil + selectedCodespace := strings.Replace(answers.Codespace, gitStatusDirty, "", -1) + return codespacesByName[selectedCodespace].cs.Codespace, nil } // getOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty. // It then fetches the codespace record with full connection details. -func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceName string) (cs *codespace.Codespace, err error) { +func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceName string) (cs *api.Codespace, err error) { if codespaceName == "" { cs, err = chooseCodespace(ctx, apiClient) if err != nil { @@ -211,3 +210,44 @@ func noArgsConstraint(cmd *cobra.Command, args []string) error { } return nil } + +type codespace struct { + *api.Codespace +} + +// displayName returns the repository nwo and branch. +// If includeName is true, the name of the codespace is included. +// If includeGitStatus is true, the branch will include a star if +// the codespace has unsaved changes. +func (c codespace) displayName(includeName, includeGitStatus bool) string { + branch := c.Branch + if includeGitStatus { + branch = c.branchWithGitStatus() + } + + if includeName { + return fmt.Sprintf( + "%s: %s [%s]", c.RepositoryNWO, branch, c.Name, + ) + } + return c.RepositoryNWO + ": " + branch +} + +// GitStatusDirty represents an unsaved changes status. +const gitStatusDirty = "*" + +// BranchWithGitStatus returns the branch with a star +// if the branch is currently being worked on. +func (c codespace) branchWithGitStatus() string { + if c.hasUnsavedChanges() { + return c.Branch + gitStatusDirty + } + + return c.Branch +} + +// HasUnsavedChanges returns whether the environment has +// unsaved changes. +func (c codespace) hasUnsavedChanges() bool { + return c.Environment.GitStatus.HasUncommitedChanges || c.Environment.GitStatus.HasUnpushedChanges +} diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index c0f699ce1..97bd0af7d 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -10,7 +10,6 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/internal/codespaces" "github.com/cli/cli/v2/internal/codespaces/api" - "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/cmd/codespace/output" "github.com/fatih/camelcase" "github.com/spf13/cobra" @@ -108,7 +107,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { // showStatus polls the codespace for a list of post create states and their status. It will keep polling // until all states have finished. Once all states have finished, we poll once more to check if any new // states have been introduced and stop polling otherwise. -func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, user *api.User, cs *codespace.Codespace) error { +func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, user *api.User, cs *api.Codespace) error { var lastState codespaces.PostCreateState var breakNextState bool diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index e95367af7..ee5436c0a 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -8,7 +8,7 @@ import ( "time" "github.com/AlecAivazis/survey/v2" - "github.com/cli/cli/v2/internal/codespaces/codespace" + "github.com/cli/cli/v2/internal/codespaces/api" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" ) @@ -59,7 +59,7 @@ func newDeleteCmd(app *App) *cobra.Command { } func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { - var codespaces []*codespace.Codespace + var codespaces []*api.Codespace nameFilter := opts.codespaceName if nameFilter == "" { codespaces, err = a.apiClient.ListCodespaces(ctx) @@ -80,10 +80,10 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { return fmt.Errorf("error fetching codespace information: %w", err) } - codespaces = []*codespace.Codespace{cs} + codespaces = []*api.Codespace{cs} } - codespacesToDelete := make([]*codespace.Codespace, 0, len(codespaces)) + codespacesToDelete := make([]*api.Codespace, 0, len(codespaces)) lastUpdatedCutoffTime := opts.now().AddDate(0, 0, -int(opts.keepDays)) for _, c := range codespaces { if nameFilter != "" && c.Name != nameFilter { @@ -142,8 +142,9 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { return nil } -func confirmDeletion(p prompter, cs *codespace.Codespace, isInteractive bool) (bool, error) { - if !cs.HasUnsavedChanges() { +func confirmDeletion(p prompter, apiCodespace *api.Codespace, isInteractive bool) (bool, error) { + cs := codespace{apiCodespace} + if !cs.hasUnsavedChanges() { return true, nil } if !isInteractive { diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index b5022962d..14896098a 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -12,7 +12,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/codespaces/api" - "github.com/cli/cli/v2/internal/codespaces/codespace" "github.com/cli/cli/v2/pkg/cmd/codespace/output" ) @@ -26,7 +25,7 @@ func TestDelete(t *testing.T) { tests := []struct { name string opts deleteOptions - codespaces []*codespace.Codespace + codespaces []*api.Codespace confirms map[string]bool deleteErr error wantErr bool @@ -39,7 +38,7 @@ func TestDelete(t *testing.T) { opts: deleteOptions{ codespaceName: "hubot-robawt-abc", }, - codespaces: []*codespace.Codespace{ + codespaces: []*api.Codespace{ { Name: "hubot-robawt-abc", }, @@ -51,7 +50,7 @@ func TestDelete(t *testing.T) { opts: deleteOptions{ repoFilter: "monalisa/spoon-knife", }, - codespaces: []*codespace.Codespace{ + codespaces: []*api.Codespace{ { Name: "monalisa-spoonknife-123", RepositoryNWO: "monalisa/Spoon-Knife", @@ -73,7 +72,7 @@ func TestDelete(t *testing.T) { deleteAll: true, keepDays: 3, }, - codespaces: []*codespace.Codespace{ + codespaces: []*api.Codespace{ { Name: "monalisa-spoonknife-123", LastUsedAt: daysAgo(1), @@ -94,7 +93,7 @@ func TestDelete(t *testing.T) { opts: deleteOptions{ deleteAll: true, }, - codespaces: []*codespace.Codespace{ + codespaces: []*api.Codespace{ { Name: "monalisa-spoonknife-123", }, @@ -117,27 +116,27 @@ func TestDelete(t *testing.T) { deleteAll: true, skipConfirm: false, }, - codespaces: []*codespace.Codespace{ + codespaces: []*api.Codespace{ { Name: "monalisa-spoonknife-123", - Environment: codespace.Environment{ - GitStatus: codespace.EnvironmentGitStatus{ + Environment: api.CodespaceEnvironment{ + GitStatus: api.CodespaceEnvironmentGitStatus{ HasUnpushedChanges: true, }, }, }, { Name: "hubot-robawt-abc", - Environment: codespace.Environment{ - GitStatus: codespace.EnvironmentGitStatus{ + Environment: api.CodespaceEnvironment{ + GitStatus: api.CodespaceEnvironmentGitStatus{ HasUncommitedChanges: true, }, }, }, { Name: "monalisa-spoonknife-c4f3", - Environment: codespace.Environment{ - GitStatus: codespace.EnvironmentGitStatus{ + Environment: api.CodespaceEnvironment{ + GitStatus: api.CodespaceEnvironmentGitStatus{ HasUnpushedChanges: false, HasUncommitedChanges: false, }, @@ -165,11 +164,11 @@ func TestDelete(t *testing.T) { }, } if tt.opts.codespaceName == "" { - apiMock.ListCodespacesFunc = func(_ context.Context) ([]*codespace.Codespace, error) { + apiMock.ListCodespacesFunc = func(_ context.Context) ([]*api.Codespace, error) { return tt.codespaces, nil } } else { - apiMock.GetCodespaceFunc = func(_ context.Context, name string, includeConnection bool) (*codespace.Codespace, error) { + apiMock.GetCodespaceFunc = func(_ context.Context, name string, includeConnection bool) (*api.Codespace, error) { return tt.codespaces[0], nil } } diff --git a/pkg/cmd/codespace/list.go b/pkg/cmd/codespace/list.go index edab5094d..4bc649bf7 100644 --- a/pkg/cmd/codespace/list.go +++ b/pkg/cmd/codespace/list.go @@ -34,13 +34,14 @@ func (a *App) List(ctx context.Context, asJSON bool) error { table := output.NewTable(os.Stdout, asJSON) table.SetHeader([]string{"Name", "Repository", "Branch", "State", "Created At"}) - for _, codespace := range codespaces { + for _, apiCodespace := range codespaces { + cs := codespace{apiCodespace} table.Append([]string{ - codespace.Name, - codespace.RepositoryNWO, - codespace.BranchWithGitStatus(), - codespace.Environment.State, - codespace.CreatedAt, + cs.Name, + cs.RepositoryNWO, + cs.branchWithGitStatus(), + cs.Environment.State, + cs.CreatedAt, }) } diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index c92c9234e..df9b6f3b2 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -8,7 +8,6 @@ import ( "sync" "github.com/cli/cli/v2/internal/codespaces/api" - "github.com/cli/cli/v2/internal/codespaces/codespace" ) // apiClientMock is a mock implementation of apiClient. @@ -20,19 +19,19 @@ import ( // AuthorizedKeysFunc: func(ctx context.Context, user string) ([]byte, error) { // panic("mock out the AuthorizedKeys method") // }, -// CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) { +// CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { // panic("mock out the CreateCodespace method") // }, // DeleteCodespaceFunc: func(ctx context.Context, name string) error { // panic("mock out the DeleteCodespace method") // }, -// GetCodespaceFunc: func(ctx context.Context, name string, includeConnection bool) (*codespace.Codespace, error) { +// GetCodespaceFunc: func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) { // panic("mock out the GetCodespace method") // }, // GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { // panic("mock out the GetCodespaceRegionLocation method") // }, -// GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespaceMoqParam *codespace.Codespace, path string) ([]byte, error) { +// GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) { // panic("mock out the GetCodespaceRepositoryContents method") // }, // GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) { @@ -44,7 +43,7 @@ import ( // GetUserFunc: func(ctx context.Context) (*api.User, error) { // panic("mock out the GetUser method") // }, -// ListCodespacesFunc: func(ctx context.Context) ([]*codespace.Codespace, error) { +// ListCodespacesFunc: func(ctx context.Context) ([]*api.Codespace, error) { // panic("mock out the ListCodespaces method") // }, // StartCodespaceFunc: func(ctx context.Context, name string) error { @@ -61,19 +60,19 @@ type apiClientMock struct { AuthorizedKeysFunc func(ctx context.Context, user string) ([]byte, error) // CreateCodespaceFunc mocks the CreateCodespace method. - CreateCodespaceFunc func(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) + CreateCodespaceFunc func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) // DeleteCodespaceFunc mocks the DeleteCodespace method. DeleteCodespaceFunc func(ctx context.Context, name string) error // GetCodespaceFunc mocks the GetCodespace method. - GetCodespaceFunc func(ctx context.Context, name string, includeConnection bool) (*codespace.Codespace, error) + GetCodespaceFunc func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) // GetCodespaceRegionLocationFunc mocks the GetCodespaceRegionLocation method. GetCodespaceRegionLocationFunc func(ctx context.Context) (string, error) // GetCodespaceRepositoryContentsFunc mocks the GetCodespaceRepositoryContents method. - GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespaceMoqParam *codespace.Codespace, path string) ([]byte, error) + GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) // GetCodespacesMachinesFunc mocks the GetCodespacesMachines method. GetCodespacesMachinesFunc func(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) @@ -85,7 +84,7 @@ type apiClientMock struct { GetUserFunc func(ctx context.Context) (*api.User, error) // ListCodespacesFunc mocks the ListCodespaces method. - ListCodespacesFunc func(ctx context.Context) ([]*codespace.Codespace, error) + ListCodespacesFunc func(ctx context.Context) ([]*api.Codespace, error) // StartCodespaceFunc mocks the StartCodespace method. StartCodespaceFunc func(ctx context.Context, name string) error @@ -131,8 +130,8 @@ type apiClientMock struct { GetCodespaceRepositoryContents []struct { // Ctx is the ctx argument value. Ctx context.Context - // CodespaceMoqParam is the codespaceMoqParam argument value. - CodespaceMoqParam *codespace.Codespace + // Codespace is the codespace argument value. + Codespace *api.Codespace // Path is the path argument value. Path string } @@ -221,7 +220,7 @@ func (mock *apiClientMock) AuthorizedKeysCalls() []struct { } // CreateCodespace calls CreateCodespaceFunc. -func (mock *apiClientMock) CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*codespace.Codespace, error) { +func (mock *apiClientMock) CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { if mock.CreateCodespaceFunc == nil { panic("apiClientMock.CreateCodespaceFunc: method is nil but apiClient.CreateCodespace was just called") } @@ -291,7 +290,7 @@ func (mock *apiClientMock) DeleteCodespaceCalls() []struct { } // GetCodespace calls GetCodespaceFunc. -func (mock *apiClientMock) GetCodespace(ctx context.Context, name string, includeConnection bool) (*codespace.Codespace, error) { +func (mock *apiClientMock) GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) { if mock.GetCodespaceFunc == nil { panic("apiClientMock.GetCodespaceFunc: method is nil but apiClient.GetCodespace was just called") } @@ -361,37 +360,37 @@ func (mock *apiClientMock) GetCodespaceRegionLocationCalls() []struct { } // GetCodespaceRepositoryContents calls GetCodespaceRepositoryContentsFunc. -func (mock *apiClientMock) GetCodespaceRepositoryContents(ctx context.Context, codespaceMoqParam *codespace.Codespace, path string) ([]byte, error) { +func (mock *apiClientMock) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) { if mock.GetCodespaceRepositoryContentsFunc == nil { panic("apiClientMock.GetCodespaceRepositoryContentsFunc: method is nil but apiClient.GetCodespaceRepositoryContents was just called") } callInfo := struct { - Ctx context.Context - CodespaceMoqParam *codespace.Codespace - Path string + Ctx context.Context + Codespace *api.Codespace + Path string }{ - Ctx: ctx, - CodespaceMoqParam: codespaceMoqParam, - Path: path, + Ctx: ctx, + Codespace: codespace, + Path: path, } mock.lockGetCodespaceRepositoryContents.Lock() mock.calls.GetCodespaceRepositoryContents = append(mock.calls.GetCodespaceRepositoryContents, callInfo) mock.lockGetCodespaceRepositoryContents.Unlock() - return mock.GetCodespaceRepositoryContentsFunc(ctx, codespaceMoqParam, path) + return mock.GetCodespaceRepositoryContentsFunc(ctx, codespace, path) } // GetCodespaceRepositoryContentsCalls gets all the calls that were made to GetCodespaceRepositoryContents. // Check the length with: // len(mockedapiClient.GetCodespaceRepositoryContentsCalls()) func (mock *apiClientMock) GetCodespaceRepositoryContentsCalls() []struct { - Ctx context.Context - CodespaceMoqParam *codespace.Codespace - Path string + Ctx context.Context + Codespace *api.Codespace + Path string } { var calls []struct { - Ctx context.Context - CodespaceMoqParam *codespace.Codespace - Path string + Ctx context.Context + Codespace *api.Codespace + Path string } mock.lockGetCodespaceRepositoryContents.RLock() calls = mock.calls.GetCodespaceRepositoryContents @@ -509,7 +508,7 @@ func (mock *apiClientMock) GetUserCalls() []struct { } // ListCodespaces calls ListCodespacesFunc. -func (mock *apiClientMock) ListCodespaces(ctx context.Context) ([]*codespace.Codespace, error) { +func (mock *apiClientMock) ListCodespaces(ctx context.Context) ([]*api.Codespace, error) { if mock.ListCodespacesFunc == nil { panic("apiClientMock.ListCodespacesFunc: method is nil but apiClient.ListCodespaces was just called") } diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go index 6bd7e2726..709b900bf 100644 --- a/pkg/cmd/codespace/ports.go +++ b/pkg/cmd/codespace/ports.go @@ -12,7 +12,7 @@ import ( "strings" "github.com/cli/cli/v2/internal/codespaces" - "github.com/cli/cli/v2/internal/codespaces/codespace" + "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/pkg/cmd/codespace/output" "github.com/cli/cli/v2/pkg/liveshare" "github.com/muhammadmuzzammil1998/jsonc" @@ -114,7 +114,7 @@ type portAttribute struct { Label string `json:"label"` } -func getDevContainer(ctx context.Context, apiClient apiClient, cs *codespace.Codespace) <-chan devContainerResult { +func getDevContainer(ctx context.Context, apiClient apiClient, cs *api.Codespace) <-chan devContainerResult { ch := make(chan devContainerResult, 1) go func() { contents, err := apiClient.GetCodespaceRepositoryContents(ctx, cs, ".devcontainer/devcontainer.json") From a509c2d88487c05d5ac84f781536e8dbd08e8543 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 11:45:43 -0400 Subject: [PATCH 5/8] Remove unused guid --- internal/codespaces/api/api.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 44ec720e9..8302fc113 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -151,7 +151,6 @@ type Codespace struct { CreatedAt string `json:"created_at"` LastUsedAt string `json:"last_used_at"` State string `json:"state"` - GUID string `json:"guid"` Branch string `json:"branch"` RepositoryName string `json:"repository_name"` RepositoryNWO string `json:"repository_nwo"` From 771ac714ac955451c3c917ea6bbf43f8d126ec3e Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 11:47:18 -0400 Subject: [PATCH 6/8] Update docs --- pkg/cmd/codespace/common.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index 5ced6f50a..b4de5faf6 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -233,10 +233,10 @@ func (c codespace) displayName(includeName, includeGitStatus bool) string { return c.RepositoryNWO + ": " + branch } -// GitStatusDirty represents an unsaved changes status. +// gitStatusDirty represents an unsaved changes status. const gitStatusDirty = "*" -// BranchWithGitStatus returns the branch with a star +// branchWithGitStatus returns the branch with a star // if the branch is currently being worked on. func (c codespace) branchWithGitStatus() string { if c.hasUnsavedChanges() { @@ -246,7 +246,7 @@ func (c codespace) branchWithGitStatus() string { return c.Branch } -// HasUnsavedChanges returns whether the environment has +// hasUnsavedChanges returns whether the environment has // unsaved changes. func (c codespace) hasUnsavedChanges() bool { return c.Environment.GitStatus.HasUncommitedChanges || c.Environment.GitStatus.HasUnpushedChanges From bdc9ad30e765c05913245247a44fcd3f8c573a13 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 13:46:04 -0400 Subject: [PATCH 7/8] Revert other rename changes --- internal/codespaces/api/api.go | 16 +++++++++------- internal/codespaces/codespaces.go | 32 +++++++++++++++---------------- internal/codespaces/states.go | 4 ++-- pkg/cmd/codespace/common.go | 8 ++++---- pkg/cmd/codespace/create.go | 4 ++-- pkg/cmd/codespace/delete.go | 4 ++-- 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 1203e5eab..6fd9e4d18 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -147,6 +147,7 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error return &response, nil } +// Codespace represents a codespace. type Codespace struct { Name string `json:"name"` CreatedAt string `json:"created_at"` @@ -177,6 +178,7 @@ type CodespaceEnvironmentGitStatus struct { } const ( + // CodespaceEnvironmentStateAvailable is the state for a running codespace environment. CodespaceEnvironmentStateAvailable = "Available" ) @@ -421,7 +423,7 @@ type CreateCodespaceParams struct { // CreateCodespace creates a codespace with the given parameters and returns a non-nil error if it // fails to create. func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams) (*Codespace, error) { - cs, err := a.startCreate(ctx, params.RepositoryID, params.Machine, params.Branch, params.Location) + codespace, err := a.startCreate(ctx, params.RepositoryID, params.Machine, params.Branch, params.Location) if err != errProvisioningInProgress { return nil, err } @@ -440,17 +442,17 @@ func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams case <-ctx.Done(): return nil, ctx.Err() case <-ticker.C: - cs, err = a.GetCodespace(ctx, cs.Name, false) + codespace, err = a.GetCodespace(ctx, codespace.Name, false) if err != nil { return nil, fmt.Errorf("failed to get codespace: %w", err) } // we continue to poll until the codespace shows as provisioned - if cs.State != CodespaceStateProvisioned { + if codespace.State != CodespaceStateProvisioned { continue } - return cs, nil + return codespace, nil } } } @@ -535,14 +537,14 @@ type getCodespaceRepositoryContentsResponse struct { Content string `json:"content"` } -func (a *API) GetCodespaceRepositoryContents(ctx context.Context, cs *Codespace, path string) ([]byte, error) { - req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+cs.RepositoryNWO+"/contents/"+path, nil) +func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Codespace, path string) ([]byte, error) { + req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+codespace.RepositoryNWO+"/contents/"+path, nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } q := req.URL.Query() - q.Add("ref", cs.Branch) + q.Add("ref", codespace.Branch) req.URL.RawQuery = q.Encode() a.setHeaders(req) diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 15499c738..ab013409b 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -15,12 +15,12 @@ type logger interface { Println(v ...interface{}) (int, error) } -func connectionReady(cs *api.Codespace) bool { - return cs.Environment.Connection.SessionID != "" && - cs.Environment.Connection.SessionToken != "" && - cs.Environment.Connection.RelayEndpoint != "" && - cs.Environment.Connection.RelaySAS != "" && - cs.Environment.State == api.CodespaceEnvironmentStateAvailable +func connectionReady(codespace *api.Codespace) bool { + return codespace.Environment.Connection.SessionID != "" && + codespace.Environment.Connection.SessionToken != "" && + codespace.Environment.Connection.RelayEndpoint != "" && + codespace.Environment.Connection.RelaySAS != "" && + codespace.Environment.State == api.CodespaceEnvironmentStateAvailable } type apiClient interface { @@ -30,17 +30,17 @@ type apiClient interface { // ConnectToLiveshare waits for a Codespace to become running, // and connects to it using a Live Share session. -func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, cs *api.Codespace) (*liveshare.Session, error) { +func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, codespace *api.Codespace) (*liveshare.Session, error) { var startedCodespace bool - if cs.Environment.State != api.CodespaceEnvironmentStateAvailable { + if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { startedCodespace = true log.Print("Starting your codespace...") - if err := apiClient.StartCodespace(ctx, cs.Name); err != nil { + if err := apiClient.StartCodespace(ctx, codespace.Name); err != nil { return nil, fmt.Errorf("error starting codespace: %w", err) } } - for retries := 0; !connectionReady(cs); retries++ { + for retries := 0; !connectionReady(codespace); retries++ { if retries > 1 { if retries%2 == 0 { log.Print(".") @@ -54,7 +54,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, cs } var err error - cs, err = apiClient.GetCodespace(ctx, cs.Name, true) + codespace, err = apiClient.GetCodespace(ctx, codespace.Name, true) if err != nil { return nil, fmt.Errorf("error getting codespace: %w", err) } @@ -67,10 +67,10 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, cs log.Println("Connecting to your codespace...") return liveshare.Connect(ctx, liveshare.Options{ - SessionID: cs.Environment.Connection.SessionID, - SessionToken: cs.Environment.Connection.SessionToken, - RelaySAS: cs.Environment.Connection.RelaySAS, - RelayEndpoint: cs.Environment.Connection.RelayEndpoint, - HostPublicKeys: cs.Environment.Connection.HostPublicKeys, + SessionID: codespace.Environment.Connection.SessionID, + SessionToken: codespace.Environment.Connection.SessionToken, + RelaySAS: codespace.Environment.Connection.RelaySAS, + RelayEndpoint: codespace.Environment.Connection.RelayEndpoint, + HostPublicKeys: codespace.Environment.Connection.HostPublicKeys, }) } diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 9a1204f4a..e8f197410 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -36,8 +36,8 @@ type PostCreateState struct { // PollPostCreateStates watches for state changes in a codespace, // and calls the supplied poller for each batch of state changes. // It runs until it encounters an error, including cancellation of the context. -func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, cs *api.Codespace, poller func([]PostCreateState)) (err error) { - session, err := ConnectToLiveshare(ctx, log, apiClient, cs) +func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient, codespace *api.Codespace, poller func([]PostCreateState)) (err error) { + session, err := ConnectToLiveshare(ctx, log, apiClient, codespace) if err != nil { return fmt.Errorf("connect to Live Share: %w", err) } diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index b4de5faf6..d304f8d0d 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -135,9 +135,9 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) ( // getOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty. // It then fetches the codespace record with full connection details. -func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceName string) (cs *api.Codespace, err error) { +func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceName string) (codespace *api.Codespace, err error) { if codespaceName == "" { - cs, err = chooseCodespace(ctx, apiClient) + codespace, err = chooseCodespace(ctx, apiClient) if err != nil { if err == errNoCodespaces { return nil, err @@ -145,13 +145,13 @@ func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceNam return nil, fmt.Errorf("choosing codespace: %w", err) } } else { - cs, err = apiClient.GetCodespace(ctx, codespaceName, true) + codespace, err = apiClient.GetCodespace(ctx, codespaceName, true) if err != nil { return nil, fmt.Errorf("getting full codespace details: %w", err) } } - return cs, nil + return codespace, nil } func safeClose(closer io.Closer, err *error) { diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 97bd0af7d..a558f4c67 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -107,7 +107,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { // showStatus polls the codespace for a list of post create states and their status. It will keep polling // until all states have finished. Once all states have finished, we poll once more to check if any new // states have been introduced and stop polling otherwise. -func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, user *api.User, cs *api.Codespace) error { +func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, user *api.User, codespace *api.Codespace) error { var lastState codespaces.PostCreateState var breakNextState bool @@ -156,7 +156,7 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient apiClient, us } } - err := codespaces.PollPostCreateStates(ctx, log, apiClient, cs, poller) + err := codespaces.PollPostCreateStates(ctx, log, apiClient, codespace, poller) if err != nil { if errors.Is(err, context.Canceled) && breakNextState { return nil // we cancelled the context to stop polling, we can ignore the error diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index ee5436c0a..2d526dca3 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -75,12 +75,12 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { nameFilter = c.Name } } else { - cs, err := a.apiClient.GetCodespace(ctx, nameFilter, false) + codespace, err := a.apiClient.GetCodespace(ctx, nameFilter, false) if err != nil { return fmt.Errorf("error fetching codespace information: %w", err) } - codespaces = []*api.Codespace{cs} + codespaces = []*api.Codespace{codespace} } codespacesToDelete := make([]*api.Codespace, 0, len(codespaces)) From b44474c32b1e2e155aacce3e812d56ebd0ea8159 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 6 Oct 2021 13:47:20 -0400 Subject: [PATCH 8/8] Revert rename for ports cmd --- pkg/cmd/codespace/ports.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go index 709b900bf..c36f46078 100644 --- a/pkg/cmd/codespace/ports.go +++ b/pkg/cmd/codespace/ports.go @@ -114,10 +114,10 @@ type portAttribute struct { Label string `json:"label"` } -func getDevContainer(ctx context.Context, apiClient apiClient, cs *api.Codespace) <-chan devContainerResult { +func getDevContainer(ctx context.Context, apiClient apiClient, codespace *api.Codespace) <-chan devContainerResult { ch := make(chan devContainerResult, 1) go func() { - contents, err := apiClient.GetCodespaceRepositoryContents(ctx, cs, ".devcontainer/devcontainer.json") + contents, err := apiClient.GetCodespaceRepositoryContents(ctx, codespace, ".devcontainer/devcontainer.json") if err != nil { ch <- devContainerResult{nil, fmt.Errorf("error getting content: %w", err)} return