WIP: Show pull requests against branches

[jesseduffield]

• PR Description

If the user has gh installed and it's on version 2 or greater, we'll refresh pull requests at the same time we run a background git fetch.

There are a lot of touch points here:

• gh is written in Go and we could avoid an external dependency by using the gh code directly, however doing so would mean taking on the burden of handling things like authentication, which would increase the scope quite a bit. Given that pulling pull requests takes a while (3.5s for me), the extra cost of shelling out to gh is negligible in comparison
• There's no way to say 'only fetch the PRs for specific branches' so we instead pull the latest 500 PRs (looking at the code this may be limited to 100 anyway). This unfortunately takes some time. Filtering to only open PRs makes no difference.
• We show PRs by default, but it can be opted out of
• If the user does not have gh installed we won't attempt to fetch anything
• If the user has gh installed but on an old version (<2) we log a message to the user and don't attempt to retry
• If there is an authentication failure we log and then retry later on
• We refresh PRs at the same rate as we do a git fetch
• If the user hasn't configured the base repo of the current git repo, we prompt them to do so.
• Currently we determine that we're in a git repo by looking at the url of the first remote, to see whether it contains github.com. Gh itself looks at the configured hosts file to see if there is an authentication setup for any host. We should do the same.

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• Docs (specifically docs/Config.md) have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[stefanhaller]

Oooh, very nice!

A few thoughts after trying it (very briefly though, so I might be missing a lot of things):

• It looks like it's fetching the PRs only after it's done fetching the remote(s). Is there a reason for this? For me, fetching often takes a lot of time, so I have to wait quite a while until the PR numbers show up.
• I'm not sure the 500 limit works well. For our monorepo at work this amounts to roughly one month; sometimes we have PRs open for longer than that, so some of my open branches don't show PR numbers. Also, it would be really nice if you would also see PR numbers for very old branches. I haven't thought at all about how to achieve that, so sorry if this is not very constructive input.
• It looks like PR numbers are only shown in the local branches panel. I'm thinking they could be even more useful in the remote branches list. (Not totally sure about that, it's just a feeling for now.)
• Do I understand it right that at the moment the only thing this does is add the PR numbers to the display? I somehow expected you would be able to do more with this; the obvious wish is to open a PR in the web browser (I have a custom command for this). Maybe the o command could be overloaded for this? Show the existing PR if we know there is one, and open a new one if not.

[jesseduffield]

It looks like it's fetching the PRs only after it's done fetching the remote(s). Is there a reason for this? For me, fetching often takes a lot of time, so I have to wait quite a while until the PR numbers show up.

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. github.com/jesseduffield/lazygit) and the branch has the name of the remote. So we need to piece those together to know that a particular PR should be assigned to a particular branch. imo getting remotes should be really fast because it's just a matter of reading a file so if it's currently a bottleneck it's worth looking into.

I'm not sure the 500 limit works well. For our monorepo at work this amounts to roughly one month; sometimes we have PRs open for longer than that, so some of my open branches don't show PR numbers. Also, it would be really nice if you would also see PR numbers for very old branches. I haven't thought at all about how to achieve that, so sorry if this is not very constructive input.

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.

It looks like PR numbers are only shown in the local branches panel. I'm thinking they could be even more useful in the remote branches list. (Not totally sure about that, it's just a feeling for now.)

I agree, and it shouldn't be hard to support that.

Do I understand it right that at the moment the only thing this does is add the PR numbers to the display? I somehow expected you would be able to do more with this; the obvious wish is to open a PR in the web browser (I have a custom command for this). Maybe the o command could be overloaded for this? Show the existing PR if we know there is one, and open a new one if not.

Yep currently it's just for display but I like your idea.

[dlvhdr]

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"
        }
      ]
    }
  }
}

[stefanhaller]

@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?

[dlvhdr]

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.

[Daemoen]

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

[mpasa]

@Daemoen You can easily do this as a customCommand. For example, for creating a PR on the current branch:

customCommands:
  - key: "o"
    command: "gh pr create"
    subprocess: true
    context: "localBranches"

[OliverJAsh]

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.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 1d0db511	✅ 30.01%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (1d0db51)	Report Missing	Report Missing	Report Missing
Head commit (3b677cb)	60273	52106	86.45%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#2781)	703	211	30.01%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[jesseduffield]

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)

[stefanhaller]

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:

• branch name ambiguity. In some repos it is common to reuse the same branch name again and again for similar PRs; one example from this repo is the Update Sponsors PRs, but there are many others. In such a case, lazygit shows the oldest one of these PR (usually a closed one), which makes no sense at all. It should either use the newest, or find an open one, or some combination of both.
• the behavior when using it in a repo for the first time (i.e. when there's no gh-resolved config yet); currently it shows a confusing browser with no title. I wonder if we should just default to 'origin' if there is one, or show no PRs if not; users then have to use gh on the command line to set the base repo.

[jesseduffield]

branch name ambiguity. In some repos it is common to reuse the same branch name again and again for similar PRs; one example from this repo is the #5228 PRs, but there are many others. In such a case, lazygit shows the oldest one of these PR (usually a closed one), which makes no sense at all. It should either use the newest, or find an open one, or some combination of both.

I've got it sorting by newest first now when fetching PRs. Picking the newest should be fine.

the behavior when using it in a repo for the first time (i.e. when there's no gh-resolved config yet); currently it shows a confusing browser with no title. I wonder if we should just default to 'origin' if there is one, or show no PRs if not; users then have to use gh on the command line to set the base repo.

Now it selects the one repo if there's only one, otherwise 'origin', otherwise prompts the user and I've got a title on the prompt now. I think this works well.