Skip to content

Commit 6f978dd

Browse files
josebaliusjosebalius
authored
Merge pull request cli#4440 from cli/jg/choose-codespace-prompt
codespace: choose codespace prompt improvements
2 parents 3d0d95c + b44474c commit 6f978dd

File tree

5 files changed

+103
-32
lines changed

5 files changed

+103
-32
lines changed

‎internal/codespaces/api/api.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ type CreateCodespaceParams struct {
425425
func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams) (*Codespace, error) {
426426
codespace, err := a.startCreate(ctx, params.RepositoryID, params.Machine, params.Branch, params.Location)
427427
if err != errProvisioningInProgress {
428-
return codespace, err
428+
return nil, err
429429
}
430430

431431
// errProvisioningInProgress indicates that codespace creation did not complete

‎internal/codespaces/states.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient,
7878
return fmt.Errorf("connection failed: %w", err)
7979

8080
case <-t.C:
81-
states, err := getPostCreateOutput(ctx, localPort, codespace, sshUser)
81+
states, err := getPostCreateOutput(ctx, localPort, sshUser)
8282
if err != nil {
8383
return fmt.Errorf("get post create output: %w", err)
8484
}
@@ -88,7 +88,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient apiClient,
8888
}
8989
}
9090

91-
func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Codespace, user string) ([]PostCreateState, error) {
91+
func getPostCreateOutput(ctx context.Context, tunnelPort int, user string) ([]PostCreateState, error) {
9292
cmd, err := NewRemoteCommand(
9393
ctx, tunnelPort, fmt.Sprintf("%s@localhost", user),
9494
"cat /workspaces/.codespaces/shared/postCreateOutput.json",

‎pkg/cmd/codespace/common.go‎

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"io"
1010
"os"
1111
"sort"
12+
"strings"
1213

1314
"github.com/AlecAivazis/survey/v2"
1415
"github.com/AlecAivazis/survey/v2/terminal"
@@ -55,6 +56,8 @@ func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace,
5556
return chooseCodespaceFromList(ctx, codespaces)
5657
}
5758

59+
// chooseCodespaceFromList returns the selected codespace from the list,
60+
// or an error if there are no codespaces.
5861
func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) (*api.Codespace, error) {
5962
if len(codespaces) == 0 {
6063
return nil, errNoCodespaces
@@ -64,14 +67,47 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) (
6467
return codespaces[i].CreatedAt > codespaces[j].CreatedAt
6568
})
6669

67-
codespacesByName := make(map[string]*api.Codespace)
70+
type codespaceWithIndex struct {
71+
cs codespace
72+
idx int
73+
}
74+
75+
namesWithConflict := make(map[string]bool)
76+
codespacesByName := make(map[string]codespaceWithIndex)
6877
codespacesNames := make([]string, 0, len(codespaces))
69-
for _, codespace := range codespaces {
70-
codespacesByName[codespace.Name] = codespace
71-
codespacesNames = append(codespacesNames, codespace.Name)
78+
for _, apiCodespace := range codespaces {
79+
cs := codespace{apiCodespace}
80+
csName := cs.displayName(false, false)
81+
displayNameWithGitStatus := cs.displayName(false, true)
82+
83+
_, hasExistingConflict := namesWithConflict[csName]
84+
if seenCodespace, ok := codespacesByName[csName]; ok || hasExistingConflict {
85+
// There is an existing codespace on the repo and branch.
86+
// We need to disambiguate by adding the codespace name
87+
// to the existing entry and the one we are processing now.
88+
if !hasExistingConflict {
89+
fullDisplayName := seenCodespace.cs.displayName(true, false)
90+
fullDisplayNameWithGitStatus := seenCodespace.cs.displayName(true, true)
91+
92+
codespacesByName[fullDisplayName] = codespaceWithIndex{seenCodespace.cs, seenCodespace.idx}
93+
codespacesNames[seenCodespace.idx] = fullDisplayNameWithGitStatus
94+
delete(codespacesByName, csName) // delete the existing map entry with old name
95+
96+
// All other codespaces with the same name should update
97+
// to their specific name, this tracks conflicting names going forward
98+
namesWithConflict[csName] = true
99+
}
100+
101+
// update this codespace names to include the name to disambiguate
102+
csName = cs.displayName(true, false)
103+
displayNameWithGitStatus = cs.displayName(true, true)
104+
}
105+
106+
codespacesByName[csName] = codespaceWithIndex{cs, len(codespacesNames)}
107+
codespacesNames = append(codespacesNames, displayNameWithGitStatus)
72108
}
73109

74-
sshSurvey := []*survey.Question{
110+
csSurvey := []*survey.Question{
75111
{
76112
Name: "codespace",
77113
Prompt: &survey.Select{
@@ -86,12 +122,15 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace) (
86122
var answers struct {
87123
Codespace string
88124
}
89-
if err := ask(sshSurvey, &answers); err != nil {
125+
if err := ask(csSurvey, &answers); err != nil {
90126
return nil, fmt.Errorf("error getting answers: %w", err)
91127
}
92128

93-
codespace := codespacesByName[answers.Codespace]
94-
return codespace, nil
129+
// Codespaces are indexed without the git status included as compared
130+
// to how it is displayed in the prompt, so the git status symbol needs
131+
// cleaning up in case it is included.
132+
selectedCodespace := strings.Replace(answers.Codespace, gitStatusDirty, "", -1)
133+
return codespacesByName[selectedCodespace].cs.Codespace, nil
95134
}
96135

97136
// 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 {
171210
}
172211
return nil
173212
}
213+
214+
type codespace struct {
215+
*api.Codespace
216+
}
217+
218+
// displayName returns the repository nwo and branch.
219+
// If includeName is true, the name of the codespace is included.
220+
// If includeGitStatus is true, the branch will include a star if
221+
// the codespace has unsaved changes.
222+
func (c codespace) displayName(includeName, includeGitStatus bool) string {
223+
branch := c.Branch
224+
if includeGitStatus {
225+
branch = c.branchWithGitStatus()
226+
}
227+
228+
if includeName {
229+
return fmt.Sprintf(
230+
"%s: %s [%s]", c.RepositoryNWO, branch, c.Name,
231+
)
232+
}
233+
return c.RepositoryNWO + ": " + branch
234+
}
235+
236+
// gitStatusDirty represents an unsaved changes status.
237+
const gitStatusDirty = "*"
238+
239+
// branchWithGitStatus returns the branch with a star
240+
// if the branch is currently being worked on.
241+
func (c codespace) branchWithGitStatus() string {
242+
if c.hasUnsavedChanges() {
243+
return c.Branch + gitStatusDirty
244+
}
245+
246+
return c.Branch
247+
}
248+
249+
// hasUnsavedChanges returns whether the environment has
250+
// unsaved changes.
251+
func (c codespace) hasUnsavedChanges() bool {
252+
return c.Environment.GitStatus.HasUncommitedChanges || c.Environment.GitStatus.HasUnpushedChanges
253+
}

‎pkg/cmd/codespace/delete.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,15 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) {
142142
return nil
143143
}
144144

145-
func confirmDeletion(p prompter, codespace *api.Codespace, isInteractive bool) (bool, error) {
146-
gs := codespace.Environment.GitStatus
147-
hasUnsavedChanges := gs.HasUncommitedChanges || gs.HasUnpushedChanges
148-
if !hasUnsavedChanges {
145+
func confirmDeletion(p prompter, apiCodespace *api.Codespace, isInteractive bool) (bool, error) {
146+
cs := codespace{apiCodespace}
147+
if !cs.hasUnsavedChanges() {
149148
return true, nil
150149
}
151150
if !isInteractive {
152-
return false, fmt.Errorf("codespace %s has unsaved changes (use --force to override)", codespace.Name)
151+
return false, fmt.Errorf("codespace %s has unsaved changes (use --force to override)", cs.Name)
153152
}
154-
return p.Confirm(fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", codespace.Name))
153+
return p.Confirm(fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", cs.Name))
155154
}
156155

157156
type surveyPrompter struct{}

‎pkg/cmd/codespace/list.go‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"os"
77

8-
"github.com/cli/cli/v2/internal/codespaces/api"
98
"github.com/cli/cli/v2/pkg/cmd/codespace/output"
109
"github.com/spf13/cobra"
1110
)
@@ -35,24 +34,17 @@ func (a *App) List(ctx context.Context, asJSON bool) error {
3534

3635
table := output.NewTable(os.Stdout, asJSON)
3736
table.SetHeader([]string{"Name", "Repository", "Branch", "State", "Created At"})
38-
for _, codespace := range codespaces {
37+
for _, apiCodespace := range codespaces {
38+
cs := codespace{apiCodespace}
3939
table.Append([]string{
40-
codespace.Name,
41-
codespace.RepositoryNWO,
42-
codespace.Branch + dirtyStar(codespace.Environment.GitStatus),
43-
codespace.Environment.State,
44-
codespace.CreatedAt,
40+
cs.Name,
41+
cs.RepositoryNWO,
42+
cs.branchWithGitStatus(),
43+
cs.Environment.State,
44+
cs.CreatedAt,
4545
})
4646
}
4747

4848
table.Render()
4949
return nil
5050
}
51-
52-
func dirtyStar(status api.CodespaceEnvironmentGitStatus) string {
53-
if status.HasUncommitedChanges || status.HasUnpushedChanges {
54-
return "*"
55-
}
56-
57-
return ""
58-
}

0 commit comments

Comments
 (0)