diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 531b2b3f8..6fd9e4d18 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -425,7 +425,7 @@ type CreateCodespaceParams struct { func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams) (*Codespace, error) { codespace, err := a.startCreate(ctx, params.RepositoryID, params.Machine, params.Branch, params.Location) if err != errProvisioningInProgress { - return codespace, err + return nil, err } // errProvisioningInProgress indicates that codespace creation did not complete diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 14fdd0b10..e8f197410 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -78,7 +78,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) } @@ -88,7 +88,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 4dae92a90..d304f8d0d 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -9,6 +9,7 @@ import ( "io" "os" "sort" + "strings" "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/terminal" @@ -55,6 +56,8 @@ func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, return chooseCodespaceFromList(ctx, codespaces) } +// chooseCodespaceFromList returns the selected codespace from the list, +// or an error if there are no codespaces. func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) (*api.Codespace, error) { if len(codespaces) == 0 { return nil, errNoCodespaces @@ -64,14 +67,47 @@ 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 + idx int } - sshSurvey := []*survey.Question{ + namesWithConflict := make(map[string]bool) + codespacesByName := make(map[string]codespaceWithIndex) + codespacesNames := make([]string, 0, len(codespaces)) + 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 { + // 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 + + // 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) + displayNameWithGitStatus = cs.displayName(true, true) + } + + codespacesByName[csName] = codespaceWithIndex{cs, len(codespacesNames)} + codespacesNames = append(codespacesNames, displayNameWithGitStatus) + } + + csSurvey := []*survey.Question{ { Name: "codespace", Prompt: &survey.Select{ @@ -86,12 +122,15 @@ 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] - return codespace, nil + // 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, gitStatusDirty, "", -1) + return codespacesByName[selectedCodespace].cs.Codespace, nil } // getOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty. @@ -171,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/delete.go b/pkg/cmd/codespace/delete.go index 36bee15b7..2d526dca3 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -142,16 +142,15 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err 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, apiCodespace *api.Codespace, isInteractive bool) (bool, error) { + cs := codespace{apiCodespace} + 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/list.go b/pkg/cmd/codespace/list.go index 9164b7921..4bc649bf7 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" ) @@ -35,24 +34,17 @@ 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.Branch + dirtyStar(codespace.Environment.GitStatus), - codespace.Environment.State, - codespace.CreatedAt, + cs.Name, + cs.RepositoryNWO, + cs.branchWithGitStatus(), + cs.Environment.State, + cs.CreatedAt, }) } table.Render() return nil } - -func dirtyStar(status api.CodespaceEnvironmentGitStatus) string { - if status.HasUncommitedChanges || status.HasUnpushedChanges { - return "*" - } - - return "" -}