diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 2dab75d5b..40ab1acb4 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -23,4 +23,4 @@ jobs: - name: Build run: | go test ./... - go build -v . + go build -v ./cmd/gh diff --git a/.goreleaser.yml b/.goreleaser.yml index b3ee2336c..c8ffa8997 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -8,6 +8,7 @@ before: - go mod tidy builds: - binary: bin/gh + main: ./cmd/gh ldflags: - -s -w -X github.com/github/gh-cli/command.Version={{.Version}} -X github.com/github/gh-cli/command.BuildDate={{time "2006-01-02"}} - -X github.com/github/gh-cli/context.oauthClientID={{.Env.GH_OAUTH_CLIENT_ID}} diff --git a/Makefile b/Makefile index d17f749bf..d3cb03449 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ ifdef GH_OAUTH_CLIENT_SECRET endif bin/gh: $(BUILD_FILES) - @go build -ldflags "$(LDFLAGS)" -o "$@" + @go build -ldflags "$(LDFLAGS)" -o "$@" ./cmd/gh test: go test ./... diff --git a/api/client.go b/api/client.go index 2aa82d1ea..c056680c9 100644 --- a/api/client.go +++ b/api/client.go @@ -7,6 +7,8 @@ import ( "io" "io/ioutil" "net/http" + "regexp" + "strings" ) // ClientOption represents an argument to NewClient @@ -34,13 +36,26 @@ func AddHeader(name, value string) ClientOption { } // VerboseLog enables request/response logging within a RoundTripper -func VerboseLog(out io.Writer) ClientOption { +func VerboseLog(out io.Writer, logBodies bool) ClientOption { return func(tr http.RoundTripper) http.RoundTripper { return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { fmt.Fprintf(out, "> %s %s\n", req.Method, req.URL.RequestURI()) + if logBodies && req.Body != nil && inspectableMIMEType(req.Header.Get("Content-type")) { + newBody := &bytes.Buffer{} + io.Copy(out, io.TeeReader(req.Body, newBody)) + fmt.Fprintln(out) + req.Body = ioutil.NopCloser(newBody) + } res, err := tr.RoundTrip(req) if err == nil { fmt.Fprintf(out, "< HTTP %s\n", res.Status) + if logBodies && res.Body != nil && inspectableMIMEType(res.Header.Get("Content-type")) { + newBody := &bytes.Buffer{} + // TODO: pretty-print response JSON + io.Copy(out, io.TeeReader(res.Body, newBody)) + fmt.Fprintln(out) + res.Body = ioutil.NopCloser(newBody) + } } return res, err }} @@ -69,9 +84,27 @@ type Client struct { type graphQLResponse struct { Data interface{} - Errors []struct { - Message string + Errors []GraphQLError +} + +// GraphQLError is a single error returned in a GraphQL response +type GraphQLError struct { + Type string + Path []string + Message string +} + +// GraphQLErrorResponse contains errors returned in a GraphQL response +type GraphQLErrorResponse struct { + Errors []GraphQLError +} + +func (gr GraphQLErrorResponse) Error() string { + errorMessages := make([]string, 0, len(gr.Errors)) + for _, e := range gr.Errors { + errorMessages = append(errorMessages, e.Message) } + return fmt.Sprintf("graphql error: '%s'", strings.Join(errorMessages, ", ")) } // GraphQL performs a GraphQL request and parses the response @@ -151,14 +184,9 @@ func handleResponse(resp *http.Response, data interface{}) error { } if len(gr.Errors) > 0 { - errorMessages := gr.Errors[0].Message - for _, e := range gr.Errors[1:] { - errorMessages += ", " + e.Message - } - return fmt.Errorf("graphql error: '%s'", errorMessages) + return &GraphQLErrorResponse{Errors: gr.Errors} } return nil - } func handleHTTPError(resp *http.Response) error { @@ -179,3 +207,9 @@ func handleHTTPError(resp *http.Response) error { return fmt.Errorf("http error, '%s' failed (%d): '%s'", resp.Request.URL, resp.StatusCode, message) } + +var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`) + +func inspectableMIMEType(t string) bool { + return strings.HasPrefix(t, "text/") || jsonTypeRE.MatchString(t) +} diff --git a/api/client_test.go b/api/client_test.go index 4d7c64917..12bfbe408 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -2,7 +2,6 @@ package api import ( "bytes" - "fmt" "io/ioutil" "reflect" "testing" @@ -47,5 +46,7 @@ func TestGraphQLError(t *testing.T) { response := struct{}{} http.StubResponse(200, bytes.NewBufferString(`{"errors":[{"message":"OH NO"}]}`)) err := client.GraphQL("", nil, &response) - eq(t, err, fmt.Errorf("graphql error: 'OH NO'")) + if err == nil || err.Error() != "graphql error: 'OH NO'" { + t.Fatalf("got %q", err.Error()) + } } diff --git a/api/queries_issue.go b/api/queries_issue.go index 7ba12f6ad..33e5b2143 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -3,6 +3,8 @@ package api import ( "fmt" "time" + + "github.com/github/gh-cli/internal/ghrepo" ) type IssuesPayload struct { @@ -91,7 +93,7 @@ func IssueCreate(client *Client, repo *Repository, params map[string]interface{} return &result.CreateIssue.Issue, nil } -func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) { +func IssueStatus(client *Client, repo ghrepo.Interface, currentUsername string) (*IssuesPayload, error) { type response struct { Repository struct { Assigned struct { @@ -135,11 +137,9 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa } }` - owner := ghRepo.RepoOwner() - repo := ghRepo.RepoName() variables := map[string]interface{}{ - "owner": owner, - "repo": repo, + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), "viewer": currentUsername, } @@ -150,7 +150,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa } if !resp.Repository.HasIssuesEnabled { - return nil, fmt.Errorf("the '%s/%s' repository has disabled issues", owner, repo) + return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo)) } payload := IssuesPayload{ @@ -171,7 +171,7 @@ func IssueStatus(client *Client, ghRepo Repo, currentUsername string) (*IssuesPa return &payload, nil } -func IssueList(client *Client, ghRepo Repo, state string, labels []string, assigneeString string, limit int) ([]Issue, error) { +func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int) ([]Issue, error) { var states []string switch state { case "open", "": @@ -197,12 +197,10 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig } ` - owner := ghRepo.RepoOwner() - repo := ghRepo.RepoName() variables := map[string]interface{}{ "limit": limit, - "owner": owner, - "repo": repo, + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), "states": states, } if len(labels) > 0 { @@ -227,22 +225,24 @@ func IssueList(client *Client, ghRepo Repo, state string, labels []string, assig } if !resp.Repository.HasIssuesEnabled { - return nil, fmt.Errorf("the '%s/%s' repository has disabled issues", owner, repo) + return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo)) } return resp.Repository.Issues.Nodes, nil } -func IssueByNumber(client *Client, ghRepo Repo, number int) (*Issue, error) { +func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, error) { type response struct { Repository struct { - Issue Issue + Issue Issue + HasIssuesEnabled bool } } query := ` query($owner: String!, $repo: String!, $issue_number: Int!) { repository(owner: $owner, name: $repo) { + hasIssuesEnabled issue(number: $issue_number) { title body @@ -264,8 +264,8 @@ func IssueByNumber(client *Client, ghRepo Repo, number int) (*Issue, error) { }` variables := map[string]interface{}{ - "owner": ghRepo.RepoOwner(), - "repo": ghRepo.RepoName(), + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), "issue_number": number, } @@ -275,5 +275,9 @@ func IssueByNumber(client *Client, ghRepo Repo, number int) (*Issue, error) { return nil, err } + if !resp.Repository.HasIssuesEnabled { + return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo)) + } + return &resp.Repository.Issue, nil } diff --git a/api/queries_pr.go b/api/queries_pr.go index 9603bf0f2..af7e30e68 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -3,6 +3,8 @@ package api import ( "fmt" "strings" + + "github.com/github/gh-cli/internal/ghrepo" ) type PullRequestsPayload struct { @@ -127,12 +129,7 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { return } -type Repo interface { - RepoName() string - RepoOwner() string -} - -func PullRequests(client *Client, ghRepo Repo, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) { +func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) { type edges struct { TotalCount int Edges []struct { @@ -229,11 +226,8 @@ func PullRequests(client *Client, ghRepo Repo, currentPRNumber int, currentPRHea } ` - owner := ghRepo.RepoOwner() - repo := ghRepo.RepoName() - - viewerQuery := fmt.Sprintf("repo:%s/%s state:open is:pr author:%s", owner, repo, currentUsername) - reviewerQuery := fmt.Sprintf("repo:%s/%s state:open review-requested:%s", owner, repo, currentUsername) + viewerQuery := fmt.Sprintf("repo:%s state:open is:pr author:%s", ghrepo.FullName(repo), currentUsername) + reviewerQuery := fmt.Sprintf("repo:%s state:open review-requested:%s", ghrepo.FullName(repo), currentUsername) branchWithoutOwner := currentPRHeadRef if idx := strings.Index(currentPRHeadRef, ":"); idx >= 0 { @@ -243,8 +237,8 @@ func PullRequests(client *Client, ghRepo Repo, currentPRNumber int, currentPRHea variables := map[string]interface{}{ "viewerQuery": viewerQuery, "reviewerQuery": reviewerQuery, - "owner": owner, - "repo": repo, + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), "headRefName": branchWithoutOwner, "number": currentPRNumber, } @@ -289,7 +283,7 @@ func PullRequests(client *Client, ghRepo Repo, currentPRNumber int, currentPRHea return &payload, nil } -func PullRequestByNumber(client *Client, ghRepo Repo, number int) (*PullRequest, error) { +func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*PullRequest, error) { type response struct { Repository struct { PullRequest PullRequest @@ -328,8 +322,8 @@ func PullRequestByNumber(client *Client, ghRepo Repo, number int) (*PullRequest, }` variables := map[string]interface{}{ - "owner": ghRepo.RepoOwner(), - "repo": ghRepo.RepoName(), + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), "pr_number": number, } @@ -342,7 +336,7 @@ func PullRequestByNumber(client *Client, ghRepo Repo, number int) (*PullRequest, return &resp.Repository.PullRequest, nil } -func PullRequestForBranch(client *Client, ghRepo Repo, branch string) (*PullRequest, error) { +func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string) (*PullRequest, error) { type response struct { Repository struct { PullRequests struct { @@ -383,8 +377,8 @@ func PullRequestForBranch(client *Client, ghRepo Repo, branch string) (*PullRequ } variables := map[string]interface{}{ - "owner": ghRepo.RepoOwner(), - "repo": ghRepo.RepoName(), + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), "headRefName": branchWithoutOwner, } @@ -403,12 +397,8 @@ func PullRequestForBranch(client *Client, ghRepo Repo, branch string) (*PullRequ return nil, &NotFoundError{fmt.Errorf("no open pull requests found for branch %q", branch)} } -func CreatePullRequest(client *Client, ghRepo Repo, params map[string]interface{}) (*PullRequest, error) { - repo, err := GitHubRepo(client, ghRepo) - if err != nil { - return nil, err - } - +// CreatePullRequest creates a pull request in a GitHub repository +func CreatePullRequest(client *Client, repo *Repository, params map[string]interface{}) (*PullRequest, error) { query := ` mutation CreatePullRequest($input: CreatePullRequestInput!) { createPullRequest(input: $input) { @@ -434,7 +424,7 @@ func CreatePullRequest(client *Client, ghRepo Repo, params map[string]interface{ } }{} - err = client.GraphQL(query, variables, &result) + err := client.GraphQL(query, variables, &result) if err != nil { return nil, err } diff --git a/api/queries_repo.go b/api/queries_repo.go index 7dff79087..46b0fa96a 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1,20 +1,66 @@ package api import ( + "bytes" + "encoding/json" "fmt" + "sort" + "strings" + + "github.com/github/gh-cli/internal/ghrepo" ) // Repository contains information about a GitHub repo type Repository struct { - ID string + ID string + Name string + Owner RepositoryOwner + + IsPrivate bool HasIssuesEnabled bool + ViewerPermission string + DefaultBranchRef struct { + Name string + Target struct { + OID string + } + } + + Parent *Repository +} + +// RepositoryOwner is the owner of a GitHub repository +type RepositoryOwner struct { + Login string +} + +// RepoOwner is the login name of the owner +func (r Repository) RepoOwner() string { + return r.Owner.Login +} + +// RepoName is the name of the repository +func (r Repository) RepoName() string { + return r.Name +} + +// IsFork is true when this repository has a parent repository +func (r Repository) IsFork() bool { + return r.Parent != nil +} + +// ViewerCanPush is true when the requesting user has push access +func (r Repository) ViewerCanPush() bool { + switch r.ViewerPermission { + case "ADMIN", "MAINTAIN", "WRITE": + return true + default: + return false + } } // GitHubRepo looks up the node ID of a named repository -func GitHubRepo(client *Client, ghRepo Repo) (*Repository, error) { - owner := ghRepo.RepoOwner() - repo := ghRepo.RepoName() - +func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { query := ` query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { @@ -23,8 +69,8 @@ func GitHubRepo(client *Client, ghRepo Repo) (*Repository, error) { } }` variables := map[string]interface{}{ - "owner": owner, - "name": repo, + "owner": repo.RepoOwner(), + "name": repo.RepoName(), } result := struct { @@ -33,7 +79,7 @@ func GitHubRepo(client *Client, ghRepo Repo) (*Repository, error) { err := client.GraphQL(query, variables, &result) if err != nil || result.Repository.ID == "" { - newErr := fmt.Errorf("failed to determine repository ID for '%s/%s'", owner, repo) + newErr := fmt.Errorf("failed to determine repository ID for '%s'", ghrepo.FullName(repo)) if err != nil { newErr = fmt.Errorf("%s: %w", newErr, err) } @@ -42,3 +88,131 @@ func GitHubRepo(client *Client, ghRepo Repo) (*Repository, error) { return &result.Repository, nil } + +// RepoNetworkResult describes the relationship between related repositories +type RepoNetworkResult struct { + ViewerLogin string + Repositories []*Repository +} + +// RepoNetwork inspects the relationship between multiple GitHub repositories +func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, error) { + queries := []string{} + for i, repo := range repos { + queries = append(queries, fmt.Sprintf(` + repo_%03d: repository(owner: %q, name: %q) { + ...repo + parent { + ...repo + } + } + `, i, repo.RepoOwner(), repo.RepoName())) + } + + // Since the query is constructed dynamically, we can't parse a response + // format using a static struct. Instead, hold the raw JSON data until we + // decide how to parse it manually. + graphqlResult := map[string]*json.RawMessage{} + result := RepoNetworkResult{} + + err := client.GraphQL(fmt.Sprintf(` + fragment repo on Repository { + id + name + owner { login } + viewerPermission + defaultBranchRef { + name + target { oid } + } + isPrivate + } + query { + viewer { login } + %s + } + `, strings.Join(queries, "")), nil, &graphqlResult) + graphqlError, isGraphQLError := err.(*GraphQLErrorResponse) + if isGraphQLError { + // If the only errors are that certain repositories are not found, + // continue processing this response instead of returning an error + tolerated := true + for _, ge := range graphqlError.Errors { + if ge.Type != "NOT_FOUND" { + tolerated = false + } + } + if tolerated { + err = nil + } + } + if err != nil { + return result, err + } + + keys := []string{} + for key := range graphqlResult { + keys = append(keys, key) + } + // sort keys to ensure `repo_{N}` entries are processed in order + sort.Sort(sort.StringSlice(keys)) + + // Iterate over keys of GraphQL response data and, based on its name, + // dynamically allocate the target struct an individual message gets decoded to. + for _, name := range keys { + jsonMessage := graphqlResult[name] + if name == "viewer" { + viewerResult := struct { + Login string + }{} + decoder := json.NewDecoder(bytes.NewReader([]byte(*jsonMessage))) + if err := decoder.Decode(&viewerResult); err != nil { + return result, err + } + result.ViewerLogin = viewerResult.Login + } else if strings.HasPrefix(name, "repo_") { + if jsonMessage == nil { + result.Repositories = append(result.Repositories, nil) + continue + } + repo := Repository{} + decoder := json.NewDecoder(bytes.NewReader([]byte(*jsonMessage))) + if err := decoder.Decode(&repo); err != nil { + return result, err + } + result.Repositories = append(result.Repositories, &repo) + } else { + return result, fmt.Errorf("unknown GraphQL result key %q", name) + } + } + return result, nil +} + +// repositoryV3 is the repository result from GitHub API v3 +type repositoryV3 struct { + NodeID string + Name string + Owner struct { + Login string + } +} + +// ForkRepo forks the repository on GitHub and returns the new repository +func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { + path := fmt.Sprintf("repos/%s/forks", ghrepo.FullName(repo)) + body := bytes.NewBufferString(`{}`) + result := repositoryV3{} + err := client.REST("POST", path, body, &result) + if err != nil { + return nil, err + } + + return &Repository{ + ID: result.NodeID, + Name: result.Name, + Owner: RepositoryOwner{ + Login: result.Owner.Login, + }, + ViewerPermission: "WRITE", + }, nil +} diff --git a/main.go b/cmd/gh/main.go similarity index 66% rename from main.go rename to cmd/gh/main.go index 726b50784..15925a6a9 100644 --- a/main.go +++ b/cmd/gh/main.go @@ -1,7 +1,10 @@ package main import ( + "errors" "fmt" + "io" + "net" "os" "path" "strings" @@ -12,6 +15,7 @@ import ( "github.com/github/gh-cli/utils" "github.com/mattn/go-isatty" "github.com/mgutz/ansi" + "github.com/spf13/cobra" ) var updaterEnabled = "" @@ -24,12 +28,10 @@ func main() { updateMessageChan <- rel }() + hasDebug := os.Getenv("DEBUG") != "" + if cmd, err := command.RootCmd.ExecuteC(); err != nil { - fmt.Fprintln(os.Stderr, err) - _, isFlagError := err.(command.FlagError) - if isFlagError || strings.HasPrefix(err.Error(), "unknown command ") { - fmt.Fprintln(os.Stderr, cmd.UsageString()) - } + printError(os.Stderr, err, cmd, hasDebug) os.Exit(1) } @@ -46,6 +48,28 @@ func main() { } } +func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) { + var dnsError *net.DNSError + if errors.As(err, &dnsError) { + fmt.Fprintf(out, "error connecting to %s\n", dnsError.Name) + if debug { + fmt.Fprintln(out, dnsError) + } + fmt.Fprintln(out, "check your internet connection or githubstatus.com") + return + } + + fmt.Fprintln(out, err) + + var flagError *command.FlagError + if errors.As(err, &flagError) || strings.HasPrefix(err.Error(), "unknown command ") { + if !strings.HasSuffix(err.Error(), "\n") { + fmt.Fprintln(out) + } + fmt.Fprintln(out, cmd.UsageString()) + } +} + func shouldCheckForUpdate() bool { errFd := os.Stderr.Fd() return updaterEnabled != "" && (isatty.IsTerminal(errFd) || isatty.IsCygwinTerminal(errFd)) diff --git a/cmd/gh/main_test.go b/cmd/gh/main_test.go new file mode 100644 index 000000000..3a62f6470 --- /dev/null +++ b/cmd/gh/main_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "bytes" + "errors" + "fmt" + "net" + "testing" + + "github.com/github/gh-cli/command" + "github.com/spf13/cobra" +) + +func Test_printError(t *testing.T) { + cmd := &cobra.Command{} + + type args struct { + err error + cmd *cobra.Command + debug bool + } + tests := []struct { + name string + args args + wantOut string + }{ + { + name: "generic error", + args: args{ + err: errors.New("the app exploded"), + cmd: nil, + debug: false, + }, + wantOut: "the app exploded\n", + }, + { + name: "DNS error", + args: args{ + err: fmt.Errorf("DNS oopsie: %w", &net.DNSError{ + Name: "api.github.com", + }), + cmd: nil, + debug: false, + }, + wantOut: `error connecting to api.github.com +check your internet connection or githubstatus.com +`, + }, + { + name: "Cobra flag error", + args: args{ + err: &command.FlagError{Err: errors.New("unknown flag --foo")}, + cmd: cmd, + debug: false, + }, + wantOut: "unknown flag --foo\n\nUsage:\n\n", + }, + { + name: "unknown Cobra command error", + args: args{ + err: errors.New("unknown command foo"), + cmd: cmd, + debug: false, + }, + wantOut: "unknown command foo\n\nUsage:\n\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &bytes.Buffer{} + printError(out, tt.args.err, tt.args.cmd, tt.args.debug) + if gotOut := out.String(); gotOut != tt.wantOut { + t.Errorf("printError() = %q, want %q", gotOut, tt.wantOut) + } + }) + } +} diff --git a/command/issue.go b/command/issue.go index 566393658..fea96ace6 100644 --- a/command/issue.go +++ b/command/issue.go @@ -11,8 +11,8 @@ import ( "time" "github.com/github/gh-cli/api" - "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/internal/ghrepo" "github.com/github/gh-cli/pkg/githubtemplate" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" @@ -47,7 +47,7 @@ var issueCmd = &cobra.Command{ An issue can be supplied as argument in any of the following formats: - by number, e.g. "123"; or -- by URL, e.g. "https://github.com///issues/123".`, +- by URL, e.g. "https://github.com/OWNER/REPO/issues/123".`, } var issueCreateCmd = &cobra.Command{ Use: "create", @@ -68,7 +68,7 @@ var issueViewCmd = &cobra.Command{ Use: "view { | | }", Args: func(cmd *cobra.Command, args []string) error { if len(args) < 1 { - return errors.New("requires an issue number as an argument") + return FlagError{errors.New("issue required as argument")} } return nil }, @@ -108,7 +108,7 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nIssues for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(baseRepo)) issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit) if err != nil { @@ -258,7 +258,7 @@ func printIssuePreview(out io.Writer, issue *api.Issue) { var issueURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/issues/(\d+)`) -func issueFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.Issue, error) { +func issueFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.Issue, error) { if issueNumber, err := strconv.Atoi(arg); err == nil { return api.IssueByNumber(apiClient, baseRepo, issueNumber) } @@ -279,7 +279,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) var templateFiles []string if rootDir, err := git.ToplevelDir(); err == nil { @@ -289,7 +289,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { // TODO: move URL generation into GitHubRepository - openURL := fmt.Sprintf("https://github.com/%s/%s/issues/new", baseRepo.RepoOwner(), baseRepo.RepoName()) + openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(baseRepo)) if len(templateFiles) > 1 { openURL += "/choose" } @@ -307,7 +307,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } if !repo.HasIssuesEnabled { - return fmt.Errorf("the '%s/%s' repository has disabled issues", baseRepo.RepoOwner(), baseRepo.RepoName()) + return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) } action := SubmitAction @@ -347,18 +347,13 @@ func issueCreate(cmd *cobra.Command, args []string) error { if action == PreviewAction { openURL := fmt.Sprintf( - "https://github.com/%s/%s/issues/new/?title=%s&body=%s", - baseRepo.RepoOwner(), - baseRepo.RepoName(), + "https://github.com/%s/issues/new/?title=%s&body=%s", + ghrepo.FullName(baseRepo), url.QueryEscape(title), url.QueryEscape(body), ) // TODO could exceed max url length for explorer - url, err := url.Parse(openURL) - if err != nil { - return err - } - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s%s in your browser.\n", url.Host, url.Path) + fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", displayURL(openURL)) return utils.OpenInBrowser(openURL) } else if action == SubmitAction { params := map[string]interface{}{ @@ -417,3 +412,11 @@ func labelList(issue api.Issue) string { } return list } + +func displayURL(urlStr string) string { + u, err := url.Parse(urlStr) + if err != nil { + return urlStr + } + return u.Hostname() + u.Path +} diff --git a/command/issue_test.go b/command/issue_test.go index e9f616f68..9cf4156b4 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -203,7 +203,7 @@ func TestIssueView(t *testing.T) { http := initFakeHTTP() http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "issue": { + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { "number": 123, "url": "https://github.com/OWNER/REPO/issues/123" } } } } @@ -236,7 +236,7 @@ func TestIssueView_preview(t *testing.T) { http := initFakeHTTP() http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "issue": { + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { "number": 123, "body": "**bold story**", "title": "ix of coins", @@ -303,12 +303,29 @@ func TestIssueView_notFound(t *testing.T) { } } +func TestIssueView_disabledIssues(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueViewCmd, `issue view 6666`) + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue view`: %v", err) + } +} + func TestIssueView_urlArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "issue": { + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { "number": 123, "url": "https://github.com/OWNER/REPO/issues/123" } } } } diff --git a/command/pr.go b/command/pr.go index 5204c1b3d..02e67814f 100644 --- a/command/pr.go +++ b/command/pr.go @@ -13,6 +13,7 @@ import ( "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/internal/ghrepo" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -42,8 +43,8 @@ var prCmd = &cobra.Command{ A pull request can be supplied as argument in any of the following formats: - by number, e.g. "123"; -- by URL, e.g. "https://github.com///pull/123"; or -- by the name of its head branch, e.g. "patch-1" or ":patch-1".`, +- by URL, e.g. "https://github.com/OWNER/REPO/pull/123"; or +- by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1".`, } var prCheckoutCmd = &cobra.Command{ Use: "checkout { | | }", @@ -67,9 +68,13 @@ var prStatusCmd = &cobra.Command{ RunE: prStatus, } var prViewCmd = &cobra.Command{ - Use: "view { | | }", + Use: "view [{ | | }]", Short: "View a pull request in the browser", - RunE: prView, + Long: `View a pull request specified by the argument in the browser. + +Without an argument, the pull request that belongs to the current +branch is opened.`, + RunE: prView, } func prStatus(cmd *cobra.Command, args []string) error { @@ -139,7 +144,7 @@ func prList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s/%s\n\n", baseRepo.RepoOwner(), baseRepo.RepoName()) + fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(baseRepo)) limit, err := cmd.Flags().GetInt("limit") if err != nil { @@ -275,7 +280,7 @@ func prView(cmd *cobra.Command, args []string) error { } if prNumber > 0 { - openURL = fmt.Sprintf("https://github.com/%s/%s/pull/%d", baseRepo.RepoOwner(), baseRepo.RepoName(), prNumber) + openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(baseRepo), prNumber) if preview { pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber) if err != nil { @@ -285,10 +290,6 @@ func prView(cmd *cobra.Command, args []string) error { } else { pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner) if err != nil { - var notFoundErr *api.NotFoundError - if errors.As(err, ¬FoundErr) { - return fmt.Errorf("%s. To open a specific pull request use the pull request's number as an argument", err) - } return err } @@ -323,7 +324,7 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) { var prURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`) -func prFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.PullRequest, error) { +func prFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.PullRequest, error) { if prNumber, err := strconv.Atoi(arg); err == nil { return api.PullRequestByNumber(apiClient, baseRepo, prNumber) } @@ -357,7 +358,7 @@ func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef st var branchOwner string if branchConfig.RemoteURL != nil { // the branch merges from a remote specified by URL - if r, err := context.RepoFromURL(branchConfig.RemoteURL); err == nil { + if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil { branchOwner = r.RepoOwner() } } else if branchConfig.RemoteName != "" { diff --git a/command/pr_create.go b/command/pr_create.go index 007ba8638..45dabd10c 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -1,12 +1,16 @@ package command import ( + "errors" "fmt" "net/url" + "sort" + "time" "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/internal/ghrepo" "github.com/github/gh-cli/pkg/githubtemplate" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" @@ -14,43 +18,95 @@ import ( func prCreate(cmd *cobra.Command, _ []string) error { ctx := contextForCommand(cmd) - - ucc, err := git.UncommittedChangeCount() + remotes, err := ctx.Remotes() if err != nil { return err } - if ucc > 0 { + + client, err := apiClientForContext(ctx) + if err != nil { + return fmt.Errorf("could not initialize API client: %w", err) + } + + baseRepoOverride, _ := cmd.Flags().GetString("repo") + repoContext, err := resolveRemotesToRepos(remotes, client, baseRepoOverride) + if err != nil { + return err + } + + baseRepo, err := repoContext.BaseRepo() + if err != nil { + return fmt.Errorf("could not determine base repository: %w", err) + } + + headBranch, err := ctx.Branch() + if err != nil { + return fmt.Errorf("could not determine the current branch: %w", err) + } + + baseBranch, err := cmd.Flags().GetString("base") + if err != nil { + return err + } + if baseBranch == "" { + baseBranch = baseRepo.DefaultBranchRef.Name + } + + didForkRepo := false + var headRemote *context.Remote + headRepo, err := repoContext.HeadRepo() + if err != nil { + if baseRepo.IsPrivate { + return fmt.Errorf("cannot write to private repository '%s'", ghrepo.FullName(baseRepo)) + } + headRepo, err = api.ForkRepo(client, baseRepo) + if err != nil { + return fmt.Errorf("error forking repo: %w", err) + } + didForkRepo = true + // TODO: support non-HTTPS git remote URLs + baseRepoURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(baseRepo)) + headRepoURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(headRepo)) + // TODO: figure out what to name the new git remote + gitRemote, err := git.AddRemote("fork", baseRepoURL, headRepoURL) + if err != nil { + return fmt.Errorf("error adding remote: %w", err) + } + headRemote = &context.Remote{ + Remote: gitRemote, + Owner: headRepo.RepoOwner(), + Repo: headRepo.RepoName(), + } + } + + if headBranch == baseBranch && ghrepo.IsSame(baseRepo, headRepo) { + return fmt.Errorf("must be on a branch named differently than %q", baseBranch) + } + + if headRemote == nil { + headRemote, err = repoContext.RemoteForRepo(headRepo) + if err != nil { + return fmt.Errorf("git remote not found for head repository: %w", err) + } + } + + if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change")) } - - repo, err := ctx.BaseRepo() - if err != nil { - return fmt.Errorf("could not determine GitHub repo: %w", err) - } - - head, err := ctx.Branch() - if err != nil { - return fmt.Errorf("could not determine current branch: %w", err) - } - - remote, err := guessRemote(ctx) - if err != nil { - return err - } - - target, err := cmd.Flags().GetString("base") - if err != nil { - return err - } - if target == "" { - // TODO use default branch - target = "master" - } - - fmt.Fprintf(colorableErr(cmd), "\nCreating pull request for %s into %s in %s/%s\n\n", utils.Cyan(head), utils.Cyan(target), repo.RepoOwner(), repo.RepoName()) - - if err = git.Push(remote, fmt.Sprintf("HEAD:%s", head)); err != nil { - return err + pushTries := 0 + maxPushTries := 3 + for { + // TODO: respect existing upstream configuration of the current branch + if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", headBranch)); err != nil { + if didForkRepo && pushTries < maxPushTries { + pushTries++ + // first wait 2 seconds after forking, then 4s, then 6s + time.Sleep(time.Duration(2*pushTries) * time.Second) + continue + } + return err + } + break } isWeb, err := cmd.Flags().GetBool("web") @@ -58,11 +114,20 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("could not parse web: %q", err) } if isWeb { - openURL := fmt.Sprintf(`https://github.com/%s/%s/pull/%s`, repo.RepoOwner(), repo.RepoName(), head) + openURL := fmt.Sprintf(`https://github.com/%s/pull/%s`, ghrepo.FullName(headRepo), headBranch) fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } + headBranchLabel := headBranch + if !ghrepo.IsSame(baseRepo, headRepo) { + headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) + } + fmt.Fprintf(colorableErr(cmd), "\nCreating pull request for %s into %s in %s\n\n", + utils.Cyan(headBranchLabel), + utils.Cyan(baseBranch), + ghrepo.FullName(baseRepo)) + title, err := cmd.Flags().GetString("title") if err != nil { return fmt.Errorf("could not parse title: %w", err) @@ -103,20 +168,6 @@ func prCreate(cmd *cobra.Command, _ []string) error { } } - base, err := cmd.Flags().GetString("base") - if err != nil { - return fmt.Errorf("could not parse base: %w", err) - } - if base == "" { - // TODO: use default branch for the repo - base = "master" - } - - client, err := apiClientForContext(ctx) - if err != nil { - return fmt.Errorf("could not initialize API client: %w", err) - } - isDraft, err := cmd.Flags().GetBool("draft") if err != nil { return fmt.Errorf("could not parse draft: %w", err) @@ -127,11 +178,11 @@ func prCreate(cmd *cobra.Command, _ []string) error { "title": title, "body": body, "draft": isDraft, - "baseRefName": base, - "headRefName": head, + "baseRefName": baseBranch, + "headRefName": headBranch, } - pr, err := api.CreatePullRequest(client, repo, params) + pr, err := api.CreatePullRequest(client, baseRepo, params) if err != nil { return fmt.Errorf("failed to create pull request: %w", err) } @@ -139,20 +190,15 @@ func prCreate(cmd *cobra.Command, _ []string) error { fmt.Fprintln(cmd.OutOrStdout(), pr.URL) } else if action == PreviewAction { openURL := fmt.Sprintf( - "https://github.com/%s/%s/compare/%s...%s?expand=1&title=%s&body=%s", - repo.RepoOwner(), - repo.RepoName(), - target, - head, + "https://github.com/%s/compare/%s...%s?expand=1&title=%s&body=%s", + ghrepo.FullName(baseRepo), + baseBranch, + headBranchLabel, url.QueryEscape(title), url.QueryEscape(body), ) // TODO could exceed max url length for explorer - url, err := url.Parse(openURL) - if err != nil { - return err - } - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s%s in your browser.\n", url.Host, url.Path) + fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", displayURL(openURL)) return utils.OpenInBrowser(openURL) } else { panic("Unreachable state") @@ -162,22 +208,6 @@ func prCreate(cmd *cobra.Command, _ []string) error { } -func guessRemote(ctx context.Context) (string, error) { - remotes, err := ctx.Remotes() - if err != nil { - return "", fmt.Errorf("could not read git remotes: %w", err) - } - - // TODO: consolidate logic with fsContext.BaseRepo - // TODO: check if the GH repo that the remote points to is writeable - remote, err := remotes.FindByName("upstream", "github", "origin", "*") - if err != nil { - return "", fmt.Errorf("could not determine suitable remote: %w", err) - } - - return remote.Name, nil -} - var prCreateCmd = &cobra.Command{ Use: "create", Short: "Create a pull request", @@ -195,3 +225,97 @@ func init() { "The branch into which you want your code merged") prCreateCmd.Flags().BoolP("web", "w", false, "Open the web browser to create a pull request") } + +// cap the number of git remotes looked up, since the user might have an +// unusally large number of git remotes +const maxRemotesForLookup = 5 + +func resolveRemotesToRepos(remotes context.Remotes, client *api.Client, base string) (resolvedRemotes, error) { + sort.Stable(remotes) + lenRemotesForLookup := len(remotes) + if lenRemotesForLookup > maxRemotesForLookup { + lenRemotesForLookup = maxRemotesForLookup + } + + hasBaseOverride := base != "" + baseOverride := ghrepo.FromFullName(base) + foundBaseOverride := false + repos := []ghrepo.Interface{} + for _, r := range remotes[:lenRemotesForLookup] { + repos = append(repos, r) + if ghrepo.IsSame(r, baseOverride) { + foundBaseOverride = true + } + } + if hasBaseOverride && !foundBaseOverride { + // additionally, look up the explicitly specified base repo if it's not + // already covered by git remotes + repos = append(repos, baseOverride) + } + + result := resolvedRemotes{remotes: remotes} + if hasBaseOverride { + result.baseOverride = baseOverride + } + networkResult, err := api.RepoNetwork(client, repos) + if err != nil { + return result, err + } + result.network = networkResult + return result, nil +} + +type resolvedRemotes struct { + baseOverride ghrepo.Interface + remotes context.Remotes + network api.RepoNetworkResult +} + +// BaseRepo is the first found repository in the "upstream", "github", "origin" +// git remote order, resolved to the parent repo if the git remote points to a fork +func (r resolvedRemotes) BaseRepo() (*api.Repository, error) { + if r.baseOverride != nil { + for _, repo := range r.network.Repositories { + if repo != nil && ghrepo.IsSame(repo, r.baseOverride) { + return repo, nil + } + } + return nil, fmt.Errorf("failed looking up information about the '%s' repository", + ghrepo.FullName(r.baseOverride)) + } + + for _, repo := range r.network.Repositories { + if repo == nil { + continue + } + if repo.IsFork() { + return repo.Parent, nil + } + return repo, nil + } + + return nil, errors.New("not found") +} + +// HeadRepo is the first found repository that has push access +func (r resolvedRemotes) HeadRepo() (*api.Repository, error) { + for _, repo := range r.network.Repositories { + if repo != nil && repo.ViewerCanPush() { + return repo, nil + } + } + return nil, errors.New("none of the repositories have push access") +} + +// RemoteForRepo finds the git remote that points to a repository +func (r resolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*context.Remote, error) { + for i, remote := range r.remotes { + if ghrepo.IsSame(remote, repo) || + // additionally, look up the resolved repository name in case this + // git remote points to this repository via a redirect + (r.network.Repositories[i] != nil && ghrepo.IsSame(r.network.Repositories[i], repo)) { + return remote, nil + } + } + return nil, errors.New("not found") +} diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 96ac2eb25..94f040f27 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -10,8 +10,10 @@ import ( "strings" "testing" + "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/internal/ghrepo" "github.com/github/gh-cli/test" "github.com/github/gh-cli/utils" ) @@ -52,8 +54,15 @@ func TestPRCreate(t *testing.T) { http := initFakeHTTP() http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "id": "REPOID" + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" } } } `)) http.StubResponse(200, bytes.NewBufferString(` @@ -103,7 +112,20 @@ func TestPRCreate_web(t *testing.T) { initContext = func() context.Context { return ctx } - initFakeHTTP() + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { @@ -116,11 +138,7 @@ func TestPRCreate_web(t *testing.T) { eq(t, err, nil) eq(t, output.String(), "") - eq(t, output.Stderr(), ` -Creating pull request for feature into master in OWNER/REPO - -Opening https://github.com/OWNER/REPO/pull/feature in your browser. -`) + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/pull/feature in your browser.\n") eq(t, len(ranCommands), 3) eq(t, strings.Join(ranCommands[1], " "), "git push --set-upstream origin HEAD:feature") @@ -139,8 +157,15 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { http := initFakeHTTP() http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "id": "REPOID" + { "data": { "repo_000": { + "id": "REPOID", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" } } } `)) http.StubResponse(200, bytes.NewBufferString(` @@ -165,3 +190,111 @@ Creating pull request for feature into master in OWNER/REPO `) } + +func Test_resolvedRemotes_clonedFork(t *testing.T) { + resolved := resolvedRemotes{ + baseOverride: nil, + remotes: context.Remotes{ + &context.Remote{ + Remote: &git.Remote{Name: "origin"}, + Owner: "OWNER", + Repo: "REPO", + }, + }, + network: api.RepoNetworkResult{ + Repositories: []*api.Repository{ + &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "OWNER"}, + ViewerPermission: "ADMIN", + Parent: &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "PARENTOWNER"}, + ViewerPermission: "READ", + }, + }, + }, + }, + } + + baseRepo, err := resolved.BaseRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(baseRepo), "PARENTOWNER/REPO") + baseRemote, err := resolved.RemoteForRepo(baseRepo) + if baseRemote != nil || err == nil { + t.Error("did not expect any remote for base") + } + + headRepo, err := resolved.HeadRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(headRepo), "OWNER/REPO") + headRemote, err := resolved.RemoteForRepo(headRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if headRemote.Name != "origin" { + t.Errorf("got remote %q", headRemote.Name) + } +} + +func Test_resolvedRemotes_triangularSetup(t *testing.T) { + resolved := resolvedRemotes{ + baseOverride: nil, + remotes: context.Remotes{ + &context.Remote{ + Remote: &git.Remote{Name: "origin"}, + Owner: "OWNER", + Repo: "REPO", + }, + &context.Remote{ + Remote: &git.Remote{Name: "fork"}, + Owner: "MYSELF", + Repo: "REPO", + }, + }, + network: api.RepoNetworkResult{ + Repositories: []*api.Repository{ + &api.Repository{ + Name: "NEWNAME", + Owner: api.RepositoryOwner{Login: "NEWOWNER"}, + ViewerPermission: "READ", + }, + &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "MYSELF"}, + ViewerPermission: "ADMIN", + }, + }, + }, + } + + baseRepo, err := resolved.BaseRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(baseRepo), "NEWOWNER/NEWNAME") + baseRemote, err := resolved.RemoteForRepo(baseRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if baseRemote.Name != "origin" { + t.Errorf("got remote %q", baseRemote.Name) + } + + headRepo, err := resolved.HeadRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(headRepo), "MYSELF/REPO") + headRemote, err := resolved.RemoteForRepo(headRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if headRemote.Name != "fork" { + t.Errorf("got remote %q", headRemote.Name) + } +} diff --git a/command/pr_test.go b/command/pr_test.go index 5a324cab1..9c17c3d4f 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -359,7 +359,7 @@ func TestPRView_noResultsForBranch(t *testing.T) { defer restoreCmd() _, err := RunCommand(prViewCmd, "pr view") - if err == nil || err.Error() != `no open pull requests found for branch "blueberries". To open a specific pull request use the pull request's number as an argument` { + if err == nil || err.Error() != `no open pull requests found for branch "blueberries"` { t.Errorf("error running command `pr view`: %v", err) } diff --git a/command/root.go b/command/root.go index 5703f30c2..be94146d8 100644 --- a/command/root.go +++ b/command/root.go @@ -35,13 +35,21 @@ func init() { // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { - return FlagError{err} + return &FlagError{Err: err} }) } // FlagError is the kind of error raised in flag processing type FlagError struct { - error + Err error +} + +func (fe FlagError) Error() string { + return fe.Err.Error() +} + +func (fe FlagError) Unwrap() error { + return fe.Err } // RootCmd is the entry point of command-line execution @@ -84,7 +92,7 @@ func BasicClient() (*api.Client, error) { opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", c.Token))) } if verbose := os.Getenv("DEBUG"); verbose != "" { - opts = append(opts, api.VerboseLog(os.Stderr)) + opts = append(opts, api.VerboseLog(os.Stderr, false)) } return api.NewClient(opts...), nil } @@ -93,7 +101,6 @@ func contextForCommand(cmd *cobra.Command) context.Context { ctx := initContext() if repo, err := cmd.Flags().GetString("repo"); err == nil && repo != "" { ctx.SetBaseRepo(repo) - ctx.SetBranch("master") } return ctx } @@ -113,7 +120,7 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { api.AddHeader("GraphQL-Features", "pe_mobile"), } if verbose := os.Getenv("DEBUG"); verbose != "" { - opts = append(opts, api.VerboseLog(os.Stderr)) + opts = append(opts, api.VerboseLog(os.Stderr, strings.Contains(verbose, "api"))) } return api.NewClient(opts...), nil } diff --git a/context/blank_context.go b/context/blank_context.go index 733ef38a8..779917e50 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/internal/ghrepo" ) // NewBlank initializes a blank Context suitable for testing @@ -17,22 +18,10 @@ type blankContext struct { authToken string authLogin string branch string - baseRepo GitHubRepository + baseRepo ghrepo.Interface remotes Remotes } -type ghRepo struct { - owner string - name string -} - -func (r ghRepo) RepoOwner() string { - return r.owner -} -func (r ghRepo) RepoName() string { - return r.name -} - func (c *blankContext) AuthToken() (string, error) { return c.authToken, nil } @@ -75,7 +64,7 @@ func (c *blankContext) SetRemotes(stubs map[string]string) { } } -func (c *blankContext) BaseRepo() (GitHubRepository, error) { +func (c *blankContext) BaseRepo() (ghrepo.Interface, error) { if c.baseRepo != nil { return c.baseRepo, nil } @@ -90,8 +79,5 @@ func (c *blankContext) BaseRepo() (GitHubRepository, error) { } func (c *blankContext) SetBaseRepo(nwo string) { - parts := strings.SplitN(nwo, "/", 2) - if len(parts) == 2 { - c.baseRepo = &ghRepo{parts[0], parts[1]} - } + c.baseRepo = ghrepo.FromFullName(nwo) } diff --git a/context/config_file.go b/context/config_file.go index 3079adef4..f3b07393b 100644 --- a/context/config_file.go +++ b/context/config_file.go @@ -10,6 +10,8 @@ import ( "gopkg.in/yaml.v3" ) +const defaultHostname = "github.com" + type configEntry struct { User string Token string `yaml:"oauth_token"` diff --git a/context/config_success.go b/context/config_success.go index 3a6fe978b..1919fc1a4 100644 --- a/context/config_success.go +++ b/context/config_success.go @@ -6,27 +6,37 @@ const oauthSuccessPage = ` Success: GitHub CLI - -

Authentication successful.

-

- You have completed logging into GitHub CLI.
- You may now close this tab and return to the terminal. -

- + +
+

Successfully authenticated GitHub CLI

+

You may now close this tab and return to the terminal.

+
` diff --git a/context/context.go b/context/context.go index c71e6d905..66bd3bda0 100644 --- a/context/context.go +++ b/context/context.go @@ -2,9 +2,9 @@ package context import ( "path" - "strings" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/internal/ghrepo" "github.com/mitchellh/go-homedir" ) @@ -16,16 +16,10 @@ type Context interface { Branch() (string, error) SetBranch(string) Remotes() (Remotes, error) - BaseRepo() (GitHubRepository, error) + BaseRepo() (ghrepo.Interface, error) SetBaseRepo(string) } -// GitHubRepository is anything that can be mapped to an OWNER/REPO pair -type GitHubRepository interface { - RepoOwner() string - RepoName() string -} - // New initializes a Context that reads from the filesystem func New() Context { return &fsContext{} @@ -36,7 +30,7 @@ type fsContext struct { config *configEntry remotes Remotes branch string - baseRepo GitHubRepository + baseRepo ghrepo.Interface authToken string } @@ -115,7 +109,7 @@ func (c *fsContext) Remotes() (Remotes, error) { return c.remotes, nil } -func (c *fsContext) BaseRepo() (GitHubRepository, error) { +func (c *fsContext) BaseRepo() (ghrepo.Interface, error) { if c.baseRepo != nil { return c.baseRepo, nil } @@ -134,8 +128,5 @@ func (c *fsContext) BaseRepo() (GitHubRepository, error) { } func (c *fsContext) SetBaseRepo(nwo string) { - parts := strings.SplitN(nwo, "/", 2) - if len(parts) == 2 { - c.baseRepo = &ghRepo{parts[0], parts[1]} - } + c.baseRepo = ghrepo.FromFullName(nwo) } diff --git a/context/remote.go b/context/remote.go index a660a85cb..2018a9cde 100644 --- a/context/remote.go +++ b/context/remote.go @@ -6,10 +6,9 @@ import ( "strings" "github.com/github/gh-cli/git" + "github.com/github/gh-cli/internal/ghrepo" ) -const defaultHostname = "github.com" - // Remotes represents a set of git remotes type Remotes []*Remote @@ -35,6 +34,26 @@ func (r Remotes) FindByRepo(owner, name string) (*Remote, error) { return nil, fmt.Errorf("no matching remote found") } +func remoteNameSortScore(name string) int { + switch strings.ToLower(name) { + case "upstream": + return 3 + case "github": + return 2 + case "origin": + return 1 + default: + return 0 + } +} + +// https://golang.org/pkg/sort/#Interface +func (r Remotes) Len() int { return len(r) } +func (r Remotes) Swap(i, j int) { r[i], r[j] = r[j], r[i] } +func (r Remotes) Less(i, j int) bool { + return remoteNameSortScore(r[i].Name) > remoteNameSortScore(r[j].Name) +} + // Remote represents a git remote mapped to a GitHub repository type Remote struct { *git.Remote @@ -55,39 +74,18 @@ func (r Remote) RepoOwner() string { // TODO: accept an interface instead of git.RemoteSet func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url.URL) (remotes Remotes) { for _, r := range gitRemotes { - var owner string - var repo string + var repo ghrepo.Interface if r.FetchURL != nil { - owner, repo, _ = repoFromURL(urlTranslate(r.FetchURL)) + repo, _ = ghrepo.FromURL(urlTranslate(r.FetchURL)) } - if r.PushURL != nil && owner == "" { - owner, repo, _ = repoFromURL(urlTranslate(r.PushURL)) + if r.PushURL != nil && repo == nil { + repo, _ = ghrepo.FromURL(urlTranslate(r.PushURL)) } remotes = append(remotes, &Remote{ Remote: r, - Owner: owner, - Repo: repo, + Owner: repo.RepoOwner(), + Repo: repo.RepoName(), }) } return } - -// RepoFromURL maps a URL to a GitHubRepository -func RepoFromURL(u *url.URL) (GitHubRepository, error) { - owner, repo, err := repoFromURL(u) - if err != nil { - return nil, err - } - return ghRepo{owner, repo}, nil -} - -func repoFromURL(u *url.URL) (string, string, error) { - if !strings.EqualFold(u.Hostname(), defaultHostname) { - return "", "", fmt.Errorf("unsupported hostname: %s", u.Hostname()) - } - parts := strings.SplitN(strings.TrimPrefix(u.Path, "/"), "/", 3) - if len(parts) < 2 { - return "", "", fmt.Errorf("invalid path: %s", u.Path) - } - return parts[0], strings.TrimSuffix(parts[1], ".git"), nil -} diff --git a/context/remote_test.go b/context/remote_test.go index 359fcaa7f..e083926d8 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -2,38 +2,11 @@ package context import ( "errors" - "net/url" "testing" "github.com/github/gh-cli/git" ) -func Test_repoFromURL(t *testing.T) { - u, _ := url.Parse("http://github.com/monalisa/octo-cat.git") - owner, repo, err := repoFromURL(u) - eq(t, err, nil) - eq(t, owner, "monalisa") - eq(t, repo, "octo-cat") -} - -func Test_repoFromURL_invalid(t *testing.T) { - cases := [][]string{ - []string{ - "https://example.com/one/two", - "unsupported hostname: example.com", - }, - []string{ - "/path/to/disk", - "unsupported hostname: ", - }, - } - for _, c := range cases { - u, _ := url.Parse(c[0]) - _, _, err := repoFromURL(u) - eq(t, err, errors.New(c[1])) - } -} - func Test_Remotes_FindByName(t *testing.T) { list := Remotes{ &Remote{Remote: &git.Remote{Name: "mona"}, Owner: "monalisa", Repo: "myfork"}, diff --git a/git/remote.go b/git/remote.go index ba29049c2..9bb24146f 100644 --- a/git/remote.go +++ b/git/remote.go @@ -2,8 +2,11 @@ package git import ( "net/url" + "os/exec" "regexp" "strings" + + "github.com/github/gh-cli/utils" ) var remoteRE = regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) @@ -67,3 +70,35 @@ func parseRemotes(gitRemotes []string) (remotes RemoteSet) { } return } + +// AddRemote adds a new git remote. The initURL is the remote URL with which the +// automatic fetch is made and finalURL, if non-blank, is set as the remote URL +// after the fetch. +func AddRemote(name, initURL, finalURL string) (*Remote, error) { + addCmd := exec.Command("git", "remote", "add", "-f", name, initURL) + err := utils.PrepareCmd(addCmd).Run() + if err != nil { + return nil, err + } + + if finalURL == "" { + finalURL = initURL + } else { + setCmd := exec.Command("git", "remote", "set-url", name, finalURL) + err := utils.PrepareCmd(setCmd).Run() + if err != nil { + return nil, err + } + } + + finalURLParsed, err := url.Parse(initURL) + if err != nil { + return nil, err + } + + return &Remote{ + Name: name, + FetchURL: finalURLParsed, + PushURL: finalURLParsed, + }, nil +} diff --git a/internal/ghrepo/repo.go b/internal/ghrepo/repo.go new file mode 100644 index 000000000..76ab93a4b --- /dev/null +++ b/internal/ghrepo/repo.go @@ -0,0 +1,61 @@ +package ghrepo + +import ( + "fmt" + "net/url" + "strings" +) + +const defaultHostname = "github.com" + +type Interface interface { + RepoName() string + RepoOwner() string +} + +func New(owner, repo string) Interface { + return &ghRepo{ + owner: owner, + name: repo, + } +} +func FullName(r Interface) string { + return fmt.Sprintf("%s/%s", r.RepoOwner(), r.RepoName()) +} + +func FromFullName(nwo string) Interface { + r := ghRepo{} + parts := strings.SplitN(nwo, "/", 2) + if len(parts) == 2 { + r.owner, r.name = parts[0], parts[1] + } + return &r +} + +func FromURL(u *url.URL) (Interface, error) { + if !strings.EqualFold(u.Hostname(), defaultHostname) { + return nil, fmt.Errorf("unsupported hostname: %s", u.Hostname()) + } + parts := strings.SplitN(strings.TrimPrefix(u.Path, "/"), "/", 3) + if len(parts) < 2 { + return nil, fmt.Errorf("invalid path: %s", u.Path) + } + return New(parts[0], strings.TrimSuffix(parts[1], ".git")), nil +} + +func IsSame(a, b Interface) bool { + return strings.EqualFold(a.RepoOwner(), b.RepoOwner()) && + strings.EqualFold(a.RepoName(), b.RepoName()) +} + +type ghRepo struct { + owner string + name string +} + +func (r ghRepo) RepoOwner() string { + return r.owner +} +func (r ghRepo) RepoName() string { + return r.name +} diff --git a/internal/ghrepo/repo_test.go b/internal/ghrepo/repo_test.go new file mode 100644 index 000000000..6ff775c84 --- /dev/null +++ b/internal/ghrepo/repo_test.go @@ -0,0 +1,40 @@ +package ghrepo + +import ( + "net/url" + "testing" +) + +func Test_repoFromURL(t *testing.T) { + u, _ := url.Parse("http://github.com/monalisa/octo-cat.git") + repo, err := FromURL(u) + if err != nil { + t.Fatalf("got error %q", err) + } + if repo.RepoOwner() != "monalisa" { + t.Errorf("got owner %q", repo.RepoOwner()) + } + if repo.RepoName() != "octo-cat" { + t.Errorf("got name %q", repo.RepoName()) + } +} + +func Test_repoFromURL_invalid(t *testing.T) { + cases := [][]string{ + []string{ + "https://example.com/one/two", + "unsupported hostname: example.com", + }, + []string{ + "/path/to/disk", + "unsupported hostname: ", + }, + } + for _, c := range cases { + u, _ := url.Parse(c[0]) + _, err := FromURL(u) + if err == nil || err.Error() != c[1] { + t.Errorf("got %q", err) + } + } +}