diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index eb3a4306a..66370305c 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -61,13 +61,20 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { t.AddField(fmt.Sprintf("gh %s", c.Name()), nil, nil) t.AddField(repo, nil, nil) + version := c.CurrentVersion() + if !c.IsBinary() && len(version) > 8 { + version = version[:8] + } - if c.Pin() != "" { - t.AddField(c.Pin(), nil, cs.Cyan) - } else if c.UpdateAvailable() { - t.AddField("Upgrade available", nil, cs.Green) + if c.IsPinned() { + t.AddField(version, nil, cs.Cyan) } else { - t.AddField("", nil, nil) + t.AddField(version, nil, nil) + } + + var updateAvailable string + if c.UpdateAvailable() { + updateAvailable = "Upgrade available" } t.EndRow() } diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 1c7e4dd6d..541b1b137 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -299,7 +299,7 @@ func TestNewCmdExtension(t *testing.T) { assert.Equal(t, 1, len(em.ListCalls())) } }, - wantStdout: "gh test\tcli/gh-test\t\ngh test2\tcli/gh-test2\tUpgrade available\n", + wantStdout: "gh test\tcli/gh-test\t1\t\ngh test2\tcli/gh-test2\t1\tUpgrade available\n", }, { name: "create extension interactive", diff --git a/pkg/cmd/extension/extension.go b/pkg/cmd/extension/extension.go index 87cd20c78..e4f109d83 100644 --- a/pkg/cmd/extension/extension.go +++ b/pkg/cmd/extension/extension.go @@ -18,7 +18,7 @@ type Extension struct { path string url string isLocal bool - pin string + isPinned bool currentVersion string latestVersion string kind ExtensionKind @@ -40,12 +40,16 @@ func (e *Extension) IsLocal() bool { return e.isLocal } -func (e *Extension) Pin() string { - return e.pin +func (e *Extension) CurrentVersion() string { + return e.currentVersion +} + +func (e *Extension) IsPinned() bool { + return e.isPinned } func (e *Extension) UpdateAvailable() bool { - if e.pin != "" || + if e.isPinned || e.isLocal || e.currentVersion == "" || e.latestVersion == "" || diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index b6067fb7d..9452c2b53 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -198,9 +198,13 @@ func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo) (Extension, error) { remoteURL := ghrepo.GenerateRepoURL(repo, "") ext.url = remoteURL ext.currentVersion = bm.Tag +<<<<<<< HEAD if bm.Pinned { ext.pin = bm.Tag } +======= + ext.isPinned = bm.IsPinned +>>>>>>> tmp return ext, nil } @@ -210,10 +214,17 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { remoteUrl := m.getRemoteUrl(fi.Name()) currentVersion := m.getCurrentVersion(fi.Name()) +<<<<<<< HEAD var pinnedVersion string pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) if _, err := os.Stat(pinPath); err == nil && len(currentVersion) > 6 { pinnedVersion = currentVersion[:7] +======= + var isPinned bool + pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) + if _, err := os.Stat(pinPath); err == nil { + isPinned = true +>>>>>>> tmp } return Extension{ @@ -222,7 +233,11 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { isLocal: false, currentVersion: currentVersion, kind: GitKind, +<<<<<<< HEAD pin: pinnedVersion, +======= + isPinned: isPinned, +>>>>>>> tmp }, nil } @@ -324,11 +339,19 @@ func (m *Manager) InstallLocal(dir string) error { } type binManifest struct { +<<<<<<< HEAD Owner string Name string Host string Tag string Pinned bool +======= + Owner string + Name string + Host string + Tag string + IsPinned bool +>>>>>>> tmp // TODO I may end up not using this; just thinking ahead to local installs Path string } @@ -400,12 +423,21 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error { } manifest := binManifest{ +<<<<<<< HEAD Name: name, Owner: repo.RepoOwner(), Host: repo.RepoHost(), Path: binPath, Tag: r.Tag, Pinned: isPinned, +======= + Name: name, + Owner: repo.RepoOwner(), + Host: repo.RepoHost(), + Path: binPath, + Tag: r.Tag, + IsPinned: isPinned, +>>>>>>> tmp } bs, err := yaml.Marshal(manifest) @@ -469,7 +501,11 @@ func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stder pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { +<<<<<<< HEAD return fmt.Errorf("failed to create pin in directory: %w", err) +======= + return fmt.Errorf("failed to create pin file in directory: %w", err) +>>>>>>> tmp } return f.Close() } @@ -533,7 +569,11 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.isLocal { return localExtensionUpgradeError } +<<<<<<< HEAD if ext.pin != "" { +======= + if ext.IsPinned() { +>>>>>>> tmp return pinnedExtensionUpgradeError } if !ext.UpdateAvailable() { diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go index fc954b19e..c596509a6 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -90,34 +90,36 @@ func closeRun(opts *CloseOptions) error { if opts.DeleteBranch { branchSwitchString := "" apiClient := api.NewClientFromHTTP(httpClient) + localBranchExists := git.HasLocalBranch(pr.HeadRefName) if opts.DeleteLocalBranch { - currentBranch, err := opts.Branch() - if err != nil { - return err - } - - var branchToSwitchTo string - if currentBranch == pr.HeadRefName { - branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) - if err != nil { - return err - } - err = git.CheckoutBranch(branchToSwitchTo) - if err != nil { - return err - } - } - - localBranchExists := git.HasLocalBranch(pr.HeadRefName) if localBranchExists { + currentBranch, err := opts.Branch() + if err != nil { + return err + } + + var branchToSwitchTo string + if currentBranch == pr.HeadRefName { + branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) + if err != nil { + return err + } + err = git.CheckoutBranch(branchToSwitchTo) + if err != nil { + return err + } + } + if err := git.DeleteLocalBranch(pr.HeadRefName); err != nil { return fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err) } - } - if branchToSwitchTo != "" { - branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo)) + if branchToSwitchTo != "" { + branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo)) + } + } else { + fmt.Fprintf(opts.IO.ErrOut, "%s Skipped deleting the local branch since current directory is not a git repository \n", cs.WarningIcon()) } } diff --git a/pkg/cmd/pr/close/close_test.go b/pkg/cmd/pr/close/close_test.go index a91431243..f0fdc7354 100644 --- a/pkg/cmd/pr/close/close_test.go +++ b/pkg/cmd/pr/close/close_test.go @@ -230,3 +230,37 @@ func TestPrClose_deleteBranch_sameBranch(t *testing.T) { ✓ Deleted branch trunk and switched to branch main `), output.Stderr()) } + +func TestPrClose_deleteBranch_notInGitRepo(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + baseRepo, pr := stubPR("OWNER/REPO:main", "OWNER/REPO:trunk") + pr.Title = "The title of the PR" + shared.RunCommandFinder("96", pr, baseRepo) + + http.Register( + httpmock.GraphQL(`mutation PullRequestClose\b`), + httpmock.GraphQLMutation(`{"id": "THE-ID"}`, + func(inputs map[string]interface{}) { + assert.Equal(t, inputs["pullRequestId"], "THE-ID") + }), + ) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/trunk"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --verify refs/heads/trunk`, 128, "could not determine current branch: fatal: not a git repository (or any of the parent directories): .git") + + output, err := runCommand(http, true, `96 --delete-branch`) + assert.NoError(t, err) + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Closed pull request #96 (The title of the PR) + ! Skipped deleting the local branch since current directory is not a git repository + ✓ Deleted branch trunk + `), output.Stderr()) +} diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 2db8ab0fb..aa1019a70 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -19,7 +19,8 @@ type Extension interface { Name() string // Extension Name without gh- Path() string // Path to executable URL() string - Pin() string // Pinned version + CurrentVersion() string + IsPinned() bool UpdateAvailable() bool IsBinary() bool IsLocal() bool diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index 9d47d1c4f..db7f50a00 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -17,12 +17,18 @@ var _ Extension = &ExtensionMock{} // // // make and configure a mocked Extension // mockedExtension := &ExtensionMock{ +// CurrentVersionFunc: func() string { +// panic("mock out the CurrentVersion method") +// }, // IsBinaryFunc: func() bool { // panic("mock out the IsBinary method") // }, // IsLocalFunc: func() bool { // panic("mock out the IsLocal method") // }, +// IsPinnedFunc: func() bool { +// panic("mock out the IsPinned method") +// }, // NameFunc: func() string { // panic("mock out the Name method") // }, @@ -45,12 +51,18 @@ var _ Extension = &ExtensionMock{} // // } type ExtensionMock struct { + // CurrentVersionFunc mocks the CurrentVersion method. + CurrentVersionFunc func() string + // IsBinaryFunc mocks the IsBinary method. IsBinaryFunc func() bool // IsLocalFunc mocks the IsLocal method. IsLocalFunc func() bool + // IsPinnedFunc mocks the IsPinned method. + IsPinnedFunc func() bool + // NameFunc mocks the Name method. NameFunc func() string @@ -68,12 +80,18 @@ type ExtensionMock struct { // calls tracks calls to the methods. calls struct { + // CurrentVersion holds details about calls to the CurrentVersion method. + CurrentVersion []struct { + } // IsBinary holds details about calls to the IsBinary method. IsBinary []struct { } // IsLocal holds details about calls to the IsLocal method. IsLocal []struct { } + // IsPinned holds details about calls to the IsPinned method. + IsPinned []struct { + } // Name holds details about calls to the Name method. Name []struct { } @@ -90,8 +108,10 @@ type ExtensionMock struct { UpdateAvailable []struct { } } + lockCurrentVersion sync.RWMutex lockIsBinary sync.RWMutex lockIsLocal sync.RWMutex + lockIsPinned sync.RWMutex lockName sync.RWMutex lockPath sync.RWMutex lockPin sync.RWMutex @@ -99,6 +119,32 @@ type ExtensionMock struct { lockUpdateAvailable sync.RWMutex } +// CurrentVersion calls CurrentVersionFunc. +func (mock *ExtensionMock) CurrentVersion() string { + if mock.CurrentVersionFunc == nil { + panic("ExtensionMock.CurrentVersionFunc: method is nil but Extension.CurrentVersion was just called") + } + callInfo := struct { + }{} + mock.lockCurrentVersion.Lock() + mock.calls.CurrentVersion = append(mock.calls.CurrentVersion, callInfo) + mock.lockCurrentVersion.Unlock() + return mock.CurrentVersionFunc() +} + +// CurrentVersionCalls gets all the calls that were made to CurrentVersion. +// Check the length with: +// len(mockedExtension.CurrentVersionCalls()) +func (mock *ExtensionMock) CurrentVersionCalls() []struct { +} { + var calls []struct { + } + mock.lockCurrentVersion.RLock() + calls = mock.calls.CurrentVersion + mock.lockCurrentVersion.RUnlock() + return calls +} + // IsBinary calls IsBinaryFunc. func (mock *ExtensionMock) IsBinary() bool { if mock.IsBinaryFunc == nil { @@ -151,6 +197,32 @@ func (mock *ExtensionMock) IsLocalCalls() []struct { return calls } +// IsPinned calls IsPinnedFunc. +func (mock *ExtensionMock) IsPinned() bool { + if mock.IsPinnedFunc == nil { + panic("ExtensionMock.IsPinnedFunc: method is nil but Extension.IsPinned was just called") + } + callInfo := struct { + }{} + mock.lockIsPinned.Lock() + mock.calls.IsPinned = append(mock.calls.IsPinned, callInfo) + mock.lockIsPinned.Unlock() + return mock.IsPinnedFunc() +} + +// IsPinnedCalls gets all the calls that were made to IsPinned. +// Check the length with: +// len(mockedExtension.IsPinnedCalls()) +func (mock *ExtensionMock) IsPinnedCalls() []struct { +} { + var calls []struct { + } + mock.lockIsPinned.RLock() + calls = mock.calls.IsPinned + mock.lockIsPinned.RUnlock() + return calls +} + // Name calls NameFunc. func (mock *ExtensionMock) Name() string { if mock.NameFunc == nil {