Merge remote-tracking branch 'origin' into webInList

This commit is contained in:
Mislav Marohnić 2020-07-16 14:51:34 +02:00
commit 8626befd3f
35 changed files with 1132 additions and 453 deletions

View file

@ -89,23 +89,37 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption {
var issuedScopesWarning bool
const (
httpOAuthAppID = "X-Oauth-Client-Id"
httpOAuthScopes = "X-Oauth-Scopes"
)
// CheckScopes checks whether an OAuth scope is present in a response
func CheckScopes(wantedScope string, cb func(string) error) ClientOption {
wantedCandidates := []string{wantedScope}
if strings.HasPrefix(wantedScope, "read:") {
wantedCandidates = append(wantedCandidates, "admin:"+strings.TrimPrefix(wantedScope, "read:"))
}
return func(tr http.RoundTripper) http.RoundTripper {
return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
res, err := tr.RoundTrip(req)
if err != nil || res.StatusCode > 299 || issuedScopesWarning {
_, hasHeader := res.Header[httpOAuthAppID]
if err != nil || res.StatusCode > 299 || !hasHeader || issuedScopesWarning {
return res, err
}
appID := res.Header.Get("X-Oauth-Client-Id")
hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",")
appID := res.Header.Get(httpOAuthAppID)
hasScopes := strings.Split(res.Header.Get(httpOAuthScopes), ",")
hasWanted := false
outer:
for _, s := range hasScopes {
if wantedScope == strings.TrimSpace(s) {
hasWanted = true
break
for _, w := range wantedCandidates {
if w == strings.TrimSpace(s) {
hasWanted = true
break outer
}
}
}

View file

@ -4,6 +4,7 @@ import (
"bytes"
"errors"
"io/ioutil"
"net/http"
"reflect"
"testing"
@ -91,5 +92,89 @@ func TestRESTError(t *testing.T) {
}
if httpErr.Error() != "HTTP 422: OH NO (https://api.github.com/repos/branch)" {
t.Errorf("got %q", httpErr.Error())
}
}
func Test_CheckScopes(t *testing.T) {
tests := []struct {
name string
wantScope string
responseApp string
responseScopes string
expectCallback bool
}{
{
name: "missing read:org",
wantScope: "read:org",
responseApp: "APPID",
responseScopes: "repo, gist",
expectCallback: true,
},
{
name: "has read:org",
wantScope: "read:org",
responseApp: "APPID",
responseScopes: "repo, read:org, gist",
expectCallback: false,
},
{
name: "has admin:org",
wantScope: "read:org",
responseApp: "APPID",
responseScopes: "repo, admin:org, gist",
expectCallback: false,
},
{
name: "no scopes in response",
wantScope: "read:org",
responseApp: "",
responseScopes: "",
expectCallback: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tr := &httpmock.Registry{}
tr.Register(httpmock.MatchAny, func(*http.Request) (*http.Response, error) {
if tt.responseScopes == "" {
return &http.Response{StatusCode: 200}, nil
}
return &http.Response{
StatusCode: 200,
Header: http.Header{
"X-Oauth-Client-Id": []string{tt.responseApp},
"X-Oauth-Scopes": []string{tt.responseScopes},
},
}, nil
})
callbackInvoked := false
var gotAppID string
fn := CheckScopes(tt.wantScope, func(appID string) error {
callbackInvoked = true
gotAppID = appID
return nil
})
rt := fn(tr)
req, err := http.NewRequest("GET", "https://api.github.com/hello", nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
issuedScopesWarning = false
_, err = rt.RoundTrip(req)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if tt.expectCallback != callbackInvoked {
t.Fatalf("expected CheckScopes callback: %v", tt.expectCallback)
}
if tt.expectCallback && gotAppID != tt.responseApp {
t.Errorf("unexpected app ID: %q", gotAppID)
}
})
}
}

View file

@ -432,9 +432,6 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
}
headRepository {
name
defaultBranchRef {
name
}
}
isCrossRepository
isDraft

View file

@ -34,6 +34,9 @@ type Repository struct {
}
Parent *Repository
// pseudo-field that keeps track of host name of this repo
hostname string
}
// RepositoryOwner is the owner of a GitHub repository
@ -51,6 +54,11 @@ func (r Repository) RepoName() string {
return r.Name
}
// RepoHost is the GitHub hostname of the repository
func (r Repository) RepoHost() string {
return r.hostname
}
// IsFork is true when this repository has a parent repository
func (r Repository) IsFork() bool {
return r.Parent != nil
@ -103,7 +111,19 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
return nil, err
}
return &result.Repository, nil
return initRepoHostname(&result.Repository, repo.RepoHost()), nil
}
func RepoDefaultBranch(client *Client, repo ghrepo.Interface) (string, error) {
if r, ok := repo.(*Repository); ok && r.DefaultBranchRef.Name != "" {
return r.DefaultBranchRef.Name, nil
}
r, err := GitHubRepo(client, repo)
if err != nil {
return "", err
}
return r.DefaultBranchRef.Name, nil
}
// RepoParent finds out the parent repository of a fork
@ -133,7 +153,7 @@ func RepoParent(client *Client, repo ghrepo.Interface) (ghrepo.Interface, error)
return nil, nil
}
parent := ghrepo.New(query.Repository.Parent.Owner.Login, query.Repository.Parent.Name)
parent := ghrepo.NewWithHost(query.Repository.Parent.Owner.Login, query.Repository.Parent.Name, repo.RepoHost())
return parent, nil
}
@ -145,6 +165,11 @@ type RepoNetworkResult struct {
// RepoNetwork inspects the relationship between multiple GitHub repositories
func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, error) {
var hostname string
if len(repos) > 0 {
hostname = repos[0].RepoHost()
}
queries := make([]string, 0, len(repos))
for i, repo := range repos {
queries = append(queries, fmt.Sprintf(`
@ -227,7 +252,7 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e
if err := decoder.Decode(&repo); err != nil {
return result, err
}
result.Repositories = append(result.Repositories, &repo)
result.Repositories = append(result.Repositories, initRepoHostname(&repo, hostname))
} else {
return result, fmt.Errorf("unknown GraphQL result key %q", name)
}
@ -235,6 +260,14 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e
return result, nil
}
func initRepoHostname(repo *Repository, hostname string) *Repository {
repo.hostname = hostname
if repo.Parent != nil {
repo.Parent.hostname = hostname
}
return repo
}
// repositoryV3 is the repository result from GitHub API v3
type repositoryV3 struct {
NodeID string
@ -265,6 +298,7 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
Login: result.Owner.Login,
},
ViewerPermission: "WRITE",
hostname: repo.RepoHost(),
}, nil
}
@ -306,7 +340,7 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) {
// `affiliations` condition, to guard against versions of GitHub with a
// faulty `affiliations` implementation
if len(forks) > 0 && forks[0].ViewerCanPush() {
return &forks[0], nil
return initRepoHostname(&forks[0], repo.RepoHost()), nil
}
return nil, &NotFoundError{errors.New("no fork found")}
}
@ -368,7 +402,8 @@ func RepoCreate(client *Client, input RepoCreateInput) (*Repository, error) {
return nil, err
}
return &response.CreateRepository.Repository, nil
// FIXME: support Enterprise hosts
return initRepoHostname(&response.CreateRepository.Repository, "github.com"), nil
}
func RepositoryReadme(client *Client, fullName string) (string, error) {

View file

@ -6,6 +6,7 @@ import (
"io"
"net"
"os"
"os/exec"
"path"
"strings"
@ -40,11 +41,27 @@ func main() {
cmd, _, err := command.RootCmd.Traverse(expandedArgs)
if err != nil || cmd == command.RootCmd {
originalArgs := expandedArgs
expandedArgs, err = command.ExpandAlias(os.Args)
isShell := false
expandedArgs, isShell, err = command.ExpandAlias(os.Args)
if err != nil {
fmt.Fprintf(stderr, "failed to process aliases: %s\n", err)
os.Exit(2)
}
if isShell {
err = command.ExecuteShellAlias(expandedArgs)
if err != nil {
if ee, ok := err.(*exec.ExitError); ok {
os.Exit(ee.ExitCode())
}
fmt.Fprintf(stderr, "failed to run external command: %s", err)
os.Exit(3)
}
os.Exit(0)
}
if hasDebug {
fmt.Fprintf(stderr, "%v -> %v\n", originalArgs, expandedArgs)
}

View file

@ -16,21 +16,39 @@ func init() {
aliasCmd.AddCommand(aliasSetCmd)
aliasCmd.AddCommand(aliasListCmd)
aliasCmd.AddCommand(aliasDeleteCmd)
aliasSetCmd.Flags().BoolP("shell", "s", false, "Declare an alias to be passed through a shell interpreter")
}
var aliasCmd = &cobra.Command{
Use: "alias",
Short: "Create shortcuts for gh commands",
Short: "Create command shortcuts",
Long: heredoc.Doc(`
Aliases can be used to make shortcuts for gh commands or to compose multiple commands.
Run "gh help alias set" to learn more.
`),
}
var aliasSetCmd = &cobra.Command{
Use: "set <alias> <expansion>",
Short: "Create a shortcut for a gh command",
Long: `Declare a word as a command alias that will expand to the specified command.
Long: heredoc.Doc(`
Declare a word as a command alias that will expand to the specified command(s).
The expansion may specify additional arguments and flags. If the expansion
includes positional placeholders such as '$1', '$2', etc., any extra arguments
that follow the invocation of an alias will be inserted appropriately.`,
The expansion may specify additional arguments and flags. If the expansion
includes positional placeholders such as '$1', '$2', etc., any extra arguments
that follow the invocation of an alias will be inserted appropriately.
If '--shell' is specified, the alias will be run through a shell interpreter (sh). This allows you
to compose commands with "|" or redirect with ">". Note that extra arguments following the alias
will not be automatically passed to the expanded expression. To have a shell alias receive
arguments, you must explicitly accept them using "$1", "$2", etc., or "$@" to accept all of them.
Platform note: on Windows, shell aliases are executed via "sh" as installed by Git For Windows. If
you have installed git on Windows in some other way, shell aliases may not work for you.
Quotes must always be used when defining a command as in the examples.`),
Example: heredoc.Doc(`
$ gh alias set pv 'pr view'
$ gh pv -w 123
@ -41,13 +59,12 @@ that follow the invocation of an alias will be inserted appropriately.`,
$ gh alias set epicsBy 'issue list --author="$1" --label="epic"'
$ gh epicsBy vilmibm
#=> gh issue list --author="vilmibm" --label="epic"
`),
Args: cobra.MinimumNArgs(2),
RunE: aliasSet,
// NB: this allows a user to eschew quotes when specifying an alias expansion. We'll have to
// revisit it if we ever want to add flags to alias set but we have no current plans for that.
DisableFlagParsing: true,
$ gh alias set --shell igrep 'gh issue list --label="$1" | grep $2'
$ gh igrep epic foo
#=> gh issue list --label="epic" | grep "foo"`),
Args: cobra.ExactArgs(2),
RunE: aliasSet,
}
func aliasSet(cmd *cobra.Command, args []string) error {
@ -63,19 +80,26 @@ func aliasSet(cmd *cobra.Command, args []string) error {
}
alias := args[0]
expansion := processArgs(args[1:])
expansionStr := strings.Join(expansion, " ")
expansion := args[1]
out := colorableOut(cmd)
fmt.Fprintf(out, "- Adding alias for %s: %s\n", utils.Bold(alias), utils.Bold(expansionStr))
fmt.Fprintf(out, "- Adding alias for %s: %s\n", utils.Bold(alias), utils.Bold(expansion))
if validCommand([]string{alias}) {
shell, err := cmd.Flags().GetBool("shell")
if err != nil {
return err
}
if shell && !strings.HasPrefix(expansion, "!") {
expansion = "!" + expansion
}
isExternal := strings.HasPrefix(expansion, "!")
if validCommand(alias) {
return fmt.Errorf("could not create alias: %q is already a gh command", alias)
}
if !validCommand(expansion) {
return fmt.Errorf("could not create alias: %s does not correspond to a gh command", utils.Bold(expansionStr))
if !isExternal && !validCommand(expansion) {
return fmt.Errorf("could not create alias: %s does not correspond to a gh command", expansion)
}
successMsg := fmt.Sprintf("%s Added alias.", utils.Green("✓"))
@ -86,11 +110,11 @@ func aliasSet(cmd *cobra.Command, args []string) error {
utils.Green("✓"),
utils.Bold(alias),
utils.Bold(oldExpansion),
utils.Bold(expansionStr),
utils.Bold(expansion),
)
}
err = aliasCfg.Add(alias, expansionStr)
err = aliasCfg.Add(alias, expansion)
if err != nil {
return fmt.Errorf("could not create alias: %s", err)
}
@ -100,28 +124,15 @@ func aliasSet(cmd *cobra.Command, args []string) error {
return nil
}
func validCommand(expansion []string) bool {
cmd, _, err := RootCmd.Traverse(expansion)
func validCommand(expansion string) bool {
split, err := shlex.Split(expansion)
if err != nil {
return false
}
cmd, _, err := RootCmd.Traverse(split)
return err == nil && cmd != RootCmd
}
func processArgs(args []string) []string {
if len(args) == 1 {
split, _ := shlex.Split(args[0])
return split
}
newArgs := []string{}
for _, a := range args {
if !strings.HasPrefix(a, "-") && strings.Contains(a, " ") {
a = fmt.Sprintf("%q", a)
}
newArgs = append(newArgs, a)
}
return newArgs
}
var aliasListCmd = &cobra.Command{
Use: "list",
Short: "List your aliases",

View file

@ -7,8 +7,19 @@ import (
"github.com/cli/cli/internal/config"
"github.com/cli/cli/test"
"github.com/stretchr/testify/assert"
)
func stubSh(value string) func() {
orig := findSh
findSh = func() (string, error) {
return value, nil
}
return func() {
findSh = orig
}
}
func TestAliasSet_gh_command(t *testing.T) {
initBlankContext("", "OWNER/REPO", "trunk")
@ -16,7 +27,7 @@ func TestAliasSet_gh_command(t *testing.T) {
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
_, err := RunCommand("alias set pr pr status")
_, err := RunCommand("alias set pr 'pr status'")
if err == nil {
t.Fatal("expected error")
}
@ -35,7 +46,7 @@ editor: vim
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
output, err := RunCommand("alias set co pr checkout")
output, err := RunCommand("alias set co 'pr checkout'")
if err != nil {
t.Fatalf("unexpected error: %s", err)
@ -65,7 +76,7 @@ aliases:
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
output, err := RunCommand("alias set co pr checkout -Rcool/repo")
output, err := RunCommand("alias set co 'pr checkout -Rcool/repo'")
if err != nil {
t.Fatalf("unexpected error: %s", err)
@ -81,7 +92,7 @@ func TestAliasSet_space_args(t *testing.T) {
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
output, err := RunCommand(`alias set il issue list -l 'cool story'`)
output, err := RunCommand(`alias set il 'issue list -l "cool story"'`)
if err != nil {
t.Fatalf("unexpected error: %s", err)
@ -99,23 +110,17 @@ func TestAliasSet_arg_processing(t *testing.T) {
ExpectedOutputLine string
ExpectedConfigLine string
}{
{"alias set co pr checkout", "- Adding alias for co: pr checkout", "co: pr checkout"},
{`alias set il "issue list"`, "- Adding alias for il: issue list", "il: issue list"},
{`alias set iz 'issue list'`, "- Adding alias for iz: issue list", "iz: issue list"},
{`alias set iy issue list --author=\$1 --label=\$2`,
`- Adding alias for iy: issue list --author=\$1 --label=\$2`,
`iy: issue list --author=\$1 --label=\$2`},
{`alias set ii 'issue list --author="$1" --label="$2"'`,
`- Adding alias for ii: issue list --author=\$1 --label=\$2`,
`ii: issue list --author=\$1 --label=\$2`},
`- Adding alias for ii: issue list --author="\$1" --label="\$2"`,
`ii: issue list --author="\$1" --label="\$2"`},
{`alias set ix issue list --author='$1' --label='$2'`,
`- Adding alias for ix: issue list --author=\$1 --label=\$2`,
`ix: issue list --author=\$1 --label=\$2`},
{`alias set ix "issue list --author='\$1' --label='\$2'"`,
`- Adding alias for ix: issue list --author='\$1' --label='\$2'`,
`ix: issue list --author='\$1' --label='\$2'`},
}
for _, c := range cases {
@ -143,7 +148,7 @@ editor: vim
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
output, err := RunCommand("alias set diff pr diff")
output, err := RunCommand("alias set diff 'pr diff'")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
@ -167,7 +172,7 @@ aliases:
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
output, err := RunCommand("alias set view pr view")
output, err := RunCommand("alias set view 'pr view'")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
@ -181,6 +186,48 @@ aliases:
}
func TestExpandAlias_shell(t *testing.T) {
defer stubSh("sh")()
cfg := `---
aliases:
ig: '!gh issue list | grep cool'
`
initBlankContext(cfg, "OWNER/REPO", "trunk")
expanded, isShell, err := ExpandAlias([]string{"gh", "ig"})
assert.True(t, isShell)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
expected := []string{"sh", "-c", "gh issue list | grep cool"}
assert.Equal(t, expected, expanded)
}
func TestExpandAlias_shell_extra_args(t *testing.T) {
defer stubSh("sh")()
cfg := `---
aliases:
ig: '!gh issue list --label=$1 | grep'
`
initBlankContext(cfg, "OWNER/REPO", "trunk")
expanded, isShell, err := ExpandAlias([]string{"gh", "ig", "bug", "foo"})
assert.True(t, isShell)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
expected := []string{"sh", "-c", "gh issue list --label=$1 | grep", "--", "bug", "foo"}
assert.Equal(t, expected, expanded)
}
func TestExpandAlias(t *testing.T) {
cfg := `---
aliases:
@ -212,7 +259,9 @@ aliases:
args = strings.Split(c.Args, " ")
}
out, err := ExpandAlias(args)
expanded, isShell, err := ExpandAlias(args)
assert.False(t, isShell)
if err == nil && c.Err != "" {
t.Errorf("expected error %s for %s", c.Err, c.Args)
@ -224,13 +273,13 @@ aliases:
continue
}
eq(t, out, c.ExpectedArgs)
assert.Equal(t, c.ExpectedArgs, expanded)
}
}
func TestAliasSet_invalid_command(t *testing.T) {
initBlankContext("", "OWNER/REPO", "trunk")
_, err := RunCommand("alias set co pe checkout")
_, err := RunCommand("alias set co 'pe checkout'")
if err == nil {
t.Fatal("expected error")
}
@ -319,3 +368,43 @@ aliases:
eq(t, mainBuf.String(), expected)
}
func TestShellAlias_flag(t *testing.T) {
initBlankContext("", "OWNER/REPO", "trunk")
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
output, err := RunCommand("alias set --shell igrep 'gh issue list | grep'")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
test.ExpectLines(t, output.String(), "Adding alias for igrep")
expected := `aliases:
igrep: '!gh issue list | grep'
`
eq(t, mainBuf.String(), expected)
}
func TestShellAlias_bang(t *testing.T) {
initBlankContext("", "OWNER/REPO", "trunk")
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
defer config.StubWriteConfig(&mainBuf, &hostsBuf)()
output, err := RunCommand("alias set igrep '!gh issue list | grep'")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
test.ExpectLines(t, output.String(), "Adding alias for igrep")
expected := `aliases:
igrep: '!gh issue list | grep'
`
eq(t, mainBuf.String(), expected)
}

View file

@ -1,11 +1,9 @@
package command
import (
"errors"
"fmt"
"io"
"net/url"
"regexp"
"strconv"
"strings"
"time"
@ -256,8 +254,9 @@ func issueList(cmd *cobra.Command, args []string) error {
})
title := listHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters)
// TODO: avoid printing header if piped to a script
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
if connectedToTerminal(cmd) {
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
}
out := cmd.OutOrStdout()
@ -330,12 +329,7 @@ func issueView(cmd *cobra.Command, args []string) error {
return err
}
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
if err != nil {
return err
}
issue, err := issueFromArg(apiClient, baseRepo, args[0])
issue, _, err := issueFromArg(ctx, apiClient, cmd, args[0])
if err != nil {
return err
}
@ -350,8 +344,11 @@ func issueView(cmd *cobra.Command, args []string) error {
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
return utils.OpenInBrowser(openURL)
}
out := colorableOut(cmd)
return printIssuePreview(out, issue)
if connectedToTerminal(cmd) {
return printHumanIssuePreview(colorableOut(cmd), issue)
}
return printRawIssuePreview(cmd.OutOrStdout(), issue)
}
func issueStateTitleWithColor(state string) string {
@ -378,7 +375,28 @@ func listHeader(repoName string, itemName string, matchCount int, totalMatchCoun
return fmt.Sprintf("Showing %d of %s in %s", matchCount, utils.Pluralize(totalMatchCount, itemName), repoName)
}
func printIssuePreview(out io.Writer, issue *api.Issue) error {
func printRawIssuePreview(out io.Writer, issue *api.Issue) error {
assignees := issueAssigneeList(*issue)
labels := issueLabelList(*issue)
projects := issueProjectList(*issue)
// Print empty strings for empty values so the number of metadata lines is consistent when
// processing many issues with head and grep.
fmt.Fprintf(out, "title:\t%s\n", issue.Title)
fmt.Fprintf(out, "state:\t%s\n", issue.State)
fmt.Fprintf(out, "author:\t%s\n", issue.Author.Login)
fmt.Fprintf(out, "labels:\t%s\n", labels)
fmt.Fprintf(out, "comments:\t%d\n", issue.Comments.TotalCount)
fmt.Fprintf(out, "assignees:\t%s\n", assignees)
fmt.Fprintf(out, "projects:\t%s\n", projects)
fmt.Fprintf(out, "milestone:\t%s\n", issue.Milestone.Title)
fmt.Fprintln(out, "--")
fmt.Fprintln(out, issue.Body)
return nil
}
func printHumanIssuePreview(out io.Writer, issue *api.Issue) error {
now := time.Now()
ago := now.Sub(issue.CreatedAt)
@ -427,21 +445,6 @@ func printIssuePreview(out io.Writer, issue *api.Issue) error {
return nil
}
var issueURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/issues/(\d+)`)
func issueFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.Issue, error) {
if issueNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#")); err == nil {
return api.IssueByNumber(apiClient, baseRepo, issueNumber)
}
if m := issueURLRE.FindStringSubmatch(arg); m != nil {
issueNumber, _ := strconv.Atoi(m[3])
return api.IssueByNumber(apiClient, baseRepo, issueNumber)
}
return nil, fmt.Errorf("invalid issue format: %q", arg)
}
func issueCreate(cmd *cobra.Command, args []string) error {
ctx := contextForCommand(cmd)
apiClient, err := apiClientForContext(ctx)
@ -497,8 +500,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/issues/new", ghrepo.FullName(baseRepo))
openURL := generateRepoURL(baseRepo, "issues/new")
if title != "" || body != "" {
milestone := ""
if len(milestoneTitles) > 0 {
@ -536,6 +538,10 @@ func issueCreate(cmd *cobra.Command, args []string) error {
interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body"))
if interactive && !connectedToTerminal(cmd) {
return fmt.Errorf("must provide --title and --body when not attached to a terminal")
}
if interactive {
var legacyTemplateFile *string
if baseOverride == "" {
@ -570,7 +576,7 @@ func issueCreate(cmd *cobra.Command, args []string) error {
}
if action == PreviewAction {
openURL := fmt.Sprintf("https://github.com/%s/issues/new/", ghrepo.FullName(baseRepo))
openURL := generateRepoURL(baseRepo, "issues/new")
milestone := ""
if len(milestoneTitles) > 0 {
milestone = milestoneTitles[0]
@ -606,6 +612,14 @@ func issueCreate(cmd *cobra.Command, args []string) error {
return nil
}
func generateRepoURL(repo ghrepo.Interface, p string, args ...interface{}) string {
baseURL := fmt.Sprintf("https://%s/%s/%s", repo.RepoHost(), repo.RepoOwner(), repo.RepoName())
if p != "" {
return baseURL + "/" + fmt.Sprintf(p, args...)
}
return baseURL
}
func addMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *issueMetadataState) error {
if !tb.HasMetadata() {
return nil
@ -697,9 +711,16 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue)
now := time.Now()
ago := now.Sub(issue.UpdatedAt)
table.AddField(issueNum, nil, colorFuncForState(issue.State))
if !table.IsTTY() {
table.AddField(issue.State, nil, nil)
}
table.AddField(replaceExcessiveWhitespace(issue.Title), nil, nil)
table.AddField(labels, nil, utils.Gray)
table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray)
if table.IsTTY() {
table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray)
} else {
table.AddField(issue.UpdatedAt.String(), nil, nil)
}
table.EndRow()
}
_ = table.Render()
@ -771,19 +792,11 @@ func issueClose(cmd *cobra.Command, args []string) error {
return err
}
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
issue, baseRepo, err := issueFromArg(ctx, apiClient, cmd, args[0])
if err != nil {
return err
}
issue, err := issueFromArg(apiClient, baseRepo, args[0])
var idErr *api.IssuesDisabledError
if errors.As(err, &idErr) {
return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo))
} else if err != nil {
return err
}
if issue.Closed {
fmt.Fprintf(colorableErr(cmd), "%s Issue #%d (%s) is already closed\n", utils.Yellow("!"), issue.Number, issue.Title)
return nil
@ -791,7 +804,7 @@ func issueClose(cmd *cobra.Command, args []string) error {
err = api.IssueClose(apiClient, baseRepo, *issue)
if err != nil {
return fmt.Errorf("API call failed:%w", err)
return err
}
fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d (%s)\n", utils.Red("✔"), issue.Number, issue.Title)
@ -806,19 +819,11 @@ func issueReopen(cmd *cobra.Command, args []string) error {
return err
}
baseRepo, err := determineBaseRepo(apiClient, cmd, ctx)
issue, baseRepo, err := issueFromArg(ctx, apiClient, cmd, args[0])
if err != nil {
return err
}
issue, err := issueFromArg(apiClient, baseRepo, args[0])
var idErr *api.IssuesDisabledError
if errors.As(err, &idErr) {
return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo))
} else if err != nil {
return err
}
if !issue.Closed {
fmt.Fprintf(colorableErr(cmd), "%s Issue #%d (%s) is already open\n", utils.Yellow("!"), issue.Number, issue.Title)
return nil
@ -826,7 +831,7 @@ func issueReopen(cmd *cobra.Command, args []string) error {
err = api.IssueReopen(apiClient, baseRepo, *issue)
if err != nil {
return fmt.Errorf("API call failed:%w", err)
return err
}
fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d (%s)\n", utils.Green("✔"), issue.Number, issue.Title)

64
command/issue_lookup.go Normal file
View file

@ -0,0 +1,64 @@
package command
import (
"fmt"
"net/url"
"regexp"
"strconv"
"strings"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/internal/ghrepo"
"github.com/spf13/cobra"
)
func issueFromArg(ctx context.Context, apiClient *api.Client, cmd *cobra.Command, arg string) (*api.Issue, ghrepo.Interface, error) {
issue, baseRepo, err := issueFromURL(apiClient, arg)
if err != nil {
return nil, nil, err
}
if issue != nil {
return issue, baseRepo, nil
}
baseRepo, err = determineBaseRepo(apiClient, cmd, ctx)
if err != nil {
return nil, nil, fmt.Errorf("could not determine base repo: %w", err)
}
issueNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#"))
if err != nil {
return nil, nil, fmt.Errorf("invalid issue format: %q", arg)
}
issue, err = issueFromNumber(apiClient, baseRepo, issueNumber)
return issue, baseRepo, err
}
var issueURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/issues/(\d+)`)
func issueFromURL(apiClient *api.Client, s string) (*api.Issue, ghrepo.Interface, error) {
u, err := url.Parse(s)
if err != nil {
return nil, nil, nil
}
if u.Scheme != "https" && u.Scheme != "http" {
return nil, nil, nil
}
m := issueURLRE.FindStringSubmatch(u.Path)
if m == nil {
return nil, nil, nil
}
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
issueNumber, _ := strconv.Atoi(m[3])
issue, err := issueFromNumber(apiClient, repo, issueNumber)
return issue, repo, err
}
func issueFromNumber(apiClient *api.Client, repo ghrepo.Interface, issueNumber int) (*api.Issue, error) {
return api.IssueByNumber(apiClient, repo, issueNumber)
}

View file

@ -19,6 +19,7 @@ import (
func TestIssueStatus(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -108,8 +109,31 @@ func TestIssueStatus_disabledIssues(t *testing.T) {
}
}
func TestIssueList(t *testing.T) {
func TestIssueList_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query IssueList\b`),
httpmock.FileResponse("../test/fixtures/issueList.json"))
output, err := RunCommand("issue list")
if err != nil {
t.Errorf("error running command `issue list`: %v", err)
}
eq(t, output.Stderr(), "")
test.ExpectLines(t, output.String(),
`1[\t]+number won[\t]+label[\t]+\d+`,
`2[\t]+number too[\t]+label[\t]+\d+`,
`4[\t]+number fore[\t]+label[\t]+\d+`)
}
func TestIssueList_tty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -126,21 +150,14 @@ Showing 3 of 3 issues in OWNER/REPO
`)
expectedIssues := []*regexp.Regexp{
regexp.MustCompile(`(?m)^1\t.*won`),
regexp.MustCompile(`(?m)^2\t.*too`),
regexp.MustCompile(`(?m)^4\t.*fore`),
}
for _, r := range expectedIssues {
if !r.MatchString(output.String()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
}
test.ExpectLines(t, output.String(),
"number won",
"number too",
"number fore")
}
func TestIssueList_withFlags(t *testing.T) {
func TestIssueList_tty_withFlags(t *testing.T) {
defer stubTerminal(true)()
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -326,7 +343,90 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) {
eq(t, url, "https://github.com/OWNER/REPO/issues/123")
}
func TestIssueView_Preview(t *testing.T) {
func TestIssueView_nontty_Preview(t *testing.T) {
defer stubTerminal(false)()
tests := map[string]struct {
ownerRepo string
command string
fixture string
expectedOutputs []string
}{
"Open issue without metadata": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_preview.json",
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tOPEN`,
`comments:\t9`,
`author:\tmarseilles`,
`assignees:`,
`\*\*bold story\*\*`,
},
},
"Open issue with metadata": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_previewWithMetadata.json",
expectedOutputs: []string{
`title:\tix of coins`,
`assignees:\tmarseilles, monaco`,
`author:\tmarseilles`,
`state:\tOPEN`,
`comments:\t9`,
`labels:\tone, two, three, four, five`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`milestone:\tuluru\n`,
`\*\*bold story\*\*`,
},
},
"Open issue with empty body": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_previewWithEmptyBody.json",
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tOPEN`,
`author:\tmarseilles`,
`labels:\ttarot`,
},
},
"Closed issue": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_previewClosedState.json",
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tCLOSED`,
`\*\*bold story\*\*`,
`author:\tmarseilles`,
`labels:\ttarot`,
`\*\*bold story\*\*`,
},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
initBlankContext("", "OWNER/REPO", tc.ownerRepo)
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture))
output, err := RunCommand(tc.command)
if err != nil {
t.Errorf("error running command `%v`: %v", tc.command, err)
}
eq(t, output.Stderr(), "")
test.ExpectLines(t, output.String(), tc.expectedOutputs...)
})
}
}
func TestIssueView_tty_Preview(t *testing.T) {
defer stubTerminal(true)()
tests := map[string]struct {
ownerRepo string
command string
@ -403,6 +503,7 @@ func TestIssueView_Preview(t *testing.T) {
func TestIssueView_web_notFound(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "errors": [
@ -448,7 +549,6 @@ func TestIssueView_disabledIssues(t *testing.T) {
func TestIssueView_web_urlArg(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "hasIssuesEnabled": true, "issue": {
@ -478,6 +578,28 @@ func TestIssueView_web_urlArg(t *testing.T) {
eq(t, url, "https://github.com/OWNER/REPO/issues/123")
}
func TestIssueCreate_nontty_error(t *testing.T) {
defer stubTerminal(false)()
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true
} } }
`))
_, err := RunCommand(`issue create -t hello`)
if err == nil {
t.Fatal("expected error running command `issue create`")
}
assert.Equal(t, "must provide --title and --body when not attached to a terminal", err.Error())
}
func TestIssueCreate(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
@ -873,12 +995,8 @@ func TestIssueClose_issuesDisabled(t *testing.T) {
`))
_, err := RunCommand("issue close 13")
if err == nil {
t.Fatalf("expected error when issues are disabled")
}
if !strings.Contains(err.Error(), "issues disabled") {
t.Fatalf("got unexpected error: %s", err)
if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" {
t.Fatalf("got error: %v", err)
}
}
@ -946,11 +1064,7 @@ func TestIssueReopen_issuesDisabled(t *testing.T) {
`))
_, err := RunCommand("issue reopen 2")
if err == nil {
t.Fatalf("expected error when issues are disabled")
}
if !strings.Contains(err.Error(), "issues disabled") {
t.Fatalf("got unexpected error: %s", err)
if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" {
t.Fatalf("got error: %v", err)
}
}

View file

@ -519,23 +519,21 @@ func prMerge(cmd *cobra.Command, args []string) error {
fmt.Fprintf(colorableOut(cmd), "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title)
if deleteBranch {
repo, err := api.GitHubRepo(apiClient, baseRepo)
if err != nil {
return err
}
currentBranch, err := ctx.Branch()
if err != nil {
return err
}
branchSwitchString := ""
if deleteLocalBranch && !crossRepoPR {
currentBranch, err := ctx.Branch()
if err != nil {
return err
}
var branchToSwitchTo string
if currentBranch == pr.HeadRefName {
branchToSwitchTo = repo.DefaultBranchRef.Name
err = git.CheckoutBranch(repo.DefaultBranchRef.Name)
branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo)
if err != nil {
return err
}
err = git.CheckoutBranch(branchToSwitchTo)
if err != nil {
return err
}

View file

@ -5,9 +5,11 @@ import (
"fmt"
"os"
"os/exec"
"strings"
"github.com/spf13/cobra"
"github.com/cli/cli/api"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/internal/run"
@ -33,7 +35,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {
baseRemote, _ := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName())
// baseRemoteSpec is a repository URL or a remote name to be used in git fetch
baseURLOrName := formatRemoteURL(cmd, ghrepo.FullName(baseRepo))
baseURLOrName := formatRemoteURL(cmd, baseRepo)
if baseRemote != nil {
baseURLOrName = baseRemote.Name
}
@ -45,6 +47,9 @@ func prCheckout(cmd *cobra.Command, args []string) error {
var cmdQueue [][]string
newBranchName := pr.HeadRefName
if strings.HasPrefix(newBranchName, "-") {
return fmt.Errorf("invalid branch name: %q", newBranchName)
}
if headRemote != nil {
// there is an existing git remote for PR head
@ -65,8 +70,13 @@ func prCheckout(cmd *cobra.Command, args []string) error {
} else {
// no git remote for PR head
defaultBranchName, err := api.RepoDefaultBranch(apiClient, baseRepo)
if err != nil {
return err
}
// avoid naming the new branch the same as the default branch
if newBranchName == pr.HeadRepository.DefaultBranchRef.Name {
if newBranchName == defaultBranchName {
newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName)
}
@ -84,7 +94,8 @@ func prCheckout(cmd *cobra.Command, args []string) error {
remote := baseURLOrName
mergeRef := ref
if pr.MaintainerCanModify {
remote = formatRemoteURL(cmd, fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name))
headRepo := ghrepo.NewWithHost(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name, baseRepo.RepoHost())
remote = formatRemoteURL(cmd, headRepo)
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
}
if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" {

View file

@ -1,7 +1,6 @@
package command
import (
"bytes"
"encoding/json"
"io/ioutil"
"os/exec"
@ -10,7 +9,9 @@ import (
"github.com/cli/cli/context"
"github.com/cli/cli/internal/run"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/test"
"github.com/stretchr/testify/assert"
)
func TestPRCheckout_sameRepo(t *testing.T) {
@ -23,9 +24,10 @@ func TestPRCheckout_sameRepo(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -33,10 +35,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": false,
"maintainerCanModify": false
@ -76,7 +75,8 @@ func TestPRCheckout_urlArg(t *testing.T) {
return ctx
}
http := initFakeHTTP()
http.StubResponse(200, bytes.NewBufferString(`
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -84,10 +84,7 @@ func TestPRCheckout_urlArg(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": false,
"maintainerCanModify": false
@ -124,7 +121,8 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
return ctx
}
http := initFakeHTTP()
http.StubResponse(200, bytes.NewBufferString(`
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -132,15 +130,17 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "POE",
"defaultBranchRef": {
"name": "master"
}
"name": "POE"
},
"isCrossRepository": false,
"maintainerCanModify": false
} } } }
`))
http.Register(httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(`
{ "data": { "repository": {
"defaultBranchRef": {"name": "master"}
} } }
`))
ranCommands := [][]string{}
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
@ -185,9 +185,10 @@ func TestPRCheckout_branchArg(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequests": { "nodes": [
{ "number": 123,
"headRefName": "feature",
@ -195,10 +196,7 @@ func TestPRCheckout_branchArg(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": true,
"maintainerCanModify": false }
@ -235,9 +233,10 @@ func TestPRCheckout_existingBranch(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -245,10 +244,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": false,
"maintainerCanModify": false
@ -288,9 +284,10 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -298,10 +295,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": true,
"maintainerCanModify": false
@ -341,9 +335,10 @@ func TestPRCheckout_differentRepo(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -351,10 +346,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": true,
"maintainerCanModify": false
@ -394,9 +386,10 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -404,10 +397,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": true,
"maintainerCanModify": false
@ -445,9 +435,10 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -455,10 +446,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": true,
"maintainerCanModify": false
@ -486,6 +474,47 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
eq(t, strings.Join(ranCommands[1], " "), "git merge --ff-only FETCH_HEAD")
}
func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("feature")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "-foo",
"headRepositoryOwner": {
"login": "hubot"
},
"headRepository": {
"name": "REPO"
},
"isCrossRepository": true,
"maintainerCanModify": false
} } } }
`))
restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
t.Errorf("unexpected external invocation: %v", cmd.Args)
return &test.OutputStub{}
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
if assert.Errorf(t, err, "expected command to fail") {
assert.Equal(t, `invalid branch name: "-foo"`, err.Error())
}
assert.Equal(t, "", output.Stderr())
}
func TestPRCheckout_maintainerCanModify(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
@ -496,9 +525,10 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
return ctx
}
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
"number": 123,
"headRefName": "feature",
@ -506,10 +536,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
"login": "hubot"
},
"headRepository": {
"name": "REPO",
"defaultBranchRef": {
"name": "master"
}
"name": "REPO"
},
"isCrossRepository": true,
"maintainerCanModify": true

View file

@ -295,7 +295,7 @@ func prCreate(cmd *cobra.Command, _ []string) error {
// In either case, we want to add the head repo as a new git remote so we
// can push to it.
if headRemote == nil {
headRepoURL := formatRemoteURL(cmd, ghrepo.FullName(headRepo))
headRepoURL := formatRemoteURL(cmd, headRepo)
// TODO: prevent clashes with another remote of a same name
gitRemote, err := git.AddRemote("fork", headRepoURL)
@ -304,8 +304,7 @@ func prCreate(cmd *cobra.Command, _ []string) error {
}
headRemote = &context.Remote{
Remote: gitRemote,
Owner: headRepo.RepoOwner(),
Repo: headRepo.RepoName(),
Repo: headRepo,
}
}
@ -438,12 +437,7 @@ func withPrAndIssueQueryParams(baseURL, title, body string, assignees, labels, p
}
func generateCompareURL(r ghrepo.Interface, base, head, title, body string, assignees, labels, projects []string, milestone string) (string, error) {
u := fmt.Sprintf(
"https://github.com/%s/compare/%s...%s?expand=1",
ghrepo.FullName(r),
base,
head,
)
u := generateRepoURL(r, "compare/%s...%s?expand=1", base, head)
url, err := withPrAndIssueQueryParams(u, title, body, assignees, labels, projects, milestone)
if err != nil {
return "", err

View file

@ -9,6 +9,7 @@ import (
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/test"
)
@ -759,13 +760,11 @@ func Test_determineTrackingBranch_noMatch(t *testing.T) {
remotes := context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "hubot",
Repo: "Spoon-Knife",
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Owner: "octocat",
Repo: "Spoon-Knife",
Repo: ghrepo.New("octocat", "Spoon-Knife"),
},
}
@ -786,13 +785,11 @@ func Test_determineTrackingBranch_hasMatch(t *testing.T) {
remotes := context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "hubot",
Repo: "Spoon-Knife",
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Owner: "octocat",
Repo: "Spoon-Knife",
Repo: ghrepo.New("octocat", "Spoon-Knife"),
},
}
@ -819,8 +816,7 @@ func Test_determineTrackingBranch_respectTrackingConfig(t *testing.T) {
remotes := context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "hubot",
Repo: "Spoon-Knife",
Repo: ghrepo.New("hubot", "Spoon-Knife"),
},
}

View file

@ -2,6 +2,7 @@ package command
import (
"fmt"
"net/url"
"regexp"
"strconv"
"strings"
@ -55,16 +56,27 @@ func prFromNumberString(ctx context.Context, apiClient *api.Client, repo ghrepo.
return nil, nil
}
var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)
func prFromURL(ctx context.Context, apiClient *api.Client, s string) (*api.PullRequest, ghrepo.Interface, error) {
r := regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`)
if m := r.FindStringSubmatch(s); m != nil {
repo := ghrepo.New(m[1], m[2])
prNumberString := m[3]
pr, err := prFromNumberString(ctx, apiClient, repo, prNumberString)
return pr, repo, err
u, err := url.Parse(s)
if err != nil {
return nil, nil, nil
}
return nil, nil, nil
if u.Scheme != "https" && u.Scheme != "http" {
return nil, nil, nil
}
m := pullURLRE.FindStringSubmatch(u.Path)
if m == nil {
return nil, nil, nil
}
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
prNumberString := m[3]
pr, err := prFromNumberString(ctx, apiClient, repo, prNumberString)
return pr, repo, err
}
func prForCurrentBranch(ctx context.Context, apiClient *api.Client, repo ghrepo.Interface) (*api.PullRequest, error) {

View file

@ -967,28 +967,25 @@ func TestPRReopen_alreadyMerged(t *testing.T) {
func TestPrMerge(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"}
} } }`))
{ "data": { "repository": { "pullRequest": {
"id": "THE-ID",
"number": 1,
"title": "The title of the PR",
"state": "OPEN",
"headRefName": "blueberries",
"headRepositoryOwner": {"login": "OWNER"}
} } } }`))
http.Register(
httpmock.GraphQL(`mutation PullRequestMerge\b`),
httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
@ -1017,6 +1014,7 @@ func TestPrMerge(t *testing.T) {
func TestPrMerge_withRepoFlag(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
defer http.Verify(t)
http.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.GraphQLQuery(`
@ -1032,18 +1030,6 @@ func TestPrMerge_withRepoFlag(t *testing.T) {
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
@ -1065,6 +1051,7 @@ func TestPrMerge_withRepoFlag(t *testing.T) {
func TestPrMerge_deleteBranch(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestForBranch\b`),
@ -1075,15 +1062,6 @@ func TestPrMerge_deleteBranch(t *testing.T) {
assert.Equal(t, "PR_10", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
@ -1108,6 +1086,7 @@ func TestPrMerge_deleteBranch(t *testing.T) {
func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
initBlankContext("", "OWNER/REPO", "another-branch")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestForBranch\b`),
@ -1118,15 +1097,6 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
assert.Equal(t, "PR_10", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
@ -1149,6 +1119,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
func TestPrMerge_noPrNumberGiven(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestForBranch\b`),
@ -1159,15 +1130,6 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) {
assert.Equal(t, "PR_10", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
@ -1196,28 +1158,25 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) {
func TestPrMerge_rebase(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"pullRequest": { "number": 2, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"}
} } }`))
{ "data": { "repository": { "pullRequest": {
"id": "THE-ID",
"number": 2,
"title": "The title of the PR",
"state": "OPEN",
"headRefName": "blueberries",
"headRepositoryOwner": {"login": "OWNER"}
} } } }`))
http.Register(
httpmock.GraphQL(`mutation PullRequestMerge\b`),
httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
assert.Equal(t, "REBASE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
@ -1245,28 +1204,25 @@ func TestPrMerge_rebase(t *testing.T) {
func TestPrMerge_squash(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"pullRequest": { "number": 3, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"}
} } }`))
{ "data": { "repository": { "pullRequest": {
"id": "THE-ID",
"number": 3,
"title": "The title of the PR",
"state": "OPEN",
"headRefName": "blueberries",
"headRepositoryOwner": {"login": "OWNER"}
} } } }`))
http.Register(
httpmock.GraphQL(`mutation PullRequestMerge\b`),
httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
assert.Equal(t, "SQUASH", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))
@ -1284,16 +1240,14 @@ func TestPrMerge_squash(t *testing.T) {
t.Fatalf("error running command `pr merge`: %v", err)
}
r := regexp.MustCompile(`Squashed and merged pull request #3`)
if !r.MatchString(output.String()) {
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.String())
}
expected := "✔ Squashed and merged pull request #3 (The title of the PR)\n✔ Deleted branch blueberries\n"
assert.Equal(t, expected, output.String())
}
func TestPrMerge_alreadyMerged(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
@ -1325,6 +1279,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
func TestPRMerge_interactive(t *testing.T) {
initBlankContext("", "OWNER/REPO", "blueberries")
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query PullRequestForBranch\b`),
@ -1341,15 +1296,6 @@ func TestPRMerge_interactive(t *testing.T) {
assert.Equal(t, "THE-ID", input["pullRequestId"].(string))
assert.Equal(t, "MERGE", input["mergeMethod"].(string))
}))
http.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"defaultBranchRef": {"name": "master"}
}
}
}`))
http.Register(
httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
httpmock.StringResponse(`{}`))

View file

@ -186,7 +186,11 @@ func repoClone(cmd *cobra.Command, args []string) error {
}
cloneURL = currentUser + "/" + cloneURL
}
cloneURL = formatRemoteURL(cmd, cloneURL)
repo, err := ghrepo.FromFullName(cloneURL)
if err != nil {
return err
}
cloneURL = formatRemoteURL(cmd, repo)
}
var repo ghrepo.Interface
@ -221,7 +225,7 @@ func repoClone(cmd *cobra.Command, args []string) error {
}
func addUpstreamRemote(cmd *cobra.Command, parentRepo ghrepo.Interface, cloneDir string) error {
upstreamURL := formatRemoteURL(cmd, ghrepo.FullName(parentRepo))
upstreamURL := formatRemoteURL(cmd, parentRepo)
cloneCmd := git.GitCommand("-C", cloneDir, "remote", "add", "-f", "upstream", upstreamURL)
cloneCmd.Stdout = os.Stdout
@ -322,7 +326,7 @@ func repoCreate(cmd *cobra.Command, args []string) error {
fmt.Fprintln(out, repo.URL)
}
remoteURL := formatRemoteURL(cmd, ghrepo.FullName(repo))
remoteURL := formatRemoteURL(cmd, repo)
if projectDirErr == nil {
_, err = git.AddRemote("origin", remoteURL)
@ -497,7 +501,7 @@ func repoFork(cmd *cobra.Command, args []string) error {
fmt.Fprintf(out, "%s Renamed %s remote to %s\n", greenCheck, utils.Bold(remoteName), utils.Bold(renameTarget))
}
forkedRepoCloneURL := formatRemoteURL(cmd, ghrepo.FullName(forkedRepo))
forkedRepoCloneURL := formatRemoteURL(cmd, forkedRepo)
_, err = git.AddRemote(remoteName, forkedRepoCloneURL)
if err != nil {
@ -515,7 +519,7 @@ func repoFork(cmd *cobra.Command, args []string) error {
}
}
if cloneDesired {
forkedRepoCloneURL := formatRemoteURL(cmd, ghrepo.FullName(forkedRepo))
forkedRepoCloneURL := formatRemoteURL(cmd, forkedRepo)
cloneDir, err := runClone(forkedRepoCloneURL, []string{})
if err != nil {
return fmt.Errorf("failed to clone fork: %w", err)
@ -588,7 +592,7 @@ func repoView(cmd *cobra.Command, args []string) error {
fullName := ghrepo.FullName(toView)
openURL := fmt.Sprintf("https://github.com/%s", fullName)
openURL := generateRepoURL(toView, "")
if web {
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", displayURL(openURL))
return utils.OpenInBrowser(openURL)

View file

@ -6,7 +6,10 @@ import (
"io"
"net/http"
"os"
"os/exec"
"path/filepath"
"regexp"
"runtime"
"runtime/debug"
"strings"
@ -15,6 +18,7 @@ import (
"github.com/cli/cli/context"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/internal/run"
apiCmd "github.com/cli/cli/pkg/cmd/api"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
@ -330,23 +334,22 @@ func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Co
return baseRepo, nil
}
func formatRemoteURL(cmd *cobra.Command, fullRepoName string) string {
func formatRemoteURL(cmd *cobra.Command, repo ghrepo.Interface) string {
ctx := contextForCommand(cmd)
protocol := "https"
var protocol string
cfg, err := ctx.Config()
if err != nil {
fmt.Fprintf(colorableErr(cmd), "%s failed to load config: %s. using defaults\n", utils.Yellow("!"), err)
} else {
cfgProtocol, _ := cfg.Get(defaultHostname, "git_protocol")
protocol = cfgProtocol
protocol, _ = cfg.Get(repo.RepoHost(), "git_protocol")
}
if protocol == "ssh" {
return fmt.Sprintf("git@%s:%s.git", defaultHostname, fullRepoName)
return fmt.Sprintf("git@%s:%s/%s.git", repo.RepoHost(), repo.RepoOwner(), repo.RepoName())
}
return fmt.Sprintf("https://%s/%s.git", defaultHostname, fullRepoName)
return fmt.Sprintf("https://%s/%s/%s.git", repo.RepoHost(), repo.RepoOwner(), repo.RepoName())
}
func determineEditor(cmd *cobra.Command) (string, error) {
@ -363,25 +366,85 @@ func determineEditor(cmd *cobra.Command) (string, error) {
return editorCommand, nil
}
func ExpandAlias(args []string) ([]string, error) {
empty := []string{}
func ExecuteShellAlias(args []string) error {
externalCmd := exec.Command(args[0], args[1:]...)
externalCmd.Stderr = os.Stderr
externalCmd.Stdout = os.Stdout
externalCmd.Stdin = os.Stdin
preparedCmd := run.PrepareCmd(externalCmd)
return preparedCmd.Run()
}
var findSh = func() (string, error) {
shPath, err := exec.LookPath("sh")
if err == nil {
return shPath, nil
}
if runtime.GOOS == "windows" {
winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.")
// We can try and find a sh executable in a Git for Windows install
gitPath, err := exec.LookPath("git")
if err != nil {
return "", winNotFoundErr
}
shPath = filepath.Join(filepath.Dir(gitPath), "..", "bin", "sh.exe")
_, err = os.Stat(shPath)
if err != nil {
return "", winNotFoundErr
}
return shPath, nil
}
return "", errors.New("unable to locate sh to execute shell alias with")
}
// ExpandAlias processes argv to see if it should be rewritten according to a user's aliases. The
// second return value indicates whether the alias should be executed in a new shell process instead
// of running gh itself.
func ExpandAlias(args []string) (expanded []string, isShell bool, err error) {
err = nil
isShell = false
expanded = []string{}
if len(args) < 2 {
// the command is lacking a subcommand
return empty, nil
return
}
ctx := initContext()
cfg, err := ctx.Config()
if err != nil {
return empty, err
return
}
aliases, err := cfg.Aliases()
if err != nil {
return empty, err
return
}
expansion, ok := aliases.Get(args[1])
if ok {
if strings.HasPrefix(expansion, "!") {
isShell = true
shPath, shErr := findSh()
if shErr != nil {
err = shErr
return
}
expanded = []string{shPath, "-c", expansion[1:]}
if len(args[2:]) > 0 {
expanded = append(expanded, "--")
expanded = append(expanded, args[2:]...)
}
return
}
extraArgs := []string{}
for i, a := range args[2:] {
if !strings.Contains(expansion, "$") {
@ -392,18 +455,24 @@ func ExpandAlias(args []string) ([]string, error) {
}
lingeringRE := regexp.MustCompile(`\$\d`)
if lingeringRE.MatchString(expansion) {
return empty, fmt.Errorf("not enough arguments for alias: %s", expansion)
err = fmt.Errorf("not enough arguments for alias: %s", expansion)
return
}
newArgs, err := shlex.Split(expansion)
var newArgs []string
newArgs, err = shlex.Split(expansion)
if err != nil {
return nil, err
return
}
newArgs = append(newArgs, extraArgs...)
return newArgs, nil
expanded = append(newArgs, extraArgs...)
return
}
return args[1:], nil
expanded = args[1:]
return
}
func connectedToTerminal(cmd *cobra.Command) bool {
return utils.IsTerminal(cmd.InOrStdin()) && utils.IsTerminal(cmd.OutOrStdout())
}

View file

@ -2,6 +2,8 @@ package command
import (
"testing"
"github.com/cli/cli/internal/ghrepo"
)
func TestChangelogURL(t *testing.T) {
@ -43,7 +45,7 @@ func TestChangelogURL(t *testing.T) {
func TestRemoteURLFormatting_no_config(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
result := formatRemoteURL(repoForkCmd, "OWNER/REPO")
result := formatRemoteURL(repoForkCmd, ghrepo.New("OWNER", "REPO"))
eq(t, result, "https://github.com/OWNER/REPO.git")
}
@ -56,6 +58,6 @@ hosts:
git_protocol: ssh
`
initBlankContext(cfg, "OWNER/REPO", "master")
result := formatRemoteURL(repoForkCmd, "OWNER/REPO")
result := formatRemoteURL(repoForkCmd, ghrepo.New("OWNER", "REPO"))
eq(t, result, "git@github.com:OWNER/REPO.git")
}

View file

@ -12,6 +12,7 @@ import (
"github.com/cli/cli/context"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/utils"
"github.com/google/shlex"
"github.com/spf13/pflag"
)
@ -169,3 +170,26 @@ func (s errorStub) Output() ([]byte, error) {
func (s errorStub) Run() error {
return errors.New(s.message)
}
func stubTerminal(connected bool) func() {
isTerminal := utils.IsTerminal
utils.IsTerminal = func(_ interface{}) bool {
return connected
}
terminalSize := utils.TerminalSize
if connected {
utils.TerminalSize = func(_ interface{}) (int, int, error) {
return 80, 20, nil
}
} else {
utils.TerminalSize = func(_ interface{}) (int, int, error) {
return 0, 0, fmt.Errorf("terminal connection stubbed to false")
}
}
return func() {
utils.IsTerminal = isTerminal
utils.TerminalSize = terminalSize
}
}

View file

@ -62,8 +62,7 @@ func (c *blankContext) SetRemotes(stubs map[string]string) {
ownerWithName := strings.SplitN(repo, "/", 2)
c.remotes = append(c.remotes, &Remote{
Remote: &git.Remote{Name: remoteName},
Owner: ownerWithName[0],
Repo: ownerWithName[1],
Repo: ghrepo.New(ownerWithName[0], ownerWithName[1]),
})
}
}

View file

@ -5,6 +5,7 @@ import (
"fmt"
"os"
"sort"
"strings"
"github.com/cli/cli/api"
"github.com/cli/cli/git"
@ -31,22 +32,31 @@ type Context interface {
// unusually large number of git remotes
const maxRemotesForLookup = 5
// ResolveRemotesToRepos takes in a list of git remotes and fetches more information about the repositories they map to.
// Only the git remotes belonging to the same hostname are ever looked up; all others are ignored.
func ResolveRemotesToRepos(remotes 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 := make([]ghrepo.Interface, 0, lenRemotesForLookup)
for _, r := range remotes[:lenRemotesForLookup] {
var hostname string
var repos []ghrepo.Interface
for i, r := range remotes {
if i == 0 {
hostname = r.RepoHost()
} else if !strings.EqualFold(r.RepoHost(), hostname) {
// ignore all remotes for a hostname different to that of the 1st remote
continue
}
repos = append(repos, r)
if ghrepo.IsSame(r, baseOverride) {
foundBaseOverride = true
}
if len(repos) == maxRemotesForLookup {
break
}
}
if hasBaseOverride && !foundBaseOverride {
// additionally, look up the explicitly specified base repo if it's not

View file

@ -57,18 +57,22 @@ func (r Remotes) Less(i, j int) bool {
// Remote represents a git remote mapped to a GitHub repository
type Remote struct {
*git.Remote
Owner string
Repo string
Repo ghrepo.Interface
}
// RepoName is the name of the GitHub repository
func (r Remote) RepoName() string {
return r.Repo
return r.Repo.RepoName()
}
// RepoOwner is the name of the GitHub account that owns the repo
func (r Remote) RepoOwner() string {
return r.Owner
return r.Repo.RepoOwner()
}
// RepoHost is the GitHub hostname that the remote points to
func (r Remote) RepoHost() string {
return r.Repo.RepoHost()
}
// TODO: accept an interface instead of git.RemoteSet
@ -86,8 +90,7 @@ func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url
}
remotes = append(remotes, &Remote{
Remote: r,
Owner: repo.RepoOwner(),
Repo: repo.RepoName(),
Repo: repo,
})
}
return

View file

@ -22,9 +22,9 @@ func eq(t *testing.T, got interface{}, expected interface{}) {
func Test_Remotes_FindByName(t *testing.T) {
list := Remotes{
&Remote{Remote: &git.Remote{Name: "mona"}, Owner: "monalisa", Repo: "myfork"},
&Remote{Remote: &git.Remote{Name: "origin"}, Owner: "monalisa", Repo: "octo-cat"},
&Remote{Remote: &git.Remote{Name: "upstream"}, Owner: "hubot", Repo: "tools"},
&Remote{Remote: &git.Remote{Name: "mona"}, Repo: ghrepo.New("monalisa", "myfork")},
&Remote{Remote: &git.Remote{Name: "origin"}, Repo: ghrepo.New("monalisa", "octo-cat")},
&Remote{Remote: &git.Remote{Name: "upstream"}, Repo: ghrepo.New("hubot", "tools")},
}
r, err := list.FindByName("upstream", "origin")
@ -84,13 +84,11 @@ func Test_resolvedRemotes_triangularSetup(t *testing.T) {
Remotes: Remotes{
&Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "OWNER",
Repo: "REPO",
Repo: ghrepo.New("OWNER", "REPO"),
},
&Remote{
Remote: &git.Remote{Name: "fork"},
Owner: "MYSELF",
Repo: "REPO",
Repo: ghrepo.New("MYSELF", "REPO"),
},
},
Network: api.RepoNetworkResult{
@ -157,8 +155,7 @@ func Test_resolvedRemotes_forkLookup(t *testing.T) {
Remotes: Remotes{
&Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "OWNER",
Repo: "REPO",
Repo: ghrepo.New("OWNER", "REPO"),
},
},
Network: api.RepoNetworkResult{
@ -190,8 +187,7 @@ func Test_resolvedRemotes_clonedFork(t *testing.T) {
Remotes: Remotes{
&Remote{
Remote: &git.Remote{Name: "origin"},
Owner: "OWNER",
Repo: "REPO",
Repo: ghrepo.New("OWNER", "REPO"),
},
},
Network: api.RepoNetworkResult{

View file

@ -1,24 +1,37 @@
# Releasing
_First create a prerelease to verify the relase infrastructure_
Our build system automatically compiles and attaches cross-platform binaries to any git tag named `vX.Y.Z`. The automated changelog is generated from commit messages starting with “Merge pull request …” that landed between this tag and the previous one (as determined topologically by git).
1. `git tag v1.2.3-pre`
2. `git push origin v1.2.3-pre`
3. Wait several minutes for the build to run <https://github.com/cli/cli/actions>
4. Verify the prerelease succeeded and has the correct artifacts at <https://github.com/cli/cli/releases>
Users who run official builds of `gh` on their machines will get notified about the new version within a 24 hour period.
_Next create a the production release_
To test out the build system, publish a prerelease tag with a name such as `vX.Y.Z-pre.0` or `vX.Y.Z-rc.1`. Note that such a release will still be public, but it will be marked as a "prerelease", meaning that it will never show up as a "latest" release nor trigger upgrade notifications for users.
5. `git tag v1.2.3`
6. `git push origin v1.2.3`
7. Wait several minutes for the build to run <https://github.com/cli/cli/actions>
8. Check <https://github.com/cli/cli/releases>
9. Verify the marketing site was updated https://cli.github.com/
10. Delete the prerelease on GitHub <https://github.com/cli/cli/releases>
## General guidelines
* Features to be released should be reviewed and approved at least one day prior to the release.
* Feature releases should bump up the minor version number.
## Tagging a new release
1. `git tag v1.2.3 && git push origin v1.2.3`
2. Wait several minutes for builds to run: <https://github.com/cli/cli/actions>
3. Check <https://github.com/cli/cli/releases>
4. Scan generated release notes and optionally add a human touch by grouping items under topic sections
5. Verify the marketing site was updated: <https://cli.github.com>
6. (Optional) Delete any pre-releases related to this release.
A successful build will result in changes across several repositories:
* <https://github.com/github/cli.github.com>
* <https://github.com/github/homebrew-gh>
* <https://github.com/Homebrew/homebrew-core/pulls>
* <https://github.com/cli/scoop-gh>
If the build fails, there is not a clean way to re-run it. The easiest way would be to start over by deleting the partial release on GitHub and re-publishing the tag. Note that this might be disruptive to users or tooling that were already notified about an upgrade. If a functional release and its binaries are already out there, it might be better to try to manually fix up only the specific workflow tasks that failed. Use your best judgement depending on the failure type.
## Release locally for debugging
A local release can be created for testing without creating anything official on the release page.
0. Make sure GoReleaser is installed: `brew install goreleaser`
1. `goreleaser --skip-validate --skip-publish --rm-dist`
2. Check and test files in `dist/`
2. Find the built products under `dist/`.

View file

@ -25,6 +25,10 @@ just imagine a flowchart
- e.g. have already discussed not wanting to do or duplicate issue
- comment acknowledging receipt
- close
- do we want someone in the community to do it?
- e.g. the task is relatively straightforward, but no people on our team have the bandwidth to take it on at the moment
- ensure that the thread contains all the context necessary for someone new to pick this up
- add `help-wanted` label
- do we want to do it, but not in the next year?
- comment acknowledging it, but that we don't plan on working on it this year.
- add `future` label

View file

@ -6,13 +6,13 @@ import (
"strings"
)
// TODO these are sprinkled across command, context, config, and ghrepo
const defaultHostname = "github.com"
// Interface describes an object that represents a GitHub repository
type Interface interface {
RepoName() string
RepoOwner() string
RepoHost() string
}
// New instantiates a GitHub repository from owner and name arguments
@ -23,6 +23,15 @@ func New(owner, repo string) Interface {
}
}
// NewWithHost is like New with an explicit host name
func NewWithHost(owner, repo, hostname string) Interface {
return &ghRepo{
owner: owner,
name: repo,
hostname: hostname,
}
}
// FullName serializes a GitHub repository into an "OWNER/REPO" string
func FullName(r Interface) string {
return fmt.Sprintf("%s/%s", r.RepoOwner(), r.RepoName())
@ -39,32 +48,52 @@ func FromFullName(nwo string) (Interface, error) {
return &r, nil
}
// FromURL extracts the GitHub repository information from a URL
// FromURL extracts the GitHub repository information from a git remote URL
func FromURL(u *url.URL) (Interface, error) {
if !strings.EqualFold(u.Hostname(), defaultHostname) && !strings.EqualFold(u.Hostname(), "www."+defaultHostname) {
return nil, fmt.Errorf("unsupported hostname: %s", u.Hostname())
if u.Hostname() == "" {
return nil, fmt.Errorf("no hostname detected")
}
parts := strings.SplitN(strings.TrimPrefix(u.Path, "/"), "/", 3)
if len(parts) < 2 {
parts := strings.SplitN(strings.Trim(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
return &ghRepo{
owner: parts[0],
name: strings.TrimSuffix(parts[1], ".git"),
hostname: normalizeHostname(u.Hostname()),
}, nil
}
func normalizeHostname(h string) string {
return strings.ToLower(strings.TrimPrefix(h, "www."))
}
// IsSame compares two GitHub repositories
func IsSame(a, b Interface) bool {
return strings.EqualFold(a.RepoOwner(), b.RepoOwner()) &&
strings.EqualFold(a.RepoName(), b.RepoName())
strings.EqualFold(a.RepoName(), b.RepoName()) &&
normalizeHostname(a.RepoHost()) == normalizeHostname(b.RepoHost())
}
type ghRepo struct {
owner string
name string
owner string
name string
hostname string
}
func (r ghRepo) RepoOwner() string {
return r.owner
}
func (r ghRepo) RepoName() string {
return r.name
}
func (r ghRepo) RepoHost() string {
if r.hostname != "" {
return r.hostname
}
return defaultHostname
}

View file

@ -12,31 +12,78 @@ func Test_repoFromURL(t *testing.T) {
name string
input string
result string
host string
err error
}{
{
name: "github.com URL",
input: "https://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "github.com URL with trailing slash",
input: "https://github.com/monalisa/octo-cat/",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "www.github.com URL",
input: "http://www.GITHUB.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "unsupported hostname",
input: "https://example.com/one/two",
name: "too many path components",
input: "https://github.com/monalisa/octo-cat/pulls",
result: "",
err: errors.New("unsupported hostname: example.com"),
host: "",
err: errors.New("invalid path: /monalisa/octo-cat/pulls"),
},
{
name: "non-GitHub hostname",
input: "https://example.com/one/two",
result: "one/two",
host: "example.com",
err: nil,
},
{
name: "filesystem path",
input: "/path/to/file",
result: "",
err: errors.New("unsupported hostname: "),
host: "",
err: errors.New("no hostname detected"),
},
{
name: "filesystem path with scheme",
input: "file:///path/to/file",
result: "",
host: "",
err: errors.New("no hostname detected"),
},
{
name: "github.com SSH URL",
input: "ssh://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "github.com HTTPS+SSH URL",
input: "https+ssh://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
{
name: "github.com git URL",
input: "git://github.com/monalisa/octo-cat.git",
result: "monalisa/octo-cat",
host: "github.com",
err: nil,
},
}
@ -61,6 +108,9 @@ func Test_repoFromURL(t *testing.T) {
if tt.result != got {
t.Errorf("expected %q, got %q", tt.result, got)
}
if tt.host != repo.RepoHost() {
t.Errorf("expected %q, got %q", tt.host, repo.RepoHost())
}
})
}
}

View file

@ -1,6 +1,7 @@
package githubtemplate
import (
"fmt"
"io/ioutil"
"path"
"regexp"
@ -53,6 +54,8 @@ mainLoop:
// FindLegacy returns the file path of the default(legacy) template
func FindLegacy(rootDir string, name string) *string {
namePattern := regexp.MustCompile(fmt.Sprintf(`(?i)^%s(\.|$)`, strings.ReplaceAll(name, "_", "[_-]")))
// https://help.github.com/en/github/building-a-strong-community/creating-a-pull-request-template-for-your-repository
candidateDirs := []string{
path.Join(rootDir, ".github"),
@ -67,7 +70,7 @@ func FindLegacy(rootDir string, name string) *string {
// detect a single template file
for _, file := range files {
if strings.EqualFold(file.Name(), name+".md") {
if namePattern.MatchString(file.Name()) && !file.IsDir() {
result := path.Join(dir, file.Name())
return &result
}

View file

@ -160,7 +160,6 @@ func TestFindLegacy(t *testing.T) {
name: "Template in root",
prepare: []string{
"README.md",
"ISSUE_TEMPLATE",
"issue_template.md",
"issue_template.txt",
"pull_request_template.md",
@ -172,6 +171,32 @@ func TestFindLegacy(t *testing.T) {
},
want: path.Join(tmpdir, "issue_template.md"),
},
{
name: "No extension",
prepare: []string{
"README.md",
"issue_template",
"docs/issue_template.md",
},
args: args{
rootDir: tmpdir,
name: "ISSUE_TEMPLATE",
},
want: path.Join(tmpdir, "issue_template"),
},
{
name: "Dash instead of underscore",
prepare: []string{
"README.md",
"issue-template.txt",
"docs/issue_template.md",
},
args: args{
rootDir: tmpdir,
name: "ISSUE_TEMPLATE",
},
want: path.Join(tmpdir, "issue-template.txt"),
},
{
name: "Template in .github takes precedence",
prepare: []string{
@ -224,8 +249,11 @@ func TestFindLegacy(t *testing.T) {
file.Close()
}
if got := FindLegacy(tt.args.rootDir, tt.args.name); *got != tt.want {
t.Errorf("Find() = %v, want %v", got, tt.want)
got := FindLegacy(tt.args.rootDir, tt.args.name)
if got == nil {
t.Errorf("FindLegacy() = nil, want %v", tt.want)
} else if *got != tt.want {
t.Errorf("FindLegacy() = %v, want %v", *got, tt.want)
}
})
os.RemoveAll(tmpdir)

View file

@ -21,6 +21,7 @@ func (r *Registry) Register(m Matcher, resp Responder) {
type Testing interface {
Errorf(string, ...interface{})
Helper()
}
func (r *Registry) Verify(t Testing) {
@ -31,6 +32,7 @@ func (r *Registry) Verify(t Testing) {
}
}
if n > 0 {
t.Helper()
// NOTE: stubs offer no useful reflection, so we can't print details
// about dead stubs and what they were trying to match
t.Errorf("%d unmatched HTTP stubs", n)

View file

@ -5,7 +5,6 @@ import (
"os"
"github.com/mattn/go-colorable"
"github.com/mattn/go-isatty"
"github.com/mgutz/ansi"
)
@ -23,22 +22,12 @@ var (
Bold = makeColorFunc("default+b")
)
func isStdoutTerminal() bool {
if !checkedTerminal {
_isStdoutTerminal = IsTerminal(os.Stdout)
checkedTerminal = true
}
return _isStdoutTerminal
}
// IsTerminal reports whether the file descriptor is connected to a terminal
func IsTerminal(f *os.File) bool {
return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd())
}
// NewColorable returns an output stream that handles ANSI color sequences on Windows
func NewColorable(f *os.File) io.Writer {
return colorable.NewColorable(f)
func NewColorable(w io.Writer) io.Writer {
if f, isFile := w.(*os.File); isFile {
return colorable.NewColorable(f)
}
return w
}
func makeColorFunc(color string) func(string) string {

View file

@ -9,8 +9,6 @@ import (
"strings"
"github.com/cli/cli/pkg/text"
"github.com/mattn/go-isatty"
"golang.org/x/crypto/ssh/terminal"
)
type TablePrinter interface {
@ -21,26 +19,23 @@ type TablePrinter interface {
}
func NewTablePrinter(w io.Writer) TablePrinter {
if outFile, isFile := w.(*os.File); isFile {
// TODO: use utils.IsTerminal()
isCygwin := isatty.IsCygwinTerminal(outFile.Fd())
if isatty.IsTerminal(outFile.Fd()) || isCygwin {
ttyWidth := 80
if w, _, err := terminal.GetSize(int(outFile.Fd())); err == nil {
ttyWidth = w
} else if isCygwin {
tputCmd := exec.Command("tput", "cols")
tputCmd.Stdin = os.Stdin
if out, err := tputCmd.Output(); err == nil {
if w, err := strconv.Atoi(strings.TrimSpace(string(out))); err == nil {
ttyWidth = w
}
if IsTerminal(w) {
isCygwin := IsCygwinTerminal(w)
ttyWidth := 80
if termWidth, _, err := TerminalSize(w); err == nil {
ttyWidth = termWidth
} else if isCygwin {
tputCmd := exec.Command("tput", "cols")
tputCmd.Stdin = os.Stdin
if out, err := tputCmd.Output(); err == nil {
if w, err := strconv.Atoi(strings.TrimSpace(string(out))); err == nil {
ttyWidth = w
}
}
return &ttyTablePrinter{
out: NewColorable(outFile),
maxWidth: ttyWidth,
}
}
return &ttyTablePrinter{
out: NewColorable(w),
maxWidth: ttyWidth,
}
}
return &tsvTablePrinter{

44
utils/terminal.go Normal file
View file

@ -0,0 +1,44 @@
package utils
import (
"fmt"
"os"
"github.com/mattn/go-isatty"
"golang.org/x/crypto/ssh/terminal"
)
func isStdoutTerminal() bool {
if !checkedTerminal {
_isStdoutTerminal = IsTerminal(os.Stdout)
checkedTerminal = true
}
return _isStdoutTerminal
}
// TODO I don't like this use of interface{} but we need to accept both io.Writer and io.Reader
// interfaces.
var IsTerminal = func(w interface{}) bool {
if f, isFile := w.(*os.File); isFile {
return isatty.IsTerminal(f.Fd()) || IsCygwinTerminal(f)
}
return false
}
func IsCygwinTerminal(w interface{}) bool {
if f, isFile := w.(*os.File); isFile {
return isatty.IsCygwinTerminal(f.Fd())
}
return false
}
var TerminalSize = func(w interface{}) (int, int, error) {
if f, isFile := w.(*os.File); isFile {
return terminal.GetSize(int(f.Fd()))
}
return 0, 0, fmt.Errorf("%v is not a file", w)
}