Repository: cli/cli
Pull request #10627 (2025-03-18 06:23:52)

fix: `gh gist edit` panic when no file in a gist
Fix https://github.com/cli/cli/issues/10626.


phanen: @BagToad Two test added, both can fail on `cli:truck` and pass in this PR.

Patch:
diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go
index e0d6e3b84..9e74adf62 100644
--- a/pkg/cmd/gist/edit/edit.go
+++ b/pkg/cmd/gist/edit/edit.go
@@ -236,6 +236,8 @@ func editRun(opts *EditOptions) error {
 		if filename == "" {
 			if len(candidates) == 1 {
 				filename = candidates[0]
+			} else if len(candidates) == 0 {
+				return errors.New("no file in the gist")
 			} else {
 				if !opts.IO.CanPrompt() {
 					return errors.New("unsure what file to edit; either specify --filename or run interactively")
diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go
index fc63f56ce..99a5524ee 100644
--- a/pkg/cmd/gist/shared/shared.go
+++ b/pkg/cmd/gist/shared/shared.go
@@ -44,6 +44,9 @@ func (g Gist) Filename() string {
 	for fn := range g.Files {
 		filenames = append(filenames, fn)
 	}
+	if len(filenames) == 0 {
+		return ""
+	}
 	sort.Strings(filenames)
 	return filenames[0]
 }
diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go
index 0b86d5e11..7188df1a5 100644
--- a/pkg/cmd/pr/list/list.go
+++ b/pkg/cmd/pr/list/list.go
@@ -64,6 +64,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
 			# List PRs authored by you
 			$ gh pr list --author "@me"
 
+			# List PRs with a specific head branch name
+			$ gh pr list --head "typo"
+
 			# List only PRs with all of the given labels
 			$ gh pr list --label bug --label "priority 1"
 
@@ -102,7 +105,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
 	cmd.Flags().IntVarP(&opts.LimitResults, "limit", "L", 30, "Maximum number of items to fetch")
 	cmdutil.StringEnumFlag(cmd, &opts.State, "state", "s", "open", []string{"open", "closed", "merged", "all"}, "Filter by state")
 	cmd.Flags().StringVarP(&opts.BaseBranch, "base", "B", "", "Filter by base branch")
-	cmd.Flags().StringVarP(&opts.HeadBranch, "head", "H", "", "Filter by head branch")
+	cmd.Flags().StringVarP(&opts.HeadBranch, "head", "H", "", `Filter by head branch ("<owner>:<branch>" syntax not supported)`)
 	cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Filter by label")
 	cmd.Flags().StringVarP(&opts.Author, "author", "A", "", "Filter by author")
 	cmd.Flags().StringVar(&appAuthor, "app", "", "Filter by GitHub App author")


Test patch:
diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go
index 728f6e894..12cdf8169 100644
--- a/pkg/cmd/gist/edit/edit_test.go
+++ b/pkg/cmd/gist/edit/edit_test.go
@@ -557,6 +557,30 @@ func Test_editRun(t *testing.T) {
 			},
 			wantErr: "gist ID or URL required when not running interactively",
 		},
+		{
+			name: "edit no-file gist (#10626)",
+			opts: &EditOptions{
+				Selector: "1234",
+			},
+			mockGist: &shared.Gist{
+				ID:    "1234",
+				Files: map[string]*shared.GistFile{},
+				Owner: &shared.GistOwner{Login: "octocat"},
+			},
+			wantErr: "no file in the gist",
+		},
+		{
+			name: "edit no-file gist, nil map (#10626)",
+			opts: &EditOptions{
+				Selector: "1234",
+			},
+			mockGist: &shared.Gist{
+				ID:    "1234",
+				Files: nil,
+				Owner: &shared.GistOwner{Login: "octocat"},
+			},
+			wantErr: "no file in the gist",
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/pkg/cmd/gist/shared/shared_test.go b/pkg/cmd/gist/shared/shared_test.go
index 15b14a939..0bc1e1f11 100644
--- a/pkg/cmd/gist/shared/shared_test.go
+++ b/pkg/cmd/gist/shared/shared_test.go
@@ -163,6 +163,33 @@ func TestPromptGists(t *testing.T) {
 			response: `{ "data": { "viewer": { "gists": { "nodes": [] } } } }`,
 			wantOut:  Gist{},
 		},
+		{
+			name: "prompt list contains no-file gist (#10626)",
+			prompterStubs: func(pm *prompter.MockPrompter) {
+				pm.RegisterSelect("Select a gist",
+					[]string{"  about 6 hours ago", "gistfile0.txt  about 6 hours ago"},
+					func(_, _ string, opts []string) (int, error) {
+						return prompter.IndexFor(opts, "  about 6 hours ago")
+					})
+			},
+			response: `{ "data": { "viewer": { "gists": { "nodes": [
+							{
+								"name": "1234",
+								"files": [],
+								"description": "",
+								"updatedAt": "%[1]v",
+								"isPublic": true
+							},
+							{
+								"name": "5678",
+								"files": [{ "name": "gistfile0.txt" }],
+								"description": "",
+								"updatedAt": "%[1]v",
+								"isPublic": true
+							}
+						] } } } }`,
+			wantOut: Gist{ID: "1234", Files: map[string]*GistFile{}, UpdatedAt: sixHoursAgo, Public: true},
+		},
 	}
 
 	ios, _, _, _ := iostreams.Test()
diff --git a/pkg/cmd/run/shared/presentation_test.go b/pkg/cmd/run/shared/presentation_test.go
new file mode 100644
index 000000000..f91e38907
--- /dev/null
+++ b/pkg/cmd/run/shared/presentation_test.go
@@ -0,0 +1,398 @@
+package shared
+
+import (
+	"testing"
+	"time"
+
+	"github.com/MakeNowJust/heredoc"
+	"github.com/cli/cli/v2/pkg/iostreams"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRenderJobs(t *testing.T) {
+	startedAt, err := time.Parse(time.RFC3339, "2009-03-19T00:00:00Z")
+	require.NoError(t, err)
+	completedAt, err := time.Parse(time.RFC3339, "2009-03-19T00:01:00Z")
+	require.NoError(t, err)
+
+	tests := []struct {
+		name        string
+		jobs        []Job
+		wantVerbose string
+		wantNormal  string
+		wantCompact string
+	}{
+		{
+			name: "nil jobs",
+			jobs: nil,
+		},
+		{
+			name: "empty jobs",
+			jobs: []Job{},
+		},
+		{
+			// This is not a real-world case, but nevertheless the code should
+			// be able to handle that without error/panic.
+			name: "in-progress job without steps",
+			jobs: []Job{
+				{
+					Name:      "foo",
+					ID:        999,
+					StartedAt: startedAt,
+					Status:    InProgress,
+				},
+			},
+			wantCompact: heredoc.Doc(`
+				* foo (ID 999)`),
+			wantNormal: heredoc.Doc(`
+				* foo (ID 999)`),
+			wantVerbose: heredoc.Doc(`
+				* foo (ID 999)`),
+		},
+		{
+			// This is not a real-world case, but nevertheless the code should
+			// be able to handle that without error/panic.
+			name: "successful job without steps",
+			jobs: []Job{
+				{
+					Name:        "foo",
+					ID:          999,
+					StartedAt:   startedAt,
+					CompletedAt: completedAt,
+					Status:      Completed,
+					Conclusion:  Success,
+				},
+			},
+			wantCompact: heredoc.Doc(`
+				✓ foo in 1m0s (ID 999)`),
+			wantNormal: heredoc.Doc(`
+				✓ foo in 1m0s (ID 999)`),
+			wantVerbose: heredoc.Doc(`
+				✓ foo in 1m0s (ID 999)`),
+		},
+		{
+			// This is not a real-world case, but nevertheless the code should
+			// be able to handle that without error/panic.
+			name: "failed job without steps",
+			jobs: []Job{
+				{
+					Name:        "foo",
+					ID:          999,
+					StartedAt:   startedAt,
+					CompletedAt: completedAt,
+					Status:      Completed,
+					Conclusion:  Failure,
+				},
+			},
+			wantCompact: heredoc.Doc(`
+				X foo in 1m0s (ID 999)`),
+			wantNormal: heredoc.Doc(`
+				X foo in 1m0s (ID 999)`),
+			wantVerbose: heredoc.Doc(`
+				X foo in 1m0s (ID 999)`),
+		},
+		{
+			name: "in-progress job with various step status values",
+			jobs: []Job{
+				{
+					Name:      "foo",
+					ID:        999,
+					StartedAt: startedAt,
+					Status:    InProgress,
+					Steps: []Step{
+						{
+							Name:        "passed",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      1,
+						},
+						{
+							Name:       "skipped",
+							Status:     Completed,
+							Conclusion: Skipped,
+							Number:     2,
+						},
+						{
+							Name:        "failed 1",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Failure,
+							Number:      3,
+						},
+						{
+							Name:        "failed 2",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Failure,
+							Number:      4,
+						},
+						{
+							Name:      "in-progress",
+							StartedAt: startedAt,
+							Status:    InProgress,
+							Number:    5,
+						},
+						{
+							Name:   "pending",
+							Status: Pending,
+							Number: 6,
+						},
+					},
+				},
+			},
+			wantCompact: heredoc.Doc(`
+				* foo (ID 999)
+				  X failed 1
+				  X failed 2
+				  * in-progress`),
+			wantNormal: heredoc.Doc(`
+				* foo (ID 999)`),
+			wantVerbose: heredoc.Doc(`
+				* foo (ID 999)
+				  ✓ passed
+				  - skipped
+				  X failed 1
+				  X failed 2
+				  * in-progress
+				  * pending`),
+		},
+		{
+			// As of my observations (babakks) when there is a failed step, the
+			// job run is marked as failed. In other words, a successful job run
+			// cannot have any failed steps. That's why there's no failed steps
+			// in this test case.
+			name: "successful job with various step status values",
+			jobs: []Job{
+				{
+					Name:        "foo",
+					ID:          999,
+					StartedAt:   startedAt,
+					CompletedAt: completedAt,
+					Status:      Completed,
+					Conclusion:  Success,
+					Steps: []Step{
+						{
+							Name:        "passed 1",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      1,
+						},
+						{
+							Name:       "skipped",
+							Status:     Completed,
+							Conclusion: Skipped,
+							Number:     2,
+						},
+						{
+							Name:        "passed 2",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      3,
+						},
+					},
+				},
+			},
+			wantCompact: heredoc.Doc(`
+				✓ foo in 1m0s (ID 999)`),
+			wantNormal: heredoc.Doc(`
+				✓ foo in 1m0s (ID 999)`),
+			wantVerbose: heredoc.Doc(`
+				✓ foo in 1m0s (ID 999)
+				  ✓ passed 1
+				  - skipped
+				  ✓ passed 2`),
+		},
+		{
+			name: "failed job with various step status values",
+			jobs: []Job{
+				{
+					Name:        "foo",
+					ID:          999,
+					StartedAt:   startedAt,
+					CompletedAt: completedAt,
+					Status:      Completed,
+					Conclusion:  Failure,
+					Steps: []Step{
+						{
+							Name:        "passed 1",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      1,
+						},
+						{
+							Name:       "skipped",
+							Status:     Completed,
+							Conclusion: Skipped,
+							Number:     2,
+						},
+						{
+							Name:        "failed 1",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Failure,
+							Number:      3,
+						},
+						{
+							Name:        "failed 2",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Failure,
+							Number:      4,
+						},
+						{
+							Name:        "passed 2",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      5,
+						},
+					},
+				},
+			},
+			wantCompact: heredoc.Doc(`
+				X foo in 1m0s (ID 999)
+				  X failed 1
+				  X failed 2`),
+			wantNormal: heredoc.Doc(`
+				X foo in 1m0s (ID 999)
+				  ✓ passed 1
+				  - skipped
+				  X failed 1
+				  X failed 2
+				  ✓ passed 2`),
+			wantVerbose: heredoc.Doc(`
+				X foo in 1m0s (ID 999)
+				  ✓ passed 1
+				  - skipped
+				  X failed 1
+				  X failed 2
+				  ✓ passed 2`),
+		},
+		{
+			name: "multiple jobs",
+			jobs: []Job{
+				{
+					Name:      "in-progress",
+					ID:        999,
+					StartedAt: startedAt,
+					Status:    InProgress,
+					Steps: []Step{
+						{
+							Name:        "passed",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      1,
+						},
+						{
+							Name:      "in-progress",
+							StartedAt: startedAt,
+							Status:    InProgress,
+							Number:    2,
+						},
+					},
+				},
+				{
+					Name:        "successful",
+					ID:          999,
+					StartedAt:   startedAt,
+					CompletedAt: completedAt,
+					Status:      Completed,
+					Conclusion:  Success,
+					Steps: []Step{
+						{
+							Name:        "passed",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      1,
+						},
+						{
+							Name:       "skipped",
+							Status:     Completed,
+							Conclusion: Skipped,
+							Number:     2,
+						},
+					},
+				},
+				{
+					Name:        "failed",
+					ID:          999,
+					StartedAt:   startedAt,
+					CompletedAt: completedAt,
+					Status:      Completed,
+					Conclusion:  Failure,
+					Steps: []Step{
+						{
+							Name:        "passed",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Success,
+							Number:      1,
+						},
+						{
+							Name:        "failed",
+							StartedAt:   startedAt,
+							CompletedAt: completedAt,
+							Status:      Completed,
+							Conclusion:  Failure,
+							Number:      2,
+						},
+					},
+				},
+			},
+			wantCompact: heredoc.Doc(`
+				* in-progress (ID 999)
+				  * in-progress
+				✓ successful in 1m0s (ID 999)
+				X failed in 1m0s (ID 999)
+				  X failed`),
+			wantNormal: heredoc.Doc(`
+				* in-progress (ID 999)
+				✓ successful in 1m0s (ID 999)
+				X failed in 1m0s (ID 999)
+				  ✓ passed
+				  X failed`),
+			wantVerbose: heredoc.Doc(`
+				* in-progress (ID 999)
+				  ✓ passed
+				  * in-progress
+				✓ successful in 1m0s (ID 999)
+				  ✓ passed
+				  - skipped
+				X failed in 1m0s (ID 999)
+				  ✓ passed
+				  X failed`),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotCompact := RenderJobsCompact(&iostreams.ColorScheme{}, tt.jobs)
+			assert.Equal(t, tt.wantCompact, gotCompact, "unexpected compact mode output")
+
+			gotNormal := RenderJobs(&iostreams.ColorScheme{}, tt.jobs, false)
+			assert.Equal(t, tt.wantNormal, gotNormal, "unexpected normal mode output")
+
+			gotVerbose := RenderJobs(&iostreams.ColorScheme{}, tt.jobs, true)
+			assert.Equal(t, tt.wantVerbose, gotVerbose, "unexpected verbose mode output")
+		})
+	}
+}