Show pull requests against branches#2781
Conversation
8357475 to
cd8cfbd
Compare
|
Oooh, very nice! A few thoughts after trying it (very briefly though, so I might be missing a lot of things):
|
It needs remotes to map from PRs back to branches i.e. the PR has an owner e.g. 'jesseduffield') and the remote has a URL (e.g.
No need to apologise, I've been thinking the same thing. I wish there was a way to pass multiple branch heads to gh or to the graphql endpoint (gh is just a way of talking to graphql) but from what I've seen and tried, there's not. We could try a parallelised approach of shelling out to a bunch of gh processes at once but it would probably be rate limited.
I agree, and it shouldn't be hard to support that.
Yep currently it's just for display but I like your idea. |
cd8cfbd to
39bd87d
Compare
134a144 to
319ca04
Compare
319ca04 to
bcb7011
Compare
|
|
||
| func fetchPullRequestsQuery(branches []string, owner string, repo string) string { | ||
| var queries []string | ||
| for i, branch := range branches { |
There was a problem hiding this comment.
You can do this in one request (not sure about the length limits for the query) with:
query {
search(first:3, query:"head:gh-integration-3 head:integration-tests-on-windows repo:jesseduffield/lazygit", type: ISSUE) {
nodes {
... on PullRequest {
url
}
}
}
}which returns:
{
"data": {
"search": {
"nodes": [
{
"url": "https://github.com/jesseduffield/lazygit/pull/2781"
},
{
"url": "https://github.com/jesseduffield/lazygit/pull/2648"
}
]
}
}
}There was a problem hiding this comment.
@dlvhdr Interesting, playing with this now. I'm new to GraphQL and all this stuff.
May I ask a few questions?
- What are some criteria for which is better? When I tried both versions using a list of 10 branches, they both took roughly the same time (a bit over 500ms).
- When you say "one request", then the original version is also just one request (i.e. one http call). It just has a bunch of subqueries, and I couldn't find a lot of information about what these even are or how they work. (But I didn't try hard.) Ok, so this wasn't a question. 😄
- Why does this even work? The github documentation says that when you put multiple things into a search query, it ANDs them together by default, so I would have expected that you need
query:"(head:gh-integration-3 OR head:integration-tests-on-windows) repo:jesseduffield/lazygit". Does github have the DWIM logic to synthesize the right boolean operator based on what field types you search for?
There was a problem hiding this comment.
Oh then I must have been mistaken.. Seems pretty much identical.
Regarding the 3rd point I couldn't find the documentation for it, but I swear it exists.
Anyway if you use https://docs.github.com/en/graphql/overview/explorer and provide this query, it works:
query FetchSponsors {
search(query:"is:pr repo:dlvhdr/gh-dash repo:jesseduffield/lazygit", type:ISSUE, first: 10) {
nodes {
... on PullRequest {
title
repository {
nameWithOwner
}
}
}
}
}Maybe github does an implicit OR if an AND doesn't make sense.
|
Is there a plan to make gh pr usable in LG, or only the external view? I would love to see it allow us to use 'gh pr create/edit/etc'.. if lg could add in support for the gh command for prs... it would be amazing and make it so i dont even have to leave my nvim/lg setup xD |
|
@Daemoen You can easily do this as a customCommand. For example, for creating a PR on the current branch: |
|
Could we show a list of pull requests and their titles? I would actually use this instead of the branch view most of the time. This way I wouldn't need to remember the name of the branch or the PR number. I just need the (human readable) title. This would also be useful for code review. Currently I have to go to GitHub, copy the branch name, and checkout the branch, whereas with this workflow I could just directly checkout the PR inside of lazygit. The branch name is an implementation detail. |
a8edf26 to
843d19a
Compare
dfc47fb to
335cbf7
Compare
335cbf7 to
35fc12e
Compare
35fc12e to
834b0f1
Compare
834b0f1 to
62cf0ac
Compare
62cf0ac to
620ff4b
Compare
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesFootnotes
|
620ff4b to
5d45212
Compare
|
Pushed some more commits. Functionality-wise this is good enough for me to merge (though I would need to do another review of the code) |
|
Nice, good to see some work on this. I was meaning to come back to this myself too, but never found the time. I have been using it locally for a long time now, and love it. However, I'm afraid I don't think I agree it's good enough to merge. The biggest open problems off the top of my head are:
|
Required for authenticating with GitHub's API using the token stored by the gh CLI.
Add GitHubCommands struct with GraphQL-based PR fetching, and GithubPullRequest model. Wire HostingService and GitHub command structs into GitCommand. Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
OnMenuPress can already deal with the selected item being nil, so this allows us to add common code to it that is run when cancelling the menu.
If the user hits escape in the "Select base repository for pull requests" prompt, don't bother them again for this repo at the next refresh.
For the branches panel we might consider unifying it with the existing `o` command for creating a PR: it could check if there is a PR already, and open it if so, or create a new one if not. However, I also want the command in the local commits panel for the checked out branch, and there's no existing "Create PR" command there; and the `o` command opens the selected commit in the browser, so it's unrelated.
…R if there is one
For esthetic reasons, checking out a branch (or other ref) blocks the UI until the refresh is done, so it's important that the refresh doesn't do unnecessary work. Refreshing pull requests is unnecessary (but costly, when waiting for it) when a branch is checked out that already existed locally. However, it is required when checking out a remote branch for the first time, so that the PR icon appears immediately when there is one.
ee0b6c0 to
6c8110f
Compare
We don't treat main any specially, so if there is a PR associated with it for whatever reason, we show it, yes. At my workplace we used to have a closed PR for main that somebody created by mistake at some point, and we had to ask github support to delete it for us so that it doesn't show up in lazygit. I mean, I could also avoid showing the icon for any main branch, assuming that it never makes sense for those. Hadn't considered this yet. Out of curiosity, why do you have one for main? It looks like it is a merged one. |
|
It's a recent PR from 6 months ago It shows this on the PR in GitHub:
|
|
Hm ok, it seems showing something like this is never useful, so I might consider hiding the icon for main branches. I'm just worried that there might be cases where there's a legit PR on a main branch (maybe not literally |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.60.0` → `v0.61.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary> ### [`v0.61.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.61.0) [Compare Source](jesseduffield/lazygit@v0.60.0...v0.61.0) <!-- Release notes generated using configuration in .github/release.yml at v0.61.0 --> The big one in this release is support for GitHub pull requests. They are shown as little GitHub icons next to each branch that has one, and you can open a MR in the browser by pressing shift-G. To enable this, all you need to do is install the [`gh`](https://cli.github.com/) tool if you haven't already, and log in using `gh auth login`. #### What's Changed ##### Features ✨ - Show pull requests against branches by [@​jesseduffield](https://github.com/jesseduffield) in [#​2781](jesseduffield/lazygit#2781) ##### Enhancements 🔥 - Add support for clicking on arrows in the file list to expand/collapse directories by [@​blakemckeany](https://github.com/blakemckeany) in [#​5365](jesseduffield/lazygit#5365) - Remove empty directories after discarding untracked files by [@​stefanhaller](https://github.com/stefanhaller) in [#​5408](jesseduffield/lazygit#5408) - Make file sort order and case sensitivity configurable, and default to mix files and folders by [@​stefanhaller](https://github.com/stefanhaller) in [#​5427](jesseduffield/lazygit#5427) - Allow customizing the window width/height thresholds for when to use portrait mode by [@​stefanhaller](https://github.com/stefanhaller) in [#​5452](jesseduffield/lazygit#5452) - Log hashes of local branches when deleting them by [@​stefanhaller](https://github.com/stefanhaller) in [#​5441](jesseduffield/lazygit#5441) - Add condition field to custom command prompts by [@​mrt181](https://github.com/mrt181) in [#​5364](jesseduffield/lazygit#5364) ##### Fixes 🔧 - Fix staging only some lines of a block of consecutive changes by [@​stefanhaller](https://github.com/stefanhaller) in [#​5396](jesseduffield/lazygit#5396) - Fix the expanded layout of the branches panel (half and full screen modes) by [@​stefanhaller](https://github.com/stefanhaller) in [#​5413](jesseduffield/lazygit#5413) - Fix searching commits or main view after switching repos by [@​stefanhaller](https://github.com/stefanhaller) in [#​5424](jesseduffield/lazygit#5424) - Scroll to top when showing subcommits by [@​stefanhaller](https://github.com/stefanhaller) in [#​5425](jesseduffield/lazygit#5425) - Fix patch commands when git config has color=always by [@​matthijskooijman](https://github.com/matthijskooijman) in [#​5405](jesseduffield/lazygit#5405) - Don't stage out-of-date submodules when asking user to auto-stage after resolving conflicts by [@​stefanhaller](https://github.com/stefanhaller) in [#​5440](jesseduffield/lazygit#5440) ##### Maintenance ⚙️ - Remove go-git dependency by [@​stefanhaller](https://github.com/stefanhaller) in [#​5420](jesseduffield/lazygit#5420) - Make Debian/Ubuntu install command architecture-independent by [@​discapes](https://github.com/discapes) in [#​5386](jesseduffield/lazygit#5386) - Bump github.com/buger/jsonparser from 1.1.1 to 1.1.2 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5423](jesseduffield/lazygit#5423) - fix: pin 7 unpinned action(s), extract 1 inline secret to env var by [@​dagecko](https://github.com/dagecko) in [#​5439](jesseduffield/lazygit#5439) - Fix dependabot config file by [@​stefanhaller](https://github.com/stefanhaller) in [#​5443](jesseduffield/lazygit#5443) - Bump actions/cache from 4 to 5 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5444](jesseduffield/lazygit#5444) - Bump actions/download-artifact from 7 to 8 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5445](jesseduffield/lazygit#5445) - Bump actions/upload-artifact from 6 to 7 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5446](jesseduffield/lazygit#5446) - Bump github.com/lucasb-eyer/go-colorful from 1.3.0 to 1.4.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5447](jesseduffield/lazygit#5447) - Bump github.com/spf13/afero from 1.9.5 to 1.15.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5448](jesseduffield/lazygit#5448) - Bump github.com/creack/pty from 1.1.11 to 1.1.24 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5449](jesseduffield/lazygit#5449) - Bump github.com/stretchr/testify from 1.10.0 to 1.11.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5450](jesseduffield/lazygit#5450) - Bump github.com/sanity-io/litter from 1.5.2 to 1.5.8 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5451](jesseduffield/lazygit#5451) - Bump github.com/adrg/xdg from 0.4.0 to 0.5.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5456](jesseduffield/lazygit#5456) - Bump github.com/spkg/bom from 0.0.0-20160624110644-59b7046e48ad to 1.0.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5457](jesseduffield/lazygit#5457) - Bump github.com/integrii/flaggy from 1.4.0 to 1.8.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5458](jesseduffield/lazygit#5458) - Bump github.com/sahilm/fuzzy from 0.1.0 to 0.1.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5459](jesseduffield/lazygit#5459) - Bump github.com/sasha-s/go-deadlock from 0.3.6 to 0.3.9 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5460](jesseduffield/lazygit#5460) ##### Docs 📖 - Add a note about AI to CONTRIBUTING.md by [@​stefanhaller](https://github.com/stefanhaller) in [#​5404](jesseduffield/lazygit#5404) - Update redo keybinding in README.md by [@​unikitty37](https://github.com/unikitty37) in [#​5387](jesseduffield/lazygit#5387) - Fix grammar in the contributor guide by [@​Rohan5commit](https://github.com/Rohan5commit) in [#​5392](jesseduffield/lazygit#5392) ##### I18n 🌎 - Update translations from Crowdin by [@​stefanhaller](https://github.com/stefanhaller) in [#​5476](jesseduffield/lazygit#5476) ##### Performance Improvements 📊 - Improve performance of discarding many files by [@​stefanhaller](https://github.com/stefanhaller) in [#​5407](jesseduffield/lazygit#5407) #### New Contributors - [@​blakemckeany](https://github.com/blakemckeany) made their first contribution in [#​5365](jesseduffield/lazygit#5365) - [@​discapes](https://github.com/discapes) made their first contribution in [#​5386](jesseduffield/lazygit#5386) - [@​unikitty37](https://github.com/unikitty37) made their first contribution in [#​5387](jesseduffield/lazygit#5387) - [@​Rohan5commit](https://github.com/Rohan5commit) made their first contribution in [#​5392](jesseduffield/lazygit#5392) - [@​matthijskooijman](https://github.com/matthijskooijman) made their first contribution in [#​5405](jesseduffield/lazygit#5405) - [@​dagecko](https://github.com/dagecko) made their first contribution in [#​5439](jesseduffield/lazygit#5439) - [@​mrt181](https://github.com/mrt181) made their first contribution in [#​5364](jesseduffield/lazygit#5364) **Full Changelog**: <jesseduffield/lazygit@v0.60.0...v0.61.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDQuOCIsInVwZGF0ZWRJblZlciI6IjQzLjEwNC44IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
|
I don't know. I don't ever want to see a PR for my main branch. But it might not be for everyone. |
|
@ManuLinares No; hiding closed PRs for main branches was discussed above, and I'll probably add this soon (on the assumption that a PR on a main branch is almost always a mistake). For other branches, it is important to see the PR even if closed or merged, because it tells you that you can now delete the branch. There is one special case that trips me up every time I run into it: creating a new branch and reusing a name that was used before. Happened to me today, the branch was called |
|
Doesn't this show that maybe this feature is, flawed? I mean I like it but the requirement that branches have to be unique is just not working. |
|
@ruudk If you have a suggestion how we can improve it, I'm all ears. |
|
@ruudk @ManuLinares @shgew Here's a PR for hiding PRs on main branches: #5501. Please check the description to see if the logic makes sense to you all. |


If the user has
ghinstalled and is logged in (gh auth login), lazygit shows GitHub PR icons next to the names of branches that have an associated PR, colored by the PR's status (green=open, red=closed, purple=merged).Selecting a branch and pressing
shift-Gopens the PR in the browser.