Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
joyceerhl opened this issue Oct 13, 2022 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality
Milestone

Comments

@joyceerhl
Copy link
Contributor

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
Copy link
Contributor Author

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

@joyceerhl
Copy link
Contributor Author

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 joyceerhl removed their assignment Dec 6, 2022
@alexr00
Copy link
Member

alexr00 commented Dec 9, 2022

@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
Copy link
Member

alexr00 commented Dec 9, 2022

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

@joyceerhl
Copy link
Contributor Author

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 microsoft/vscode#35109 (though I'm unsure what blockers exist for that now).

@alexr00
Copy link
Member

alexr00 commented Dec 12, 2022

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/ 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
Copy link
Member

alexr00 commented Dec 14, 2022

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 #4047 (comment)

@alexr00 alexr00 added the feature-request Request for new features or functionality label Dec 15, 2023
@alexr00 alexr00 added this to the Backlog milestone Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

2 participants