Skip to content

Commit 7a8db80

Browse files
mislavmislav
committed
Prompt for push target during pr create
We no longer guess the head repository using heuristics; instead, we present the user with the choice of pushable repositories and an additional option to create a new fork. The new `pr create --head` flag is available for the user to specify the head branch in `branch` or `owner:branch` format and completely skip any forking or auto-pushing checks.
1 parent d534a94 commit 7a8db80

File tree

5 files changed

+269
-801
lines changed

5 files changed

+269
-801
lines changed

‎api/queries_repo.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
9191
query RepositoryInfo($owner: String!, $name: String!) {
9292
repository(owner: $owner, name: $name) {
9393
id
94+
name
95+
owner { login }
9496
hasIssuesEnabled
9597
description
9698
viewerPermission
@@ -317,8 +319,8 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
317319
}, nil
318320
}
319321

320-
// RepoFindFork finds a fork of repo affiliated with the viewer
321-
func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) {
322+
// RepoFindForks finds forks of the repo that are affiliated with the viewer
323+
func RepoFindForks(client *Client, repo ghrepo.Interface, limit int) ([]*Repository, error) {
322324
result := struct {
323325
Repository struct {
324326
Forks struct {
@@ -330,12 +332,13 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) {
330332
variables := map[string]interface{}{
331333
"owner": repo.RepoOwner(),
332334
"repo": repo.RepoName(),
335+
"limit": limit,
333336
}
334337

335338
if err := client.GraphQL(repo.RepoHost(), `
336-
query RepositoryFindFork($owner: String!, $repo: String!) {
339+
query RepositoryFindFork($owner: String!, $repo: String!, $limit: Int!) {
337340
repository(owner: $owner, name: $repo) {
338-
forks(first: 1, affiliations: [OWNER, COLLABORATOR]) {
341+
forks(first: $limit, affiliations: [OWNER, COLLABORATOR]) {
339342
nodes {
340343
id
341344
name
@@ -350,14 +353,18 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) {
350353
return nil, err
351354
}
352355

353-
forks := result.Repository.Forks.Nodes
354-
// we check ViewerCanPush, even though we expect it to always be true per
355-
// `affiliations` condition, to guard against versions of GitHub with a
356-
// faulty `affiliations` implementation
357-
if len(forks) > 0 && forks[0].ViewerCanPush() {
358-
return InitRepoHostname(&forks[0], repo.RepoHost()), nil
356+
var results []*Repository
357+
for _, r := range result.Repository.Forks.Nodes {
358+
// we check ViewerCanPush, even though we expect it to always be true per
359+
// `affiliations` condition, to guard against versions of GitHub with a
360+
// faulty `affiliations` implementation
361+
if !r.ViewerCanPush() {
362+
continue
363+
}
364+
results = append(results, InitRepoHostname(&r, repo.RepoHost()))
359365
}
360-
return nil, &NotFoundError{errors.New("no fork found")}
366+
367+
return results, nil
361368
}
362369

363370
type RepoMetadataResult struct {

‎context/context.go‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -139,36 +139,21 @@ func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, e
139139
return selectedRepo, err
140140
}
141141

142-
func (r *ResolvedRemotes) HeadRepo(baseRepo ghrepo.Interface) (ghrepo.Interface, error) {
142+
func (r *ResolvedRemotes) HeadRepos() ([]*api.Repository, error) {
143143
if r.network == nil {
144144
err := resolveNetwork(r)
145145
if err != nil {
146146
return nil, err
147147
}
148148
}
149149

150-
// try to find a pushable fork among existing remotes
151-
for _, repo := range r.network.Repositories {
152-
if repo != nil && repo.Parent != nil && repo.ViewerCanPush() && ghrepo.IsSame(repo.Parent, baseRepo) {
153-
return repo, nil
154-
}
155-
}
156-
157-
// a fork might still exist on GitHub, so let's query for it
158-
var notFound *api.NotFoundError
159-
if repo, err := api.RepoFindFork(r.apiClient, baseRepo); err == nil {
160-
return repo, nil
161-
} else if !errors.As(err, &notFound) {
162-
return nil, err
163-
}
164-
165-
// fall back to any listed repository that has push access
150+
var results []*api.Repository
166151
for _, repo := range r.network.Repositories {
167152
if repo != nil && repo.ViewerCanPush() {
168-
return repo, nil
153+
results = append(results, repo)
169154
}
170155
}
171-
return nil, errors.New("none of the repositories have push access")
156+
return results, nil
172157
}
173158

174159
// RemoteForRepo finds the git remote that points to a repository

‎pkg/cmd/pr/create/create.go‎

Lines changed: 114 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"strings"
88
"time"
99

10+
"github.com/AlecAivazis/survey/v2"
1011
"github.com/MakeNowJust/heredoc"
1112
"github.com/cli/cli/api"
1213
"github.com/cli/cli/context"
@@ -17,6 +18,7 @@ import (
1718
"github.com/cli/cli/pkg/cmdutil"
1819
"github.com/cli/cli/pkg/githubtemplate"
1920
"github.com/cli/cli/pkg/iostreams"
21+
"github.com/cli/cli/pkg/prompt"
2022
"github.com/cli/cli/utils"
2123
"github.com/spf13/cobra"
2224
)
@@ -40,6 +42,7 @@ type CreateOptions struct {
4042
Title string
4143
Body string
4244
BaseBranch string
45+
HeadBranch string
4346

4447
Reviewers []string
4548
Assignees []string
@@ -60,13 +63,23 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
6063
cmd := &cobra.Command{
6164
Use: "create",
6265
Short: "Create a pull request",
66+
Long: heredoc.Doc(`
67+
Create a pull request on GitHub.
68+
69+
When the current branch isn't fully pushed to a git remote, a prompt will ask where
70+
to push the branch and offer an option to fork the base repository. Use '--head' to
71+
explicitly skip any forking or pushing behavior.
72+
73+
A prompt will also ask for the title and the body of the pull request. Use '--title'
74+
and '--body' to skip this, or use '--fill' to autofill these values from git commits.
75+
`),
6376
Example: heredoc.Doc(`
6477
$ gh pr create --title "The bug is fixed" --body "Everything works again"
6578
$ gh issue create --label "bug,help wanted"
6679
$ gh issue create --label bug --label "help wanted"
6780
$ gh pr create --reviewer monalisa,hubot
6881
$ gh pr create --project "Roadmap"
69-
$ gh pr create --base develop
82+
$ gh pr create --base develop --head monalisa:feature
7083
`),
7184
Args: cmdutil.NoArgsQuoteReminder,
7285
RunE: func(cmd *cobra.Command, args []string) error {
@@ -96,9 +109,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
96109

97110
fl := cmd.Flags()
98111
fl.BoolVarP(&opts.IsDraft, "draft", "d", false, "Mark pull request as a draft")
99-
fl.StringVarP(&opts.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.")
100-
fl.StringVarP(&opts.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.")
101-
fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The branch into which you want your code merged")
112+
fl.StringVarP(&opts.Title, "title", "t", "", "Title for the pull request")
113+
fl.StringVarP(&opts.Body, "body", "b", "", "Body for the pull request")
114+
fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The `branch` into which you want your code merged")
115+
fl.StringVarP(&opts.HeadBranch, "head", "H", "", "The `branch` that contains commits for your pull request (default: current branch)")
102116
fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request")
103117
fl.BoolVarP(&opts.Autofill, "fill", "f", false, "Do not prompt for title/body and just use commit info")
104118
fl.StringSliceVarP(&opts.Reviewers, "reviewer", "r", nil, "Request reviews from people by their `login`")
@@ -132,6 +146,8 @@ func createRun(opts *CreateOptions) error {
132146
if r, ok := br.(*api.Repository); ok {
133147
baseRepo = r
134148
} else {
149+
// TODO: if RepoNetwork is going to be requested anyway in `repoContext.HeadRepos()`,
150+
// consider piggybacking on that result instead of performing a separate lookup
135151
var err error
136152
baseRepo, err = api.GitHubRepo(client, br)
137153
if err != nil {
@@ -142,34 +158,108 @@ func createRun(opts *CreateOptions) error {
142158
return fmt.Errorf("could not determine base repository: %w", err)
143159
}
144160

145-
headBranch, err := opts.Branch()
146-
if err != nil {
147-
return fmt.Errorf("could not determine the current branch: %w", err)
161+
isPushEnabled := false
162+
headBranch := opts.HeadBranch
163+
headBranchLabel := opts.HeadBranch
164+
if headBranch == "" {
165+
headBranch, err = opts.Branch()
166+
if err != nil {
167+
return fmt.Errorf("could not determine the current branch: %w", err)
168+
}
169+
headBranchLabel = headBranch
170+
isPushEnabled = true
171+
} else if idx := strings.IndexRune(headBranch, ':'); idx >= 0 {
172+
headBranch = headBranch[idx+1:]
173+
}
174+
175+
if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 {
176+
fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change"))
148177
}
149178

150179
var headRepo ghrepo.Interface
151180
var headRemote *context.Remote
152181

153-
// determine whether the head branch is already pushed to a remote
154-
headBranchPushedTo := determineTrackingBranch(remotes, headBranch)
155-
if headBranchPushedTo != nil {
156-
for _, r := range remotes {
157-
if r.Name != headBranchPushedTo.RemoteName {
158-
continue
182+
if isPushEnabled {
183+
// determine whether the head branch is already pushed to a remote
184+
if pushedTo := determineTrackingBranch(remotes, headBranch); pushedTo != nil {
185+
isPushEnabled = false
186+
for _, r := range remotes {
187+
if r.Name != pushedTo.RemoteName {
188+
continue
189+
}
190+
headRepo = r
191+
headRemote = r
192+
break
159193
}
160-
headRepo = r
161-
headRemote = r
162-
break
163194
}
164195
}
165196

166-
// otherwise, determine the head repository with info obtained from the API
167-
if headRepo == nil {
168-
if r, err := repoContext.HeadRepo(baseRepo); err == nil {
169-
headRepo = r
197+
// otherwise, ask the user for the head repository using info obtained from the API
198+
if headRepo == nil && isPushEnabled && opts.IO.CanPrompt() {
199+
pushableRepos, err := repoContext.HeadRepos()
200+
if err != nil {
201+
return err
202+
}
203+
204+
if len(pushableRepos) == 0 {
205+
pushableRepos, err = api.RepoFindForks(client, baseRepo, 3)
206+
if err != nil {
207+
return err
208+
}
209+
}
210+
211+
currentLogin, err := api.CurrentLoginName(client, baseRepo.RepoHost())
212+
if err != nil {
213+
return err
214+
}
215+
216+
hasOwnFork := false
217+
var pushOptions []string
218+
for _, r := range pushableRepos {
219+
pushOptions = append(pushOptions, ghrepo.FullName(r))
220+
if r.RepoOwner() == currentLogin {
221+
hasOwnFork = true
222+
}
223+
}
224+
225+
if !hasOwnFork {
226+
pushOptions = append(pushOptions, "Create a fork of "+ghrepo.FullName(baseRepo))
227+
}
228+
pushOptions = append(pushOptions, "Skip pushing the branch")
229+
pushOptions = append(pushOptions, "Cancel")
230+
231+
var selectedOption int
232+
err = prompt.SurveyAskOne(&survey.Select{
233+
Message: fmt.Sprintf("Where should we push the '%s' branch?", headBranch),
234+
Options: pushOptions,
235+
}, &selectedOption)
236+
if err != nil {
237+
return err
238+
}
239+
240+
if selectedOption < len(pushableRepos) {
241+
headRepo = pushableRepos[selectedOption]
242+
if !ghrepo.IsSame(baseRepo, headRepo) {
243+
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch)
244+
}
245+
} else if pushOptions[selectedOption] == "Skip pushing the branch" {
246+
isPushEnabled = false
247+
} else if pushOptions[selectedOption] == "Cancel" {
248+
return cmdutil.SilentError
249+
} else {
250+
// "Create a fork of ..."
251+
if baseRepo.IsPrivate {
252+
return fmt.Errorf("cannot fork private repository %s", ghrepo.FullName(baseRepo))
253+
}
254+
headBranchLabel = fmt.Sprintf("%s:%s", currentLogin, headBranch)
170255
}
171256
}
172257

258+
if headRepo == nil && isPushEnabled && !opts.IO.CanPrompt() {
259+
fmt.Fprintf(opts.IO.ErrOut, "aborted: you must first push the current branch to a remote, or use the --head flag")
260+
return cmdutil.SilentError
261+
}
262+
173263
baseBranch := opts.BaseBranch
174264
if baseBranch == "" {
175265
baseBranch = baseRepo.DefaultBranchRef.Name
@@ -178,10 +268,6 @@ func createRun(opts *CreateOptions) error {
178268
return fmt.Errorf("must be on a branch named differently than %q", baseBranch)
179269
}
180270

181-
if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 {
182-
fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change"))
183-
}
184-
185271
var milestoneTitles []string
186272
if opts.Milestone != "" {
187273
milestoneTitles = []string{opts.Milestone}
@@ -211,10 +297,6 @@ func createRun(opts *CreateOptions) error {
211297
}
212298

213299
if !opts.WebMode {
214-
headBranchLabel := headBranch
215-
if headRepo != nil && !ghrepo.IsSame(baseRepo, headRepo) {
216-
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch)
217-
}
218300
existingPR, err := api.PullRequestForBranch(client, baseRepo, baseBranch, headBranchLabel)
219301
var notFound *api.NotFoundError
220302
if err != nil && !errors.As(err, &notFound) {
@@ -297,23 +379,15 @@ func createRun(opts *CreateOptions) error {
297379
didForkRepo := false
298380
// if a head repository could not be determined so far, automatically create
299381
// one by forking the base repository
300-
if headRepo == nil {
301-
if baseRepo.IsPrivate {
302-
return fmt.Errorf("cannot fork private repository '%s'", ghrepo.FullName(baseRepo))
303-
}
382+
if headRepo == nil && isPushEnabled {
304383
headRepo, err = api.ForkRepo(client, baseRepo)
305384
if err != nil {
306385
return fmt.Errorf("error forking repo: %w", err)
307386
}
308387
didForkRepo = true
309388
}
310389

311-
headBranchLabel := headBranch
312-
if !ghrepo.IsSame(baseRepo, headRepo) {
313-
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch)
314-
}
315-
316-
if headRemote == nil {
390+
if headRemote == nil && headRepo != nil {
317391
headRemote, _ = repoContext.RemoteForRepo(headRepo)
318392
}
319393

@@ -324,7 +398,7 @@ func createRun(opts *CreateOptions) error {
324398
//
325399
// In either case, we want to add the head repo as a new git remote so we
326400
// can push to it.
327-
if headRemote == nil {
401+
if headRemote == nil && isPushEnabled {
328402
cfg, err := opts.Config()
329403
if err != nil {
330404
return err
@@ -345,7 +419,7 @@ func createRun(opts *CreateOptions) error {
345419
}
346420

347421
// automatically push the branch if it hasn't been pushed anywhere yet
348-
if headBranchPushedTo == nil {
422+
if isPushEnabled {
349423
pushTries := 0
350424
maxPushTries := 3
351425
for {

0 commit comments

Comments
 (0)