Simplify codespace picker (#5826)
This doesn't change anything about how it's formatted, but extracts a new function formatCodespacesForSelect so we can test and see how it treats the different combinations of codespaces. Co-authored-by: Mislav Marohnić <mislav@github.com>
This commit is contained in:
Greggory Rothmeier
GitHub
parent
01bff6b91e
commit
9b86fe41c0
2 changed files with 186 additions and 132 deletions
|
|
@ -10,7 +10,6 @@ import (
|
|||
"log"
|
||||
"os"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/AlecAivazis/survey/v2"
|
||||
"github.com/AlecAivazis/survey/v2/terminal"
|
||||
|
|
@ -133,111 +132,48 @@ 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.
|
||||
// chooseCodespaceFromList returns the codespace that the user has interactively selected 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
|
||||
}
|
||||
|
||||
sort.Slice(codespaces, func(i, j int) bool {
|
||||
return codespaces[i].CreatedAt > codespaces[j].CreatedAt
|
||||
sortedCodespaces := codespaces
|
||||
sort.Slice(sortedCodespaces, func(i, j int) bool {
|
||||
return sortedCodespaces[i].CreatedAt > sortedCodespaces[j].CreatedAt
|
||||
})
|
||||
|
||||
type codespaceWithIndex struct {
|
||||
cs codespace
|
||||
idx int
|
||||
}
|
||||
|
||||
namesWithConflict := make(map[string]bool)
|
||||
codespacesByName := make(map[string]codespaceWithIndex)
|
||||
codespacesNames := make([]string, 0, len(codespaces))
|
||||
codespacesDirty := make(map[string]bool)
|
||||
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
|
||||
|
||||
// Update the git status dirty map to reflect the new name.
|
||||
if seenCodespace.cs.hasUnsavedChanges() {
|
||||
codespacesDirty[fullDisplayNameWithGitStatus] = true
|
||||
}
|
||||
|
||||
// delete the existing map entry with old name
|
||||
delete(codespacesByName, csName)
|
||||
|
||||
// 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)
|
||||
|
||||
if cs.hasUnsavedChanges() {
|
||||
codespacesDirty[displayNameWithGitStatus] = true
|
||||
}
|
||||
}
|
||||
|
||||
csSurvey := []*survey.Question{
|
||||
{
|
||||
Name: "codespace",
|
||||
Prompt: &survey.Select{
|
||||
Message: "Choose codespace:",
|
||||
Options: codespacesNames,
|
||||
Default: codespacesNames[0],
|
||||
Options: formatCodespacesForSelect(sortedCodespaces),
|
||||
},
|
||||
Validate: survey.Required,
|
||||
},
|
||||
}
|
||||
|
||||
var answers struct {
|
||||
Codespace string
|
||||
Codespace int
|
||||
}
|
||||
if err := ask(csSurvey, &answers); err != nil {
|
||||
return nil, fmt.Errorf("error getting answers: %w", err)
|
||||
}
|
||||
|
||||
// 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 := answers.Codespace
|
||||
isDirty := codespacesDirty[selectedCodespace]
|
||||
if isDirty {
|
||||
selectedCodespace = withoutGitStatus(answers.Codespace)
|
||||
}
|
||||
return codespacesByName[selectedCodespace].cs.Codespace, nil
|
||||
return sortedCodespaces[answers.Codespace], nil
|
||||
}
|
||||
|
||||
// withoutGitStatus returns the string without the git status symbol.
|
||||
func withoutGitStatus(cname string) string {
|
||||
return replaceLastOccurence(cname, gitStatusDirty, "")
|
||||
}
|
||||
func formatCodespacesForSelect(codespaces []*api.Codespace) []string {
|
||||
names := make([]string, len(codespaces))
|
||||
|
||||
// replaceLastOccurence replaces the last occurence of a string with another string.
|
||||
func replaceLastOccurence(str, old, replace string) string {
|
||||
i := strings.LastIndex(str, old)
|
||||
if i == -1 {
|
||||
return str
|
||||
for i, apiCodespace := range codespaces {
|
||||
cs := codespace{apiCodespace}
|
||||
names[i] = cs.displayName()
|
||||
}
|
||||
return str[:i] + replace + str[i+len(old):]
|
||||
|
||||
return names
|
||||
}
|
||||
|
||||
// getOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty.
|
||||
|
|
@ -339,29 +275,14 @@ type codespace struct {
|
|||
*api.Codespace
|
||||
}
|
||||
|
||||
// displayName returns the repository nwo and branch.
|
||||
// If includeName is true, the name of the codespace (including displayName) 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.GitStatus.Ref
|
||||
if includeGitStatus {
|
||||
branch = c.branchWithGitStatus()
|
||||
// displayName formats the codespace name for the interactive selector prompt.
|
||||
func (c codespace) displayName() string {
|
||||
branch := c.branchWithGitStatus()
|
||||
displayName := c.DisplayName
|
||||
if displayName == "" {
|
||||
displayName = c.Name
|
||||
}
|
||||
|
||||
if includeName {
|
||||
var displayName = c.Name
|
||||
if c.DisplayName != "" {
|
||||
displayName = c.DisplayName
|
||||
}
|
||||
return fmt.Sprintf(
|
||||
"%s: %s (%s)", c.Repository.FullName, displayName, branch,
|
||||
)
|
||||
}
|
||||
return fmt.Sprintf(
|
||||
"%s: %s", c.Repository.FullName, branch,
|
||||
)
|
||||
|
||||
return fmt.Sprintf("%s (%s): %s", c.Repository.FullName, branch, displayName)
|
||||
}
|
||||
|
||||
// gitStatusDirty represents an unsaved changes status.
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package codespace
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/v2/internal/codespaces/api"
|
||||
|
|
@ -10,14 +11,9 @@ func Test_codespace_displayName(t *testing.T) {
|
|||
type fields struct {
|
||||
Codespace *api.Codespace
|
||||
}
|
||||
type args struct {
|
||||
includeName bool
|
||||
includeGitStatus bool
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
fields fields
|
||||
args args
|
||||
want string
|
||||
}{
|
||||
{
|
||||
|
|
@ -33,11 +29,7 @@ func Test_codespace_displayName(t *testing.T) {
|
|||
DisplayName: "scuba steve",
|
||||
},
|
||||
},
|
||||
args: args{
|
||||
includeName: false,
|
||||
includeGitStatus: false,
|
||||
},
|
||||
want: "cli/cli: trunk",
|
||||
want: "cli/cli (trunk): scuba steve",
|
||||
},
|
||||
{
|
||||
name: "No included name - included gitstatus - no unsaved changes",
|
||||
|
|
@ -52,11 +44,7 @@ func Test_codespace_displayName(t *testing.T) {
|
|||
DisplayName: "scuba steve",
|
||||
},
|
||||
},
|
||||
args: args{
|
||||
includeName: false,
|
||||
includeGitStatus: true,
|
||||
},
|
||||
want: "cli/cli: trunk",
|
||||
want: "cli/cli (trunk): scuba steve",
|
||||
},
|
||||
{
|
||||
name: "No included name - included gitstatus - unsaved changes",
|
||||
|
|
@ -72,11 +60,7 @@ func Test_codespace_displayName(t *testing.T) {
|
|||
DisplayName: "scuba steve",
|
||||
},
|
||||
},
|
||||
args: args{
|
||||
includeName: false,
|
||||
includeGitStatus: true,
|
||||
},
|
||||
want: "cli/cli: trunk*",
|
||||
want: "cli/cli (trunk*): scuba steve",
|
||||
},
|
||||
{
|
||||
name: "Included name - included gitstatus - unsaved changes",
|
||||
|
|
@ -92,11 +76,7 @@ func Test_codespace_displayName(t *testing.T) {
|
|||
DisplayName: "scuba steve",
|
||||
},
|
||||
},
|
||||
args: args{
|
||||
includeName: true,
|
||||
includeGitStatus: true,
|
||||
},
|
||||
want: "cli/cli: scuba steve (trunk*)",
|
||||
want: "cli/cli (trunk*): scuba steve",
|
||||
},
|
||||
{
|
||||
name: "Included name - included gitstatus - no unsaved changes",
|
||||
|
|
@ -112,11 +92,7 @@ func Test_codespace_displayName(t *testing.T) {
|
|||
DisplayName: "scuba steve",
|
||||
},
|
||||
},
|
||||
args: args{
|
||||
includeName: true,
|
||||
includeGitStatus: true,
|
||||
},
|
||||
want: "cli/cli: scuba steve (trunk)",
|
||||
want: "cli/cli (trunk): scuba steve",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
|
|
@ -124,9 +100,166 @@ func Test_codespace_displayName(t *testing.T) {
|
|||
c := codespace{
|
||||
Codespace: tt.fields.Codespace,
|
||||
}
|
||||
if got := c.displayName(tt.args.includeName, tt.args.includeGitStatus); got != tt.want {
|
||||
if got := c.displayName(); got != tt.want {
|
||||
t.Errorf("codespace.displayName() = %v, want %v", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_formatCodespacesForSelect(t *testing.T) {
|
||||
type args struct {
|
||||
codespaces []*api.Codespace
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
wantCodespacesNames []string
|
||||
}{
|
||||
{
|
||||
name: "One codespace: Shows only repo and branch name",
|
||||
args: args{
|
||||
codespaces: []*api.Codespace{
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "scuba steve",
|
||||
},
|
||||
},
|
||||
},
|
||||
wantCodespacesNames: []string{
|
||||
"cli/cli (trunk): scuba steve",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Two codespaces on the same repo/branch: Adds the codespace's display name",
|
||||
args: args{
|
||||
codespaces: []*api.Codespace{
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "scuba steve",
|
||||
},
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "flappy bird",
|
||||
},
|
||||
},
|
||||
},
|
||||
wantCodespacesNames: []string{
|
||||
"cli/cli (trunk): scuba steve",
|
||||
"cli/cli (trunk): flappy bird",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Two codespaces on the different branches: Shows only repo and branch name",
|
||||
args: args{
|
||||
codespaces: []*api.Codespace{
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "scuba steve",
|
||||
},
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "feature",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "flappy bird",
|
||||
},
|
||||
},
|
||||
},
|
||||
wantCodespacesNames: []string{
|
||||
"cli/cli (trunk): scuba steve",
|
||||
"cli/cli (feature): flappy bird",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Two codespaces on the different repos: Shows only repo and branch name",
|
||||
args: args{
|
||||
codespaces: []*api.Codespace{
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "github/cli",
|
||||
},
|
||||
DisplayName: "scuba steve",
|
||||
},
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "flappy bird",
|
||||
},
|
||||
},
|
||||
},
|
||||
wantCodespacesNames: []string{
|
||||
"github/cli (trunk): scuba steve",
|
||||
"cli/cli (trunk): flappy bird",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Two codespaces on the same repo/branch, one dirty: Adds the codespace's display name and *",
|
||||
args: args{
|
||||
codespaces: []*api.Codespace{
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "scuba steve",
|
||||
},
|
||||
{
|
||||
GitStatus: api.CodespaceGitStatus{
|
||||
Ref: "trunk",
|
||||
HasUncommitedChanges: true,
|
||||
},
|
||||
Repository: api.Repository{
|
||||
FullName: "cli/cli",
|
||||
},
|
||||
DisplayName: "flappy bird",
|
||||
},
|
||||
},
|
||||
},
|
||||
wantCodespacesNames: []string{
|
||||
"cli/cli (trunk): scuba steve",
|
||||
"cli/cli (trunk*): flappy bird",
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
gotCodespacesNames := formatCodespacesForSelect(tt.args.codespaces)
|
||||
|
||||
if !reflect.DeepEqual(gotCodespacesNames, tt.wantCodespacesNames) {
|
||||
t.Errorf("codespacesNames: got %v, want %v", gotCodespacesNames, tt.wantCodespacesNames)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue