From 3bb6983b3503686250792f85a18651a806b8d478 Mon Sep 17 00:00:00 2001 From: naman <1977419+metalogical@users.noreply.github.com> Date: Wed, 10 Jun 2020 11:41:44 -0700 Subject: [PATCH 1/3] fix regression in support for detached HEAD state for gh pr status --- command/pr.go | 2 +- context/context.go | 6 +++++- git/git.go | 5 ++++- git/git_test.go | 5 ++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/command/pr.go b/command/pr.go index e92fa7cf0..f1c1eb101 100644 --- a/command/pr.go +++ b/command/pr.go @@ -116,7 +116,7 @@ func prStatus(cmd *cobra.Command, args []string) error { repoOverride, _ := cmd.Flags().GetString("repo") currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo) - if err != nil && repoOverride == "" && err.Error() != "git: not on any branch" { + if err != nil && repoOverride == "" && err != git.ErrNotOnAnyBranch { return fmt.Errorf("could not query for pull request for current branch: %w", err) } diff --git a/context/context.go b/context/context.go index 236f9e722..4f1aa10ba 100644 --- a/context/context.go +++ b/context/context.go @@ -207,7 +207,11 @@ func (c *fsContext) Branch() (string, error) { } currentBranch, err := git.CurrentBranch() - if err != nil { + switch err { + case nil: + case git.ErrNotOnAnyBranch: + return "", err + default: return "", fmt.Errorf("could not determine current branch: %w", err) } diff --git a/git/git.go b/git/git.go index 355f905f0..ff40adde3 100644 --- a/git/git.go +++ b/git/git.go @@ -13,6 +13,9 @@ import ( "github.com/cli/cli/internal/run" ) +// ErrNotOnAnyBranch indicates that the users is in detached HEAD state +var ErrNotOnAnyBranch = errors.New("git: not on any branch") + // Ref represents a git commit reference type Ref struct { Hash string @@ -64,7 +67,7 @@ func CurrentBranch() (string, error) { if errors.As(err, &cmdErr) { if cmdErr.Stderr.Len() == 0 { // Detached head - return "", errors.New("git: not on any branch") + return "", ErrNotOnAnyBranch } } diff --git a/git/git_test.go b/git/git_test.go index c84023e40..1a8f32677 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -67,9 +67,8 @@ func Test_CurrentBranch_detached_head(t *testing.T) { if err == nil { t.Errorf("expected an error") } - expectedError := "git: not on any branch" - if err.Error() != expectedError { - t.Errorf("got unexpected error: %s instead of %s", err.Error(), expectedError) + if err != ErrNotOnAnyBranch { + t.Errorf("got unexpected error: %s instead of %s", err, ErrNotOnAnyBranch) } if len(cs.Calls) != 1 { t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) From fb7b01b40c3353015ec9b706d68b8626b356ec6d Mon Sep 17 00:00:00 2001 From: naman <1977419+metalogical@users.noreply.github.com> Date: Thu, 11 Jun 2020 17:05:25 -0700 Subject: [PATCH 2/3] resolve PR review --- command/pr.go | 2 +- command/pr_test.go | 32 ++++++++++++++++++++++++++++++++ context/blank_context.go | 3 ++- context/context.go | 6 +----- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/command/pr.go b/command/pr.go index f1c1eb101..10a41246d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -116,7 +116,7 @@ func prStatus(cmd *cobra.Command, args []string) error { repoOverride, _ := cmd.Flags().GetString("repo") currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo) - if err != nil && repoOverride == "" && err != git.ErrNotOnAnyBranch { + if err != nil && repoOverride == "" && !errors.Is(err, git.ErrNotOnAnyBranch) { return fmt.Errorf("could not query for pull request for current branch: %w", err) } diff --git a/command/pr_test.go b/command/pr_test.go index a11c60820..ee90a66b9 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -299,6 +299,38 @@ Requesting a code review from you } } +func TestPRStatus_detachedHead(t *testing.T) { + initBlankContext("", "OWNER/REPO", "") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": {} } + `)) + + output, err := RunCommand("pr status") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := ` +Relevant pull requests in OWNER/REPO + +Current branch + There is no current branch + +Created by you + You have no open pull requests + +Requesting a code review from you + You have no pull requests to review + +` + if output.String() != expected { + t.Errorf("expected %q, got %q", expected, output.String()) + } +} + func TestPRList(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() diff --git a/context/blank_context.go b/context/blank_context.go index 3ea657abe..7bb5282ec 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -18,6 +18,7 @@ func NewBlank() *blankContext { type blankContext struct { authToken string branch string + branchErr error baseRepo ghrepo.Interface remotes Remotes } @@ -40,7 +41,7 @@ func (c *blankContext) SetAuthToken(t string) { func (c *blankContext) Branch() (string, error) { if c.branch == "" { - return "", fmt.Errorf("branch was not initialized") + return "", fmt.Errorf("branch was not initialized: %w", git.ErrNotOnAnyBranch) } return c.branch, nil } diff --git a/context/context.go b/context/context.go index 4f1aa10ba..236f9e722 100644 --- a/context/context.go +++ b/context/context.go @@ -207,11 +207,7 @@ func (c *fsContext) Branch() (string, error) { } currentBranch, err := git.CurrentBranch() - switch err { - case nil: - case git.ErrNotOnAnyBranch: - return "", err - default: + if err != nil { return "", fmt.Errorf("could not determine current branch: %w", err) } From fffa39368356e870f70ff051518c1814b9df7f6a Mon Sep 17 00:00:00 2001 From: naman <1977419+metalogical@users.noreply.github.com> Date: Thu, 11 Jun 2020 17:08:18 -0700 Subject: [PATCH 3/3] fix lint --- context/blank_context.go | 1 - 1 file changed, 1 deletion(-) diff --git a/context/blank_context.go b/context/blank_context.go index 7bb5282ec..80a2cd298 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -18,7 +18,6 @@ func NewBlank() *blankContext { type blankContext struct { authToken string branch string - branchErr error baseRepo ghrepo.Interface remotes Remotes }