Merge pull request #4440 from cli/jg/choose-codespace-prompt

codespace: choose codespace prompt improvements

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

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",

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
+}

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{}

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 ""
-}