From d79763ffc4ae64d404d522c1972d4d4537236888 Mon Sep 17 00:00:00 2001 From: Yujun Li Date: Thu, 20 Feb 2025 17:27:03 +0800 Subject: [PATCH] fix: missing nil pointer check on PRs and Issues view #527 --- ui/components/issuesidebar/issuesidebar.go | 12 +++++++++++ ui/components/prsidebar/prsidebar.go | 16 ++++++++++++++ ui/components/prssection/checkout.go | 4 ++++ ui/components/prssection/diff.go | 7 +++++- ui/components/prssection/watchChecks.go | 4 ++++ ui/tasks.go | 2 +- ui/ui.go | 25 +++++++++++----------- 7 files changed, 55 insertions(+), 15 deletions(-) diff --git a/ui/components/issuesidebar/issuesidebar.go b/ui/components/issuesidebar/issuesidebar.go index 3db6afa9..e6737d41 100644 --- a/ui/components/issuesidebar/issuesidebar.go +++ b/ui/components/issuesidebar/issuesidebar.go @@ -271,6 +271,10 @@ func (m *Model) shouldCancelComment() bool { } func (m *Model) SetIsCommenting(isCommenting bool) tea.Cmd { + if m.issue == nil { + return nil + } + if !m.isCommenting && isCommenting { m.inputBox.Reset() } @@ -288,6 +292,10 @@ func (m *Model) GetIsAssigning() bool { } func (m *Model) SetIsAssigning(isAssigning bool) tea.Cmd { + if m.issue == nil { + return nil + } + if !m.isAssigning && isAssigning { m.inputBox.Reset() } @@ -317,6 +325,10 @@ func (m *Model) GetIsUnassigning() bool { } func (m *Model) SetIsUnassigning(isUnassigning bool) tea.Cmd { + if m.issue == nil { + return nil + } + if !m.isUnassigning && isUnassigning { m.inputBox.Reset() } diff --git a/ui/components/prsidebar/prsidebar.go b/ui/components/prsidebar/prsidebar.go index 3ab2bea9..0ffdbfe1 100644 --- a/ui/components/prsidebar/prsidebar.go +++ b/ui/components/prsidebar/prsidebar.go @@ -378,6 +378,10 @@ func (m *Model) shouldCancelComment() bool { } func (m *Model) SetIsCommenting(isCommenting bool) tea.Cmd { + if m.pr == nil { + return nil + } + if !m.isCommenting && isCommenting { m.inputBox.Reset() } @@ -399,6 +403,10 @@ func (m *Model) GetIsApproving() bool { } func (m *Model) SetIsApproving(isApproving bool) tea.Cmd { + if m.pr == nil { + return nil + } + if !m.isApproving && isApproving { m.inputBox.Reset() } @@ -417,6 +425,10 @@ func (m *Model) GetIsAssigning() bool { } func (m *Model) SetIsAssigning(isAssigning bool) tea.Cmd { + if m.pr == nil { + return nil + } + if !m.isAssigning && isAssigning { m.inputBox.Reset() } @@ -446,6 +458,10 @@ func (m *Model) GetIsUnassigning() bool { } func (m *Model) SetIsUnassigning(isUnassigning bool) tea.Cmd { + if m.pr == nil { + return nil + } + if !m.isUnassigning && isUnassigning { m.inputBox.Reset() } diff --git a/ui/components/prssection/checkout.go b/ui/components/prssection/checkout.go index 05c12417..a4b23fa3 100644 --- a/ui/components/prssection/checkout.go +++ b/ui/components/prssection/checkout.go @@ -15,6 +15,10 @@ import ( func (m *Model) checkout() (tea.Cmd, error) { pr := m.GetCurrRow() + if pr == nil { + return nil, errors.New("No pr selected") + } + repoName := pr.GetRepoNameWithOwner() repoPath, ok := common.GetRepoLocalPath(repoName, m.Ctx.Config.RepoPaths) diff --git a/ui/components/prssection/diff.go b/ui/components/prssection/diff.go index 3066c23b..d96f577c 100644 --- a/ui/components/prssection/diff.go +++ b/ui/components/prssection/diff.go @@ -9,11 +9,16 @@ import ( ) func (m Model) diff() tea.Cmd { + currRowData := m.GetCurrRow() + if currRowData == nil { + return nil + } + c := exec.Command( "gh", "pr", "diff", - fmt.Sprint(m.GetCurrRow().GetNumber()), + fmt.Sprint(currRowData.GetNumber()), "-R", m.GetCurrRow().GetRepoNameWithOwner(), ) diff --git a/ui/components/prssection/watchChecks.go b/ui/components/prssection/watchChecks.go index 6b0c8b09..756fbd31 100644 --- a/ui/components/prssection/watchChecks.go +++ b/ui/components/prssection/watchChecks.go @@ -18,6 +18,10 @@ import ( func (m *Model) watchChecks() tea.Cmd { pr := m.GetCurrRow() + if pr == nil { + return nil + } + prNumber := pr.GetNumber() title := pr.GetTitle() url := pr.GetUrl() diff --git a/ui/tasks.go b/ui/tasks.go index 475af422..52afc718 100644 --- a/ui/tasks.go +++ b/ui/tasks.go @@ -27,7 +27,7 @@ func (m *Model) openBrowser() tea.Cmd { openCmd := func() tea.Msg { b := browser.New("", os.Stdout, os.Stdin) currRow := m.getCurrRowData() - if reflect.ValueOf(currRow).IsNil() { + if currRow == nil || reflect.ValueOf(currRow).IsNil() { return constants.TaskFinishedMsg{TaskId: taskId, Err: errors.New("Current selection doesn't have a URL")} } err := b.Browse(currRow.GetUrl()) diff --git a/ui/ui.go b/ui/ui.go index eacf838d..c8986659 100644 --- a/ui/ui.go +++ b/ui/ui.go @@ -163,6 +163,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { footerCmd tea.Cmd cmds []tea.Cmd currSection = m.getCurrSection() + currRowData = m.getCurrRowData() ) switch msg := msg.(type) { @@ -259,13 +260,12 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } case key.Matches(msg, m.keys.CopyNumber): - row := m.getCurrRowData() var cmd tea.Cmd - if reflect.ValueOf(row).IsNil() { + if currRowData == nil || reflect.ValueOf(currRowData).IsNil() { cmd = m.notifyErr("Current selection isn't associated with a PR/Issue") return m, cmd } - number := fmt.Sprint(row.GetNumber()) + number := fmt.Sprint(currRowData.GetNumber()) err := clipboard.WriteAll(number) if err != nil { cmd = m.notifyErr(fmt.Sprintf("Failed copying to clipboard %v", err)) @@ -276,12 +276,11 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case key.Matches(msg, m.keys.CopyUrl): var cmd tea.Cmd - row := m.getCurrRowData() - if reflect.ValueOf(row).IsNil() { + if currRowData == nil || reflect.ValueOf(currRowData).IsNil() { cmd = m.notifyErr("Current selection isn't associated with a PR/Issue") return m, cmd } - url := row.GetUrl() + url := currRowData.GetUrl() err := clipboard.WriteAll(url) if err != nil { cmd = m.notifyErr(fmt.Sprintf("Failed copying to clipboard %v", err)) @@ -377,35 +376,35 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, cmd case key.Matches(msg, keys.PRKeys.Close): - if currSection != nil { + if currRowData != nil && currSection != nil { currSection.SetPromptConfirmationAction("close") cmd = currSection.SetIsPromptConfirmationShown(true) } return m, cmd case key.Matches(msg, keys.PRKeys.Ready): - if currSection != nil { + if currRowData != nil && currSection != nil { currSection.SetPromptConfirmationAction("ready") cmd = currSection.SetIsPromptConfirmationShown(true) } return m, cmd case key.Matches(msg, keys.PRKeys.Reopen): - if currSection != nil { + if currRowData != nil && currSection != nil { currSection.SetPromptConfirmationAction("reopen") cmd = currSection.SetIsPromptConfirmationShown(true) } return m, cmd case key.Matches(msg, keys.PRKeys.Merge): - if currSection != nil { + if currRowData != nil && currSection != nil { currSection.SetPromptConfirmationAction("merge") cmd = currSection.SetIsPromptConfirmationShown(true) } return m, cmd case key.Matches(msg, keys.PRKeys.Update): - if currSection != nil { + if currRowData != nil && currSection != nil { currSection.SetPromptConfirmationAction("update") cmd = currSection.SetIsPromptConfirmationShown(true) } @@ -455,14 +454,14 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, cmd case key.Matches(msg, keys.IssueKeys.Close): - if currSection != nil { + if currRowData != nil && currSection != nil { currSection.SetPromptConfirmationAction("close") cmd = currSection.SetIsPromptConfirmationShown(true) } return m, cmd case key.Matches(msg, keys.IssueKeys.Reopen): - if currSection != nil { + if currRowData != nil && currSection != nil { currSection.SetPromptConfirmationAction("reopen") cmd = currSection.SetIsPromptConfirmationShown(true) }