vscode-pull-request-github icon indicating copy to clipboard operation
vscode-pull-request-github copied to clipboard

Checked out PRs are not preloaded in VS Code desktop when using GitHub Repositories

Open joyceerhl opened this issue 3 years ago • 7 comments

This works as expected in web provided you're looking at a url like https://insiders.vscode.dev/github/microsoft/vscode/pull/163305, because the id 163305 gets passed in correctly. On desktop the id ends up being the branch name which is not correct, and that causes GitHub Repositories to not run the preload command to focus GHPRI's review mode. This is possibly an issue in GitHub Repositories, still investigating...

joyceerhl avatar Oct 13 '22 00:10 joyceerhl

This appears to work correctly for PRs from forks, but not for PRs where the branch exists on the current repository.

joyceerhl avatar Oct 13 '22 01:10 joyceerhl

Interestingly, this works correctly if I use Open Remote Repository > Open Pull Request from GitHub but doesn't work if I right click on a PR from the GitHub tree view and select Checkout Pull Request.

joyceerhl avatar Oct 19 '22 18:10 joyceerhl

@joyceerhl do you mean that github.api.preloadPullRequest is not executed? I tried this out on desktop and that's what I see (github.api.preloadPullRequest isn't executed).

I don't think there's anything I can do from the GHPRI side to make the preload command call happen.

What I can do:

  • When a PR is checked out I can persist some state about the intended branch.
  • On extension activation I can check for that state and do the "preload".
  • On every extension activation/branch change I can delete that state.

alexr00 avatar Dec 09 '22 09:12 alexr00

@joyceerhl before I make this change: is there any plan to remove the window reload when checking a branch on desktop, since that would also solve this issue?

alexr00 avatar Dec 09 '22 09:12 alexr00

I discovered that github.api.preloadPullRequest is executed for PRs across forks e.g. https://github.com/microsoft/vscode/pull/168273, because with remote PRs, GHPRI calls checkout with a branch of the format pr/<remote>/<pr ID>, and RemoteHub does the right thing with that.

But for PRs on the current repo, GHPRI calls checkout passing just the branch name, and in that case RemoteHub's impl of checkout just checks out the branch without setting the workspace to indicate that the PR should be checked out. Since Checkout Pull Request indicates a clear intent to focus the PR and not just check out the branch, could GHPRI pass pr/origin/<PR number> to the checkout call (which I verified does fix the problem), or would this be a breaking change for desktop?

Alternatively, does GHPRI have code to check whether there is a PR associated with the current branch so it can independently decide to open the PR view even if RemoteHub isn't calling github.api.preloadPullRequest?

is there any plan to remove the window reload when checking a branch on desktop

This cannot happen without https://github.com/microsoft/vscode/issues/35109 (though I'm unsure what blockers exist for that now).

joyceerhl avatar Dec 09 '22 22:12 joyceerhl

Alternatively, does GHPRI have code to check whether there is a PR associated with the current branch so it can independently decide to open the PR view even if RemoteHub isn't calling github.api.preloadPullRequest?

We already detect this, but we can't use it to open the view on reload as it would be overly eager and open on reload even when not checking out a PR.

could GHPRI pass pr/origin/<PR number> to the checkout call (which I verified does fix the problem), or would this be a breaking change for desktop?

I'll try it out and see.

alexr00 avatar Dec 12 '22 10:12 alexr00

Passing pr/origin/<PR branch> to the checkout call would be a breaking change since that wouldn't be referencing a branch that exists on desktop. I'll try the proposal outlined in https://github.com/microsoft/vscode-pull-request-github/issues/4047#issuecomment-1344058790

alexr00 avatar Dec 14 '22 13:12 alexr00