diff --git a/docs-master/keybindings/Keybindings_en.md b/docs-master/keybindings/Keybindings_en.md index 0277cb653..2aac3e182 100644 --- a/docs-master/keybindings/Keybindings_en.md +++ b/docs-master/keybindings/Keybindings_en.md @@ -234,6 +234,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Go to next hunk | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit | Commit staged changes. | | `` w `` | Commit changes without pre-commit hook | | @@ -316,6 +317,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Go to next hunk | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit | Commit staged changes. | | `` w `` | Commit changes without pre-commit hook | | diff --git a/docs-master/keybindings/Keybindings_ja.md b/docs-master/keybindings/Keybindings_ja.md index dc4d085ee..d273dbd2e 100644 --- a/docs-master/keybindings/Keybindings_ja.md +++ b/docs-master/keybindings/Keybindings_ja.md @@ -203,6 +203,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 次のハンクに移動 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | サイドパネルに戻る | | | `` c `` | コミット | ステージされた変更をコミットします。 | | `` w `` | pre-commitフックなしで変更をコミット | | @@ -293,6 +294,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 次のハンクに移動 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | サイドパネルに戻る | | | `` c `` | コミット | ステージされた変更をコミットします。 | | `` w `` | pre-commitフックなしで変更をコミット | | diff --git a/docs-master/keybindings/Keybindings_ko.md b/docs-master/keybindings/Keybindings_ko.md index e87332ed2..4c28bfaa6 100644 --- a/docs-master/keybindings/Keybindings_ko.md +++ b/docs-master/keybindings/Keybindings_ko.md @@ -95,6 +95,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 다음 hunk를 선택 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | 커밋 변경내용 | 스테이징된 변경 사항 커밋. | | `` w `` | Commit changes without pre-commit hook | | @@ -188,6 +189,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 다음 hunk를 선택 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | 커밋 변경내용 | 스테이징된 변경 사항 커밋. | | `` w `` | Commit changes without pre-commit hook | | diff --git a/docs-master/keybindings/Keybindings_nl.md b/docs-master/keybindings/Keybindings_nl.md index d130c9531..6fb5291a9 100644 --- a/docs-master/keybindings/Keybindings_nl.md +++ b/docs-master/keybindings/Keybindings_nl.md @@ -242,6 +242,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Selecteer de volgende hunk | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit veranderingen | Commit gestagede wijzigingen. | | `` w `` | Commit veranderingen zonder pre-commit hook | | @@ -316,6 +317,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Selecteer de volgende hunk | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit veranderingen | Commit gestagede wijzigingen. | | `` w `` | Commit veranderingen zonder pre-commit hook | | diff --git a/docs-master/keybindings/Keybindings_pl.md b/docs-master/keybindings/Keybindings_pl.md index 3c191bc8a..46117e5fb 100644 --- a/docs-master/keybindings/Keybindings_pl.md +++ b/docs-master/keybindings/Keybindings_pl.md @@ -110,6 +110,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Idź do następnego fragmentu | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit | Zatwierdź zmiany zatwierdzone. | | `` w `` | Zatwierdź zmiany bez hooka pre-commit | | @@ -211,6 +212,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Idź do następnego fragmentu | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit | Zatwierdź zmiany zatwierdzone. | | `` w `` | Zatwierdź zmiany bez hooka pre-commit | | diff --git a/docs-master/keybindings/Keybindings_pt.md b/docs-master/keybindings/Keybindings_pt.md index 2723f1189..704ccebf4 100644 --- a/docs-master/keybindings/Keybindings_pt.md +++ b/docs-master/keybindings/Keybindings_pt.md @@ -246,6 +246,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Ir para o próximo trecho | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit | Submeter mudanças em staging | | `` w `` | Fazer commit de alterações sem pré-commit | | @@ -325,6 +326,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Ir para o próximo trecho | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Commit | Submeter mudanças em staging | | `` w `` | Fazer commit de alterações sem pré-commit | | diff --git a/docs-master/keybindings/Keybindings_ru.md b/docs-master/keybindings/Keybindings_ru.md index 591b55c3d..33f7da705 100644 --- a/docs-master/keybindings/Keybindings_ru.md +++ b/docs-master/keybindings/Keybindings_ru.md @@ -85,6 +85,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Выбрать следующую часть | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Сохранить изменения | Commit staged changes. | | `` w `` | Закоммитить изменения без предварительного хука коммита | | @@ -110,6 +111,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | Выбрать следующую часть | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | Exit back to side panel | | | `` c `` | Сохранить изменения | Commit staged changes. | | `` w `` | Закоммитить изменения без предварительного хука коммита | | diff --git a/docs-master/keybindings/Keybindings_zh-CN.md b/docs-master/keybindings/Keybindings_zh-CN.md index f51a4f8c0..9b5afed6e 100644 --- a/docs-master/keybindings/Keybindings_zh-CN.md +++ b/docs-master/keybindings/Keybindings_zh-CN.md @@ -281,6 +281,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 选择下一个区块 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | 退出回到侧边面板 | | | `` c `` | 提交变更 | 提交暂存文件 | | `` w `` | 提交变更而无需预先提交钩子 | | @@ -322,6 +323,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 选择下一个区块 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | 退出回到侧边面板 | | | `` c `` | 提交变更 | 提交暂存文件 | | `` w `` | 提交变更而无需预先提交钩子 | | diff --git a/docs-master/keybindings/Keybindings_zh-TW.md b/docs-master/keybindings/Keybindings_zh-TW.md index e04968bef..bee7ce31f 100644 --- a/docs-master/keybindings/Keybindings_zh-TW.md +++ b/docs-master/keybindings/Keybindings_zh-TW.md @@ -70,6 +70,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 選擇下一段 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | 退出回到側邊面板 | | | `` c `` | 提交變更 | 提交暫存區變更 | | `` w `` | 沒有預提交 hook 就提交更改 | | @@ -344,6 +345,7 @@ _This file is auto-generated. To update, make the changes in the pkg/i18n direct | `` , l `` | 選擇下一段 | | | `` N `` | Go to previous file | | | `` n `` | Go to next file | | +| `` G `` | Open pull request at selected line | Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found. | | `` `` | 退出回到側邊面板 | | | `` c `` | 提交變更 | 提交暫存區變更 | | `` w `` | 沒有預提交 hook 就提交更改 | | diff --git a/pkg/gui/context/commit_files_context.go b/pkg/gui/context/commit_files_context.go index 4ad7334ef..ad7d09b4f 100644 --- a/pkg/gui/context/commit_files_context.go +++ b/pkg/gui/context/commit_files_context.go @@ -19,16 +19,36 @@ type CommitFilesContext struct { } var ( - _ types.IListContext = (*CommitFilesContext)(nil) - _ types.DiffableContext = (*CommitFilesContext)(nil) - _ types.IFilterableContext = (*CommitFilesContext)(nil) - _ types.DiffMainViewContext = (*CommitFilesContext)(nil) + _ types.IListContext = (*CommitFilesContext)(nil) + _ types.DiffableContext = (*CommitFilesContext)(nil) + _ types.IFilterableContext = (*CommitFilesContext)(nil) + _ types.DiffMainViewContext = (*CommitFilesContext)(nil) + _ types.PullRequestDiffContext = (*CommitFilesContext)(nil) ) func (self *CommitFilesContext) GetDiffMainViewType() types.DiffMainViewType { return types.DiffMainViewTypePatchBuilding } +// BranchForPullRequest asks the panel this one was entered from: the files are a +// commit's, and which branch's pull request that commit is up for review in is known +// there rather than here. +func (self *CommitFilesContext) BranchForPullRequest() string { + if parent, ok := self.GetParentContext().(types.PullRequestDiffContext); ok { + return parent.BranchForPullRequest() + } + return "" +} + +// CommitsForPullRequest asks the panel this one was entered from as well: the files +// listed here are those of the commits selected there. +func (self *CommitFilesContext) CommitsForPullRequest() ([]*models.Commit, string) { + if parent, ok := self.GetParentContext().(types.PullRequestDiffContext); ok { + return parent.CommitsForPullRequest() + } + return nil, "" +} + func NewCommitFilesContext(c *ContextCommon) *CommitFilesContext { viewModel := filetree.NewCommitFileTreeViewModel( func() []*models.CommitFile { return c.Model().CommitFiles }, diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 8bab33801..ffb64d3f2 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -31,16 +31,70 @@ type commitDropIndicator struct { } var ( - _ types.IListContext = (*LocalCommitsContext)(nil) - _ types.DiffableContext = (*LocalCommitsContext)(nil) - _ types.ISearchableContext = (*LocalCommitsContext)(nil) - _ types.DiffMainViewContext = (*LocalCommitsContext)(nil) + _ types.IListContext = (*LocalCommitsContext)(nil) + _ types.DiffableContext = (*LocalCommitsContext)(nil) + _ types.ISearchableContext = (*LocalCommitsContext)(nil) + _ types.DiffMainViewContext = (*LocalCommitsContext)(nil) + _ types.PullRequestDiffContext = (*LocalCommitsContext)(nil) ) func (self *LocalCommitsContext) GetDiffMainViewType() types.DiffMainViewType { return types.DiffMainViewTypePatchBuilding } +// BranchForPullRequest returns the checked-out branch: this panel shows its commits, +// so a pull request for it is where they are up for review. +func (self *LocalCommitsContext) BranchForPullRequest() string { + return self.ListContextTrait.c.Model().CheckedOutBranch +} + +func (self *LocalCommitsContext) CommitsForPullRequest() ([]*models.Commit, string) { + selectedCommits, _, _ := self.GetSelectedItems() + commits := commitsShownInDiff(selectedCommits, self.GetSelected(), self.GetSelectedRefRangeForDiffFiles()) + return commits, pullRequestBaseForCommits(self.GetCommits(), commits) +} + +// commitsShownInDiff returns the commits whose combined diff a panel listing a branch's +// commits renders into the main view: the selected range where it has a range to diff, +// and the commit at the cursor otherwise. The panel hands the same two to +// DiffHelper.GetUpdateTaskForRenderingCommitsDiff, so anything acting on the diff on +// screen acts on the commits that diff is of. +func commitsShownInDiff( + selectedCommits []*models.Commit, commitAtCursor *models.Commit, refRange *types.RefRange, +) []*models.Commit { + if refRange != nil { + return selectedCommits + } + if commitAtCursor == nil { + return nil + } + return []*models.Commit{commitAtCursor} +} + +// pullRequestBaseForCommits returns the hash of the commit the diff of the given commits +// starts after: the parent of the oldest of them. A pull request holds only the commits +// of the branch that are pushed, so a parent that isn't pushed is none of its own. The +// diff then starts where the pull request itself does, and "" says so. +func pullRequestBaseForCommits(allCommits []*models.Commit, commits []*models.Commit) string { + if len(commits) == 0 { + return "" + } + + oldest := commits[len(commits)-1] + if oldest.IsFirstCommit() { + return "" + } + + parentHash := oldest.Parents()[0] + parentIsInPullRequest := lo.ContainsBy(allCommits, func(commit *models.Commit) bool { + return commit.Hash() == parentHash && commit.Status == models.StatusPushed + }) + if !parentIsInPullRequest { + return "" + } + return parentHash +} + func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext { dropIndicator := &commitDropIndicator{insertionIndex: -1} viewModel := NewLocalCommitsViewModel( diff --git a/pkg/gui/context/local_commits_context_test.go b/pkg/gui/context/local_commits_context_test.go index f93af3a72..44fe6abe8 100644 --- a/pkg/gui/context/local_commits_context_test.go +++ b/pkg/gui/context/local_commits_context_test.go @@ -4,8 +4,12 @@ import ( "testing" "time" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gui/style" + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/stretchr/testify/assert" ) @@ -51,3 +55,101 @@ func TestAddMovingCommitsIndicator(t *testing.T) { }, }, items) } + +func TestCommitsShownInDiff(t *testing.T) { + hashPool := &utils.StringPool{} + newer := models.NewCommit(hashPool, models.NewCommitOpts{Hash: "newer"}) + older := models.NewCommit(hashPool, models.NewCommitOpts{Hash: "older"}) + selected := []*models.Commit{newer, older} + + scenarios := []struct { + name string + selectedCommit *models.Commit + refRange *types.RefRange + expected []*models.Commit + }{ + { + name: "a range is diffed as a whole", + selectedCommit: newer, + refRange: &types.RefRange{From: older, To: newer}, + expected: selected, + }, + { + name: "without a range to diff, only the commit at the cursor is", + selectedCommit: newer, + expected: []*models.Commit{newer}, + }, + { + name: "nothing is diffed while nothing is selected", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, commitsShownInDiff(selected, s.selectedCommit, s.refRange)) + }) + } +} + +func TestPullRequestBaseForCommits(t *testing.T) { + hashPool := &utils.StringPool{} + commit := func(hash string, parent string, status models.CommitStatus) *models.Commit { + return models.NewCommit(hashPool, models.NewCommitOpts{ + Hash: hash, + Parents: lo.Ternary(parent == "", []string{}, []string{parent}), + Status: status, + }) + } + + // A branch of three pushed commits whose tip was amended, on top of a commit that + // is in a main branch already, as the panel lists them: newest first. + amended := commit("amended", "third", models.StatusUnpushed) + third := commit("third", "second", models.StatusPushed) + second := commit("second", "first", models.StatusPushed) + first := commit("first", "merged", models.StatusPushed) + merged := commit("merged", "ancient", models.StatusMerged) + ancient := commit("ancient", "", models.StatusMerged) + allCommits := []*models.Commit{amended, third, second, first, merged, ancient} + + scenarios := []struct { + name string + commits []*models.Commit + expected string + }{ + { + name: "a single commit starts after its parent", + commits: []*models.Commit{second}, + expected: "first", + }, + { + name: "a range starts after the parent of its oldest commit", + commits: []*models.Commit{third, second}, + expected: "first", + }, + { + name: "the pull request's first commit starts where the pull request does", + commits: []*models.Commit{first}, + }, + { + name: "so does a range reaching down to it", + commits: []*models.Commit{third, second, first}, + }, + { + name: "and so does the first commit of the repository", + commits: []*models.Commit{ancient}, + }, + { + name: "a parent the panel doesn't list is none of the pull request's", + commits: []*models.Commit{commit("elsewhere", "unlisted", models.StatusPushed)}, + }, + { + name: "nothing is shown", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, pullRequestBaseForCommits(allCommits, s.commits)) + }) + } +} diff --git a/pkg/gui/context/sub_commits_context.go b/pkg/gui/context/sub_commits_context.go index 14a6e49ca..7649dc1a3 100644 --- a/pkg/gui/context/sub_commits_context.go +++ b/pkg/gui/context/sub_commits_context.go @@ -21,16 +21,33 @@ type SubCommitsContext struct { } var ( - _ types.IListContext = (*SubCommitsContext)(nil) - _ types.DiffableContext = (*SubCommitsContext)(nil) - _ types.ISearchableContext = (*SubCommitsContext)(nil) - _ types.DiffMainViewContext = (*SubCommitsContext)(nil) + _ types.IListContext = (*SubCommitsContext)(nil) + _ types.DiffableContext = (*SubCommitsContext)(nil) + _ types.ISearchableContext = (*SubCommitsContext)(nil) + _ types.DiffMainViewContext = (*SubCommitsContext)(nil) + _ types.PullRequestDiffContext = (*SubCommitsContext)(nil) ) func (self *SubCommitsContext) GetDiffMainViewType() types.DiffMainViewType { return types.DiffMainViewTypePatchBuilding } +// BranchForPullRequest returns the branch this panel was entered from, whose commits it +// shows. The panel is also entered from a tag, a remote branch and the reflog, none of +// which a pull request is made from. +func (self *SubCommitsContext) BranchForPullRequest() string { + if branch, ok := self.GetRef().(*models.Branch); ok { + return branch.Name + } + return "" +} + +func (self *SubCommitsContext) CommitsForPullRequest() ([]*models.Commit, string) { + selectedCommits, _, _ := self.GetSelectedItems() + commits := commitsShownInDiff(selectedCommits, self.GetSelected(), self.GetSelectedRefRangeForDiffFiles()) + return commits, pullRequestBaseForCommits(self.GetCommits(), commits) +} + func NewSubCommitsContext( c *ContextCommon, ) *SubCommitsContext { diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index 6e2f22a1b..1010a8e94 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -1,8 +1,13 @@ package controllers import ( + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" "time" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" @@ -154,6 +159,14 @@ func (self *MainViewController) GetKeybindings(opts types.KeybindingsOpts) []*ty DescriptionFunc: self.diffSelectionDescriptionText(self.c.Tr.NextFileInDiff), GetDisabledReason: self.diffSelectionDisabledReason, }, + { + Keys: opts.GetKeys(opts.Config.Commits.OpenPullRequestInBrowser), + Handler: self.openPullRequestAtSelectedLine, + Description: self.c.Tr.OpenPullRequestAtSelectedLine, + DescriptionFunc: self.pullRequestDescription(self.c.Tr.OpenPullRequestAtSelectedLine), + GetDisabledReason: self.openPullRequestDisabledReason, + Tooltip: self.c.Tr.OpenPullRequestAtSelectedLineTooltip, + }, { Keys: opts.GetKeys(opts.Config.Universal.Return), Handler: self.escape, @@ -439,6 +452,19 @@ func (self *MainViewController) diffActionDescription(staging string, patchBuild } } +// pullRequestDescription describes a command that acts on the pull request of the +// branch the diff belongs to. Over a diff that belongs to no branch (the working tree's, +// a stash entry's) it describes it as nothing; this keeps the command out of the +// keybindings menu there. +func (self *MainViewController) pullRequestDescription(description string) func() string { + return self.diffSelectionDescription(func() string { + if self.pullRequestBranch() == "" { + return "" + } + return description + }) +} + // copySelection copies the selected diff lines to the clipboard — not as the diff // renderer drew them, but as they read in the diff itself, which is both what you meant // to copy and the only form a renderer can't have mangled. A selection that is all @@ -530,6 +556,46 @@ func (self *MainViewController) discardSelectionDisabledReason() *types.Disabled return nil } +// openPullRequestDisabledReason disables opening a line in the pull request where the +// pull request has no view of what is on screen. The branch may have no pull request, +// and the pane may be showing a diff that is not the commit's own: a diff against +// another ref, or the custom patch, whose lines sit at the numbers the patch gives them +// rather than the commit's. +func (self *MainViewController) openPullRequestDisabledReason() *types.DisabledReason { + if reason := self.diffSelectionDisabledReason(); reason != nil { + return reason + } + if self.c.Modes().Diffing.Active() { + return &types.DisabledReason{Text: self.c.Tr.NotAvailableInDiffingMode} + } + if self.c.Helpers().DiffLine.ShowsCustomPatch(self.context.GetView()) { + return &types.DisabledReason{Text: self.c.Tr.NotAvailableForCustomPatch} + } + if reason := self.c.Helpers().Host.NoPullRequestDisabledReason(self.pullRequestBranch()); reason != nil { + return reason + } + return self.commitsOutsidePullRequestDisabledReason() +} + +// commitsOutsidePullRequestDisabledReason disables opening a line of a diff whose +// commits the pull request doesn't hold: it holds the commits of its branch that are on +// the remote, so an unpushed commit is none of its own, and neither is one that is in a +// main branch already and so from before the branch. Asked for such a commit, its pages +// say they can't find it. +func (self *MainViewController) commitsOutsidePullRequestDisabledReason() *types.DisabledReason { + commits, _ := self.pullRequestCommits() + if lo.EveryBy(commits, func(commit *models.Commit) bool { + return commit.Status == models.StatusPushed + }) { + return nil + } + + if len(commits) == 1 { + return &types.DisabledReason{Text: self.c.Tr.CommitNotInPullRequest} + } + return &types.DisabledReason{Text: self.c.Tr.CommitsNotInPullRequest} +} + func (self *MainViewController) onClickInAlreadyFocusedView(opts gocui.ViewMouseBindingOpts) error { self.selectClickedDiffLine(opts.Y) return nil @@ -1051,6 +1117,104 @@ func (self *MainViewController) editDiffLine(viewLine int, beforeEdit func()) er return self.c.Helpers().Files.EditFileAtLine(info.Path, lineNumber) } +// openPullRequestAtSelectedLine opens the pull request of the branch whose commit the +// main view is showing the diff of, at the line the selection is on, so that the line +// can be commented on there. +func (self *MainViewController) openPullRequestAtSelectedLine() error { + pr, ok := self.c.Helpers().Host.PullRequestForBranch(self.pullRequestBranch()) + if !ok { + // Guarded against by the disabled reason, but a refresh in the background may + // have taken the pull request away since it was asked. + return errors.New(self.c.Tr.NoPullRequestForBranch) + } + + commits, baseHash := self.pullRequestCommits() + if len(commits) == 0 { + return nil + } + + view := self.context.GetView() + info, ok := self.c.Helpers().DiffLine.GetDiffLineInfo(view, view.SelectedLineIdx()) + if !ok { + return nil + } + relativePath := repoRelativePath(self.c.Git().RepoPaths.WorktreePath(), info.Path) + if relativePath == "" { + return nil + } + + self.c.LogAction(self.c.Tr.Actions.OpenPullRequest) + url := githubPullRequestLineURL(pr.Url, githubCommitRange(commits, baseHash), relativePath, info) + return self.c.OS().OpenLink(url) +} + +// pullRequestBranch returns the branch whose pull request would show the diff in this +// pane, as the panel beneath names it, and "" where no pull request shows it. +func (self *MainViewController) pullRequestBranch() string { + prContext, ok := self.sidePanelBeneath().(types.PullRequestDiffContext) + if !ok { + return "" + } + return prContext.BranchForPullRequest() +} + +// pullRequestCommits returns the commits whose diff the pane is showing, and the commit +// that diff starts after, as the panel beneath names them. The diff's line numbers are +// the ones the pull request's page for those commits shows. +func (self *MainViewController) pullRequestCommits() ([]*models.Commit, string) { + prContext, ok := self.sidePanelBeneath().(types.PullRequestDiffContext) + if !ok { + return nil, "" + } + return prContext.CommitsForPullRequest() +} + +// githubPullRequestLineURL builds the URL of a line of a file, in the diff a pull request +// shows for the given commits. The file is named by the SHA-256 of its path as git spells +// it, and the line by which side of the diff it is on. +// +// GitHub documents none of this; the form was read off the URLs its own pages carry (see +// https://github.com/orgs/community/discussions/55764). +func githubPullRequestLineURL( + prURL string, commitRange string, relativePath string, info types.DiffLineInfo, +) string { + pathHash := sha256.Sum256([]byte(relativePath)) + anchor := "diff-" + hex.EncodeToString(pathHash[:]) + githubDiffLineSuffix(info) + return fmt.Sprintf("%s/changes/%s#%s", prURL, commitRange, anchor) +} + +// githubCommitRange names the commits a pull request is to show the diff of: a single +// commit by its hash, and a range of them as the commit the diff starts after, then the +// commit it ends at. A range that starts where the pull request itself does names BASE +// as the commit it starts after, the keyword its pages use for the commit the pull +// request was opened against; naming that commit by its hash gets a page that says it +// can't find those commits. +func githubCommitRange(commits []*models.Commit, baseHash string) string { + newest := commits[0].Hash() + if len(commits) == 1 { + return newest + } + if baseHash == "" { + baseHash = "BASE" + } + return baseHash + ".." + newest +} + +// githubDiffLineSuffix names a line within a file's diff: R for the new version of the +// file, L for the old one, which is where a deleted line is found. Some rows are no line +// of the file at all (the header naming it, or a marker like "\ No newline at end of +// file"); those name none, and the anchor points at the file itself. +func githubDiffLineSuffix(info types.DiffLineInfo) string { + switch info.Type { + case types.DiffLineDeleted: + return fmt.Sprintf("L%d", info.OldLine) + case types.DiffLineAdded, types.DiffLineContext, types.DiffLineHunkHeader: + return fmt.Sprintf("R%d", info.NewLine) + default: + return "" + } +} + func (self *MainViewController) openSearch() error { if manager := self.c.GetViewBufferManagerForView(self.context.GetView()); manager != nil { manager.ReadToEnd(func() { diff --git a/pkg/gui/controllers/main_view_controller_test.go b/pkg/gui/controllers/main_view_controller_test.go new file mode 100644 index 000000000..d7ec54f34 --- /dev/null +++ b/pkg/gui/controllers/main_view_controller_test.go @@ -0,0 +1,115 @@ +package controllers + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/stretchr/testify/assert" +) + +func TestGithubPullRequestLineURL(t *testing.T) { + const prURL = "https://github.com/jesseduffield/lazygit/pull/1234" + const commitHash = "1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b" + + // The anchor names the file by the SHA-256 of its repo-relative path, taken over + // exactly those bytes: no leading slash, no trailing newline, forward slashes. + const fileHash = "067980d6efc4249367ceb61b0d93a00bca100a0ddb6d4a72b6dbb0eb9d3825cc" // "dir/file1" + + scenarios := []struct { + name string + path string + info types.DiffLineInfo + expected string + }{ + { + name: "an added line is on the right side of the diff", + path: "dir/file1", + info: types.DiffLineInfo{Type: types.DiffLineAdded, NewLine: 12}, + expected: prURL + "/changes/" + commitHash + "#diff-" + fileHash + "R12", + }, + { + name: "a deleted line is on the left side, at the line it sat on", + path: "dir/file1", + // A deletion's NewLine is only where it sits in the new version of the + // file; the line it is, is the old one. + info: types.DiffLineInfo{Type: types.DiffLineDeleted, NewLine: 12, OldLine: 34}, + expected: prURL + "/changes/" + commitHash + "#diff-" + fileHash + "L34", + }, + { + name: "a context line is on the right side too", + path: "dir/file1", + info: types.DiffLineInfo{Type: types.DiffLineContext, NewLine: 7, OldLine: 5}, + expected: prURL + "/changes/" + commitHash + "#diff-" + fileHash + "R7", + }, + { + name: "a hunk header points at the first line of its hunk", + path: "dir/file1", + info: types.DiffLineInfo{Type: types.DiffLineHunkHeader, NewLine: 20}, + expected: prURL + "/changes/" + commitHash + "#diff-" + fileHash + "R20", + }, + { + name: "the header naming a file points at the file alone", + path: "dir/file1", + info: types.DiffLineInfo{Type: types.DiffLineFileHeader}, + expected: prURL + "/changes/" + commitHash + "#diff-" + fileHash, + }, + { + name: "a row that is no line of the file points at the file alone", + path: "dir/file1", + info: types.DiffLineInfo{Type: types.DiffLineOther}, + expected: prURL + "/changes/" + commitHash + "#diff-" + fileHash, + }, + { + name: "a file at the root of the repo", + path: "file1", + info: types.DiffLineInfo{Type: types.DiffLineAdded, NewLine: 1}, + expected: prURL + "/changes/" + commitHash + + "#diff-c147efcfc2d7ea666a9e4f5187b115c90903f0fc896a56df9a6ef5d8f3fc9f31R1", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, githubPullRequestLineURL(prURL, commitHash, s.path, s.info)) + }) + } +} + +func TestGithubCommitRange(t *testing.T) { + hashPool := &utils.StringPool{} + newest := models.NewCommit(hashPool, models.NewCommitOpts{Hash: "newest"}) + oldest := models.NewCommit(hashPool, models.NewCommitOpts{Hash: "oldest"}) + + scenarios := []struct { + name string + commits []*models.Commit + baseHash string + expected string + }{ + { + name: "a single commit is named on its own", + commits: []*models.Commit{newest}, + baseHash: "parent", + expected: "newest", + }, + { + name: "a range is named as the commits it lies between", + commits: []*models.Commit{newest, oldest}, + baseHash: "parent", + expected: "parent..newest", + }, + { + name: "a range starting where the pull request does lies above BASE", + commits: []*models.Commit{newest, oldest}, + expected: "BASE..newest", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, githubCommitRange(s.commits, s.baseHash)) + }) + } +} diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index ef013b8e9..58b8dbb40 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -1,6 +1,7 @@ package types import ( + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/utils" @@ -213,6 +214,29 @@ const ( DiffMainViewTypePatchBuilding ) +// PullRequestDiffContext is implemented by the side panel contexts that show, in +// their focused main view, the diff of a commit of a branch: the commits panel and +// the sub-commits panel, and the commit files panel entered from either of them. A +// pull request for that branch has a view of that commit, so a line of the diff can +// be pointed at in it. A panel showing a diff that no pull request has a view of (the +// working tree's, a stash entry's, a reflog entry's) doesn't implement this. +type PullRequestDiffContext interface { + Context + + // BranchForPullRequest returns the local branch whose pull request would show + // the diff in the main view, and "" where no branch does: the panel may have + // nothing selected, HEAD may be detached, or what was drilled into may be a tag + // or a remote branch rather than a local one. + BranchForPullRequest() string + + // CommitsForPullRequest returns the commits whose combined diff the main view is + // showing, newest first as the panel lists them, together with the hash of the + // commit that diff starts after: the parent of the oldest of them, where the + // pull request has that parent as one of its own commits, and "" where the diff + // starts where the pull request itself does. + CommitsForPullRequest() ([]*models.Commit, string) +} + // DiffPaneContext is one of the two panes the main section can show, as the thing // that holds a diff with a selection in it. The panels that act on such a selection // are handed the pane it was made in, and speak to it through this. diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 170d1abb1..795e8d289 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -293,6 +293,8 @@ type TranslationSet struct { UnsupportedGitService string CopyPullRequestURL string OpenPullRequestInBrowser string + OpenPullRequestAtSelectedLine string + OpenPullRequestAtSelectedLineTooltip string NoPullRequestForBranch string NoBranchOnRemote string Fetch string @@ -310,6 +312,10 @@ type TranslationSet struct { ToggleSelectHunk string SelectHunk string NothingToSelectInDiff string + NotAvailableInDiffingMode string + NotAvailableForCustomPatch string + CommitNotInPullRequest string + CommitsNotInPullRequest string SelectLineByLine string ToggleSelectHunkTooltip string ToggleSelectionForPatch string @@ -1467,6 +1473,8 @@ func EnglishTranslationSet() *TranslationSet { CreatePullRequest: `Create pull request`, CopyPullRequestURL: `Copy pull request URL to clipboard`, OpenPullRequestInBrowser: `Open pull request in browser`, + OpenPullRequestAtSelectedLine: `Open pull request at selected line`, + OpenPullRequestAtSelectedLineTooltip: "Open the branch's pull request in your browser, at the line the selection is on, so that you can comment on it there. Only pull requests on GitHub are found.", NoPullRequestForBranch: `No pull request found for this branch`, NoBranchOnRemote: `This branch doesn't exist on remote. You need to push it to remote first.`, Fetch: `Fetch`, @@ -1485,6 +1493,10 @@ func EnglishTranslationSet() *TranslationSet { DismissRangeSelect: "Dismiss range select", ToggleSelectHunk: "Toggle hunk selection", NothingToSelectInDiff: "There is nothing to select here", + NotAvailableInDiffingMode: "Not available in diffing mode", + NotAvailableForCustomPatch: "Not available for the custom patch", + CommitNotInPullRequest: "This commit is not part of the pull request", + CommitsNotInPullRequest: "Not all of these commits are part of the pull request", SelectHunk: "Select hunks", SelectLineByLine: "Select line-by-line", ToggleSelectHunkTooltip: "Toggle line-by-line vs. hunk selection mode.", diff --git a/pkg/integration/tests/main_view/open_pull_request_only_over_a_commits_diff.go b/pkg/integration/tests/main_view/open_pull_request_only_over_a_commits_diff.go new file mode 100644 index 000000000..c4d9c11f2 --- /dev/null +++ b/pkg/integration/tests/main_view/open_pull_request_only_over_a_commits_diff.go @@ -0,0 +1,105 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var OpenPullRequestOnlyOverACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Opening a diff line in the pull request is offered over a commit's own diff, and refused over the other diffs the main view shows", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\n") + shell.Commit("second commit") + + shell.UpdateFile("file1", "one\nTWO\nTHREE\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + // The working tree's diff is no commit of a branch, so no pull request has a + // view of it and the command isn't offered there at all. + t.Views().Files(). + Focus(). + SelectedLine(Contains("file1")). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Press(keys.Universal.OptionMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Keybindings")). + Tap(func() { + // The command is bound right below the one asserted here, so a menu + // showing that one would be showing this one too if it had it. + t.Views().Menu(). + Content(Contains("Go to next file")). + Content(DoesNotContain("Open pull request at selected line")) + }). + Cancel() + + // Over a commit's diff it is offered, and says so where the branch has no pull + // request to open. + t.Views().Commits(). + Focus(). + SelectedLine(Contains("second commit")). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + Press(keys.Commits.OpenPullRequestInBrowser) + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("No pull request found for this branch")). + Confirm() + + // The pane previewing the custom patch shows the patch's lines at the numbers + // the patch gives them, which are not the ones the pull request shows. + t.Views().Main(). + IsFocused(). + PressPrimaryAction(). + Press(keys.Universal.TogglePanel) + + t.Views().Secondary(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + Press(keys.Commits.OpenPullRequestInBrowser) + + t.ExpectToast(Contains("Not available for the custom patch")) + + // In diffing mode the main view shows a diff against another ref rather than + // the commit's own, and the pull request has no view of that either. + t.Views().Commits(). + Focus(). + Press(keys.Universal.DiffingMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Diffing")). + Select(MatchesRegexp(`Diff \w+`)). + Confirm() + + t.Views().Commits(). + SelectNextItem(). + SelectedLine(Contains("first commit")). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectionIsActive(). + Press(keys.Commits.OpenPullRequestInBrowser) + + t.ExpectToast(Contains("Not available in diffing mode")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 2402e799f..bfe9340d9 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -442,6 +442,7 @@ var tests = []*components.IntegrationTest{ main_view.NoSelectionOverACommitLog, main_view.NoSelectionOverAConflictHint, main_view.NoSelectionWhenNoChanges, + main_view.OpenPullRequestOnlyOverACommitsDiff, main_view.PatchMarksFollowARendererSwitch, main_view.PatchMarksShowWhileTheDiffIsFocused, main_view.RangeSelectDiffLines,