From 14ce1f99a705d9c8e15a21afa62780f6e888eb02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 15 Apr 2020 14:28:07 +0200 Subject: [PATCH 01/30] Ask for an additional `read:org` OAuth scope This is to facilitate: - requesting teams for review on `pr create` - allowing `repo create ORG/REPO --team TEAM` --- auth/oauth.go | 10 ++++++++-- context/config_setup.go | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/auth/oauth.go b/auth/oauth.go index dc7a9d085..3f9d9ddc4 100644 --- a/auth/oauth.go +++ b/auth/oauth.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "os" + "strings" "github.com/cli/cli/pkg/browser" ) @@ -29,6 +30,7 @@ type OAuthFlow struct { Hostname string ClientID string ClientSecret string + Scopes []string WriteSuccessHTML func(io.Writer) VerboseStream io.Writer } @@ -45,11 +47,15 @@ func (oa *OAuthFlow) ObtainAccessToken() (accessToken string, err error) { } port := listener.Addr().(*net.TCPAddr).Port + scopes := "repo" + if oa.Scopes != nil { + scopes = strings.Join(oa.Scopes, " ") + } + q := url.Values{} q.Set("client_id", oa.ClientID) q.Set("redirect_uri", fmt.Sprintf("http://127.0.0.1:%d/callback", port)) - // TODO: make scopes configurable - q.Set("scope", "repo") + q.Set("scope", scopes) q.Set("state", state) startURL := fmt.Sprintf("https://%s/login/oauth/authorize?%s", oa.Hostname, q.Encode()) diff --git a/context/config_setup.go b/context/config_setup.go index 56bdfe55a..a55b8dfa6 100644 --- a/context/config_setup.go +++ b/context/config_setup.go @@ -36,6 +36,7 @@ func setupConfigFile(filename string) (*configEntry, error) { Hostname: oauthHost, ClientID: oauthClientID, ClientSecret: oauthClientSecret, + Scopes: []string{"repo", "read:org"}, WriteSuccessHTML: func(w io.Writer) { fmt.Fprintln(w, oauthSuccessPage) }, From 3d566dc5a6bb74abb5f8aa4ae763add14b4027d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 15 Apr 2020 17:25:15 +0200 Subject: [PATCH 02/30] Detect and warn about `read:org` OAuth scope being missing --- api/client.go | 41 +++++++++++++++++++++++++++++++++++++++++ command/root.go | 1 + 2 files changed, 42 insertions(+) diff --git a/api/client.go b/api/client.go index 2fc72aaa6..e1fef7219 100644 --- a/api/client.go +++ b/api/client.go @@ -7,6 +7,7 @@ import ( "io" "io/ioutil" "net/http" + "os" "regexp" "strings" @@ -63,6 +64,46 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption { } } +var issuedScopesWarning bool + +// CheckScopes checks whether an OAuth scope is present in a response +func CheckScopes(wantedScope string) ClientOption { + 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 || issuedScopesWarning { + return res, err + } + + isApp := res.Header.Get("X-Oauth-Client-Id") != "" + hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",") + + hasWanted := false + for _, s := range hasScopes { + if wantedScope == strings.TrimSpace(s) { + hasWanted = true + break + } + } + + if !hasWanted { + fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") + // TODO: offer to take the person through the authentication flow again? + // TODO: retry the original request if it was a read? + if isApp { + fmt.Fprintln(os.Stderr, "To re-authenticate, please `rm ~/.config/gh/config.yml` and try again.") + } else { + // the person has pasted a Personal Access Token + fmt.Fprintln(os.Stderr, "Re-generate your token in `rm ~/.config/gh/config.yml` and try again.") + } + issuedScopesWarning = true + } + + return res, nil + }} + } +} + type funcTripper struct { roundTrip func(*http.Request) (*http.Response, error) } diff --git a/command/root.go b/command/root.go index fa65d7e05..6eafd01d4 100644 --- a/command/root.go +++ b/command/root.go @@ -131,6 +131,7 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { opts = append(opts, apiVerboseLog()) } opts = append(opts, + api.CheckScopes("read:org"), api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), // antiope-preview: Checks From be51d095ea3ea9294851af28ed9b0a10564d03d1 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 21 Apr 2020 13:30:07 -0700 Subject: [PATCH 03/30] Separating core from non-core commands --- command/root.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/command/root.go b/command/root.go index fa65d7e05..60fe77daa 100644 --- a/command/root.go +++ b/command/root.go @@ -47,6 +47,8 @@ func init() { // TODO: // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") + RootCmd.SetHelpFunc(aTempHelpFuncfunc) + RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { return err @@ -206,3 +208,33 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac return baseRepo, nil } + +type helpEntry struct { + Title string + Body string +} + +func aTempHelpFuncfunc(command *cobra.Command, s []string) { + out := colorableOut(command) + fmt.Fprint(out, "\nWork seamlessly with GitHub from the command line.\n\n") + + var commands []string + for _, c := range command.Commands() { + s := c.Name() + ":" + strings.Repeat(" ", c.NamePadding()) + c.Short + commands = append(commands, s) + } + + helpEntries := []helpEntry{ + {"USAGE", ` + gh [flags] + Commands are run inside of a GitHub repository.`}, + {"CORE COMMANDS", strings.Join(commands, "\n")}, + } + + for _, e := range helpEntries { + if e.Title != "" { + fmt.Fprintln(out, utils.Bold(e.Title)) + } + fmt.Fprintln(out, strings.TrimLeft(e.Body, "\n")+"\n") + } +} From b8ecf1fab56c28397806ca1f6b34519380caa1a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 15:16:24 +0200 Subject: [PATCH 04/30] Bump homebrew formula immediately after release --- .github/workflows/releases.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 2f9908167..e9d183592 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -26,6 +26,13 @@ jobs: args: release --release-notes=CHANGELOG.md env: GITHUB_TOKEN: ${{secrets.UPLOAD_GITHUB_TOKEN}} + - name: Bump homebrew-core formula + uses: mislav/bump-homebrew-formula-action@v1 + if: "!contains(github.ref, '-')" # skip prereleases + with: + formula-name: gh + env: + COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} - name: move cards if: "!contains(github.ref, '-')" env: @@ -38,13 +45,6 @@ jobs: api() { bin/hub api -H 'accept: application/vnd.github.inertia-preview+json' "$@"; } cards=$(api projects/columns/$PENDING_COLUMN/cards | jq ".[].id") for card in $cards; do api projects/columns/cards/$card/moves --field position=top --field column_id=$DONE_COLUMN; done - - name: Bump homebrew-core formula - uses: mislav/bump-homebrew-formula-action@v1 - if: "!contains(github.ref, '-')" # skip prereleases - with: - formula-name: gh - env: - COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} msi: needs: goreleaser runs-on: windows-latest From 60a67b852a43469df09035445131b54de5e5e933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 15:59:44 +0200 Subject: [PATCH 05/30] Automatically generate site docs on release --- .github/workflows/releases.yml | 16 +++++++++++++--- Makefile | 10 ++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index e9d183592..7fe2ca3af 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -33,13 +33,23 @@ jobs: formula-name: gh env: COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} - - name: move cards - if: "!contains(github.ref, '-')" + - name: Checkout documentation site + if: "!contains(github.ref, '-')" # skip prereleases + uses: actions/checkout@v2 + with: + repository: github/cli.github.com + path: site + fetch-depth: 0 + token: ${{secrets.SITE_GITHUB_TOKEN}} + - name: Publish documentation site + if: "!contains(github.ref, '-')" # skip prereleases + run: make site-publish + - name: Move project cards + if: "!contains(github.ref, '-')" # skip prereleases env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} PENDING_COLUMN: 8189733 DONE_COLUMN: 7110130 - shell: bash run: | curl -fsSL https://github.com/github/hub/raw/master/script/get | bash -s 2.14.1 api() { bin/hub api -H 'accept: application/vnd.github.inertia-preview+json' "$@"; } diff --git a/Makefile b/Makefile index 5a93dac9b..ffbd5bd7e 100644 --- a/Makefile +++ b/Makefile @@ -34,3 +34,13 @@ site-docs: site git -C site add 'manual/gh*.md' git -C site commit -m 'update help docs' .PHONY: site-docs + +site-publish: site-docs +ifndef GITHUB_REF + $(error GITHUB_REF is not set) +endif + sed -i.bak -E 's/(assign version = )".+"/\1"$(GITHUB_REF:refs/tags/v%=%)"/' site/index.html + rm -f site/index.html.bak + git -C site commit -m '$(GITHUB_REF:refs/tags/v%=%)' index.html + git -C site push +.PHONY: site-publish From 1b12ef863ef533afbf5e47779994a66c25ffe561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 15:59:58 +0200 Subject: [PATCH 06/30] Simplify release documentation --- docs/releasing.md | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/docs/releasing.md b/docs/releasing.md index 00646a18a..dfaebd76a 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -1,27 +1,17 @@ # Releasing -## Test Locally - -`go test ./...` - -## Push new docs - -build docs locally: `make site` - -build and push docs to production: `make site-docs` - -## Release locally for debugging - -A local release can be created for testing without creating anything official on the release page. - -1. `env GH_OAUTH_CLIENT_SECRET= GH_OAUTH_CLIENT_ID= goreleaser --skip-validate --skip-publish --rm-dist` -2. Check and test files in `dist/` - ## Release to production This can all be done from your local terminal. -1. `git tag 'vVERSION_NUMBER' # example git tag 'v0.0.1'` -2. `git push origin vVERSION_NUMBER` -3. Wait a few minutes for the build to run and CI to pass. Look at the [actions tab](https://github.com/cli/cli/actions) to check the progress. -4. Go to and look at the release +1. `git tag v1.2.3` +2. `git push origin v1.2.3` +3. Wait a few minutes for the build to run +4. Check + +## Release locally for debugging + +A local release can be created for testing without creating anything official on the release page. + +1. `goreleaser --skip-validate --skip-publish --rm-dist` +2. Check and test files in `dist/` From 83502fdbae866b5cd5c8aeba4ef6608c80d3f419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 16:13:16 +0200 Subject: [PATCH 07/30] Avoid failing if site docs are already up-to-date --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ffbd5bd7e..f2b4805c8 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ site-docs: site for f in site/manual/gh*.md; do sed -i.bak -e '/^### SEE ALSO/,$$d' "$$f"; done rm -f site/manual/*.bak git -C site add 'manual/gh*.md' - git -C site commit -m 'update help docs' + git -C site commit -m 'update help docs' || true .PHONY: site-docs site-publish: site-docs From 3c8b87c9c67224558c814480a3998f5ebc376e43 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Apr 2020 11:23:14 -0500 Subject: [PATCH 08/30] respect configured editor --- command/title_body_survey.go | 10 +++++++++- pkg/surveyext/editor.go | 27 ++++++++++++++++++--------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/command/title_body_survey.go b/command/title_body_survey.go index 8d4dd4d47..2f6270c36 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -82,6 +82,13 @@ func selectTemplate(templatePaths []string) (string, error) { } func titleBodySurvey(cmd *cobra.Command, providedTitle, providedBody string, defs defaults, templatePaths []string) (*titleBody, error) { + ctx := contextForCommand(cmd) + cfg, err := ctx.Config() + if err != nil { + return nil, fmt.Errorf("could not read config: %w", err) + } + editorName, _ := cfg.Get(defaultHostname, "editor") + var inProgress titleBody inProgress.Title = defs.Title templateContents := "" @@ -109,6 +116,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle, providedBody string, def bodyQuestion := &survey.Question{ Name: "body", Prompt: &surveyext.GhEditor{ + EditorName: editorName, Editor: &survey.Editor{ Message: "Body", FileName: "*.md", @@ -127,7 +135,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle, providedBody string, def qs = append(qs, bodyQuestion) } - err := SurveyAsk(qs, &inProgress) + err = SurveyAsk(qs, &inProgress) if err != nil { return nil, fmt.Errorf("could not prompt: %w", err) } diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index 8a936f294..4ed0c2844 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -18,25 +18,34 @@ import ( ) var ( - bom = []byte{0xef, 0xbb, 0xbf} - editor = "nano" // EXTENDED to switch from vim as a default editor + bom = []byte{0xef, 0xbb, 0xbf} + defaultEditor = "nano" // EXTENDED to switch from vim as a default editor ) func init() { if runtime.GOOS == "windows" { - editor = "notepad" + defaultEditor = "notepad" } else if g := os.Getenv("GIT_EDITOR"); g != "" { - editor = g + defaultEditor = g } else if v := os.Getenv("VISUAL"); v != "" { - editor = v + defaultEditor = v } else if e := os.Getenv("EDITOR"); e != "" { - editor = e + defaultEditor = e } } // EXTENDED to enable different prompting behavior type GhEditor struct { *survey.Editor + EditorName string +} + +func (e *GhEditor) editorName() string { + if e.EditorName == "" { + return defaultEditor + } + + return e.EditorName } // EXTENDED to change prompt text @@ -69,7 +78,7 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int // EXTENDED to support printing editor in prompt EditorTemplateData{ Editor: *e.Editor, - EditorName: filepath.Base(editor), + EditorName: filepath.Base(e.editorName()), Config: config, }, ) @@ -110,7 +119,7 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int EditorTemplateData{ // EXTENDED to support printing editor in prompt Editor: *e.Editor, - EditorName: filepath.Base(editor), + EditorName: filepath.Base(e.editorName()), ShowHelp: true, Config: config, }, @@ -155,7 +164,7 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int stdio := e.Stdio() - args, err := shellquote.Split(editor) + args, err := shellquote.Split(e.editorName()) if err != nil { return "", err } From a4429aae41dab01f68563b0e6e53a3c844caffb7 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 10:00:33 -0700 Subject: [PATCH 09/30] rearrange code --- command/root.go | 73 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 20 deletions(-) diff --git a/command/root.go b/command/root.go index 60fe77daa..9c9b5e692 100644 --- a/command/root.go +++ b/command/root.go @@ -47,7 +47,7 @@ func init() { // TODO: // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") - RootCmd.SetHelpFunc(aTempHelpFuncfunc) + RootCmd.SetHelpFunc(rootHelpFunc) RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { @@ -72,12 +72,8 @@ func (fe FlagError) Unwrap() error { // RootCmd is the entry point of command-line execution var RootCmd = &cobra.Command{ - Use: "gh", Short: "GitHub CLI", - Long: `Work seamlessly with GitHub from the command line. - -GitHub CLI is in early stages of development, and we'd love to hear your -feedback at `, + Long: `Work seamlessly with GitHub from the command line.`, SilenceErrors: true, SilenceUsage: true, @@ -209,28 +205,50 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac return baseRepo, nil } -type helpEntry struct { - Title string - Body string -} +func rootHelpFunc(command *cobra.Command, s []string) { + type helpEntry struct { + Title string + Body string + } -func aTempHelpFuncfunc(command *cobra.Command, s []string) { - out := colorableOut(command) - fmt.Fprint(out, "\nWork seamlessly with GitHub from the command line.\n\n") - - var commands []string + coreCommandNames := []string{"issue", "pr", "repo"} + var coreCommands []string + var additionalCommands []string for _, c := range command.Commands() { - s := c.Name() + ":" + strings.Repeat(" ", c.NamePadding()) + c.Short - commands = append(commands, s) + if c.Short == "" { + continue + } + s := " " + rpad(c.Name()+":", c.NamePadding()) + c.Short + if includes(coreCommandNames, c.Name()) { + coreCommands = append(coreCommands, s) + } else { + additionalCommands = append(additionalCommands, s) + } } helpEntries := []helpEntry{ + { + "", + command.Long}, {"USAGE", ` - gh [flags] - Commands are run inside of a GitHub repository.`}, - {"CORE COMMANDS", strings.Join(commands, "\n")}, + # most gh commands need to be run in a directory containing a GitHub repository. + gh [subcommand] [flags]`}, + {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, + {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, + {"FEEDBACK", ` + Fill out our feedback form + Open an issue using “gh issue create -R cli/cli`}, + {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, + {"EXAMPLES", ` + $ cd your-repository + $ gh pr list + $ gh pr checkout 321`}, + {"LEARN MORE", ` + Use "gh --help" for more information about a command. + Read the manual at `}, } + out := colorableOut(command) for _, e := range helpEntries { if e.Title != "" { fmt.Fprintln(out, utils.Bold(e.Title)) @@ -238,3 +256,18 @@ func aTempHelpFuncfunc(command *cobra.Command, s []string) { fmt.Fprintln(out, strings.TrimLeft(e.Body, "\n")+"\n") } } + +// rpad adds padding to the right of a string. +func rpad(s string, padding int) string { + template := fmt.Sprintf("%%-%ds ", padding) + return fmt.Sprintf(template, s) +} + +func includes(a []string, s string) bool { + for _, x := range a { + if x == s { + return true + } + } + return false +} From bec58ede9830841a143b9e3cf2a03b722eecb8fd Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 20 Apr 2020 14:50:30 -0500 Subject: [PATCH 10/30] respect ssh this adds recognition of the git_protocol setting when: - creating a repo - cloning a repo - forking a repo - forking/pushing during pr create - checking out a PR additionally, it: - consolidates remote adding to use AddRemote; this introduces a fetch where there previously hadn't been one - changes repo clone to accept an ssh url - changes repo fork to accept an ssh url i just added basic unit tests; adding new test cases for all of the above scenarios seemed like diminishing returns. --- command/pr_checkout.go | 4 ++-- command/pr_create.go | 4 ++-- command/repo.go | 32 +++++++++++++++++++------------- command/repo_test.go | 28 ++++++++++++++++++++-------- command/root.go | 19 +++++++++++++++++++ command/root_test.go | 19 +++++++++++++++++++ git/remote.go | 16 +++++++++++++--- 7 files changed, 94 insertions(+), 28 deletions(-) diff --git a/command/pr_checkout.go b/command/pr_checkout.go index d237aefbb..7da76f6ce 100644 --- a/command/pr_checkout.go +++ b/command/pr_checkout.go @@ -47,7 +47,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 := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(baseRepo)) + baseURLOrName := formatRemoteURL(cmd, ghrepo.FullName(baseRepo)) if baseRemote != nil { baseURLOrName = baseRemote.Name } @@ -97,7 +97,7 @@ func prCheckout(cmd *cobra.Command, args []string) error { remote := baseURLOrName mergeRef := ref if pr.MaintainerCanModify { - remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) + remote = formatRemoteURL(cmd, fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name)) mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName) } if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" { diff --git a/command/pr_create.go b/command/pr_create.go index 96b3e27df..2d029677a 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -250,8 +250,8 @@ 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 { - // TODO: support non-HTTPS git remote URLs - headRepoURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(headRepo)) + headRepoURL := formatRemoteURL(cmd, ghrepo.FullName(headRepo)) + // TODO: prevent clashes with another remote of a same name gitRemote, err := git.AddRemote("fork", headRepoURL) if err != nil { diff --git a/command/repo.go b/command/repo.go index c0f16b09c..3885f621f 100644 --- a/command/repo.go +++ b/command/repo.go @@ -127,7 +127,7 @@ func runClone(cloneURL string, args []string) (target string, err error) { func repoClone(cmd *cobra.Command, args []string) error { cloneURL := args[0] if !strings.Contains(cloneURL, ":") { - cloneURL = fmt.Sprintf("https://github.com/%s.git", cloneURL) + cloneURL = formatRemoteURL(cmd, cloneURL) } var repo ghrepo.Interface @@ -158,7 +158,7 @@ func repoClone(cmd *cobra.Command, args []string) error { } if parentRepo != nil { - err := addUpstreamRemote(parentRepo, cloneDir) + err := addUpstreamRemote(cmd, parentRepo, cloneDir) if err != nil { return err } @@ -167,9 +167,8 @@ func repoClone(cmd *cobra.Command, args []string) error { return nil } -func addUpstreamRemote(parentRepo ghrepo.Interface, cloneDir string) error { - // TODO: support SSH remote URLs - upstreamURL := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(parentRepo)) +func addUpstreamRemote(cmd *cobra.Command, parentRepo ghrepo.Interface, cloneDir string) error { + upstreamURL := formatRemoteURL(cmd, ghrepo.FullName(parentRepo)) cloneCmd := git.GitCommand("-C", cloneDir, "remote", "add", "-f", "upstream", upstreamURL) cloneCmd.Stdout = os.Stdout @@ -267,14 +266,10 @@ func repoCreate(cmd *cobra.Command, args []string) error { fmt.Fprintln(out, repo.URL) } - remoteURL := repo.URL + ".git" + remoteURL := formatRemoteURL(cmd, ghrepo.FullName(repo)) if projectDirErr == nil { - // TODO: use git.AddRemote - remoteAdd := git.GitCommand("remote", "add", "origin", remoteURL) - remoteAdd.Stdout = os.Stdout - remoteAdd.Stderr = os.Stderr - err = run.PrepareCmd(remoteAdd).Run() + _, err = git.AddRemote("origin", remoteURL) if err != nil { return err } @@ -361,6 +356,15 @@ func repoFork(cmd *cobra.Command, args []string) error { return fmt.Errorf("did not understand argument: %w", err) } + } else if strings.HasPrefix(repoArg, "git@") { + parsedURL, err := git.ParseURL(repoArg) + if err != nil { + return fmt.Errorf("did not understand argument: %w", err) + } + toFork, err = ghrepo.FromURL(parsedURL) + if err != nil { + return fmt.Errorf("did not understand argument: %w", err) + } } else { toFork = ghrepo.FromFullName(repoArg) if toFork.RepoName() == "" || toFork.RepoOwner() == "" { @@ -437,7 +441,9 @@ 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)) } - _, err = git.AddRemote(remoteName, forkedRepo.CloneURL) + forkedRepoCloneURL := formatRemoteURL(cmd, ghrepo.FullName(forkedRepo)) + + _, err = git.AddRemote(remoteName, forkedRepoCloneURL) if err != nil { return fmt.Errorf("failed to add remote: %w", err) } @@ -458,7 +464,7 @@ func repoFork(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to clone fork: %w", err) } - err = addUpstreamRemote(toFork, cloneDir) + err = addUpstreamRemote(cmd, toFork, cloneDir) if err != nil { return err } diff --git a/command/repo_test.go b/command/repo_test.go index bd10e4218..f50eb0cac 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -153,7 +153,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) { expectedCmds := []string{ "git remote rename origin upstream", - "git remote add -f origin https://github.com/someone/repo.git", + "git remote add -f origin https://github.com/someone/REPO.git", } for x, cmd := range seenCmds { @@ -287,7 +287,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { expectedCmds := []string{ "git remote rename origin upstream", - "git remote add -f origin https://github.com/someone/repo.git", + "git remote add -f origin https://github.com/someone/REPO.git", } for x, cmd := range seenCmds { @@ -499,7 +499,11 @@ func TestRepoCreate(t *testing.T) { { "data": { "createRepository": { "repository": { "id": "REPOID", - "url": "https://github.com/OWNER/REPO" + "url": "https://github.com/OWNER/REPO", + "name": "REPO", + "owner": { + "login": "OWNER" + } } } } } `)) @@ -522,7 +526,7 @@ func TestRepoCreate(t *testing.T) { if seenCmd == nil { t.Fatal("expected a command to run") } - eq(t, strings.Join(seenCmd.Args, " "), "git remote add origin https://github.com/OWNER/REPO.git") + eq(t, strings.Join(seenCmd.Args, " "), "git remote add -f origin https://github.com/OWNER/REPO.git") var reqBody struct { Query string @@ -564,7 +568,11 @@ func TestRepoCreate_org(t *testing.T) { { "data": { "createRepository": { "repository": { "id": "REPOID", - "url": "https://github.com/ORG/REPO" + "url": "https://github.com/ORG/REPO", + "name": "REPO", + "owner": { + "login": "ORG" + } } } } } `)) @@ -587,7 +595,7 @@ func TestRepoCreate_org(t *testing.T) { if seenCmd == nil { t.Fatal("expected a command to run") } - eq(t, strings.Join(seenCmd.Args, " "), "git remote add origin https://github.com/ORG/REPO.git") + eq(t, strings.Join(seenCmd.Args, " "), "git remote add -f origin https://github.com/ORG/REPO.git") var reqBody struct { Query string @@ -629,7 +637,11 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { { "data": { "createRepository": { "repository": { "id": "REPOID", - "url": "https://github.com/ORG/REPO" + "url": "https://github.com/ORG/REPO", + "name": "REPO", + "owner": { + "login": "ORG" + } } } } } `)) @@ -652,7 +664,7 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { if seenCmd == nil { t.Fatal("expected a command to run") } - eq(t, strings.Join(seenCmd.Args, " "), "git remote add origin https://github.com/ORG/REPO.git") + eq(t, strings.Join(seenCmd.Args, " "), "git remote add -f origin https://github.com/ORG/REPO.git") var reqBody struct { Query string diff --git a/command/root.go b/command/root.go index 31a0ebca6..40be6a7e0 100644 --- a/command/root.go +++ b/command/root.go @@ -222,3 +222,22 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac return baseRepo, nil } + +func formatRemoteURL(cmd *cobra.Command, fullRepoName string) string { + ctx := contextForCommand(cmd) + + protocol := "https" + 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 + } + + if protocol == "ssh" { + return fmt.Sprintf("git@%s:%s.git", defaultHostname, fullRepoName) + } + + return fmt.Sprintf("https://%s/%s.git", defaultHostname, fullRepoName) +} diff --git a/command/root_test.go b/command/root_test.go index c0b1fd7d3..dbed9628d 100644 --- a/command/root_test.go +++ b/command/root_test.go @@ -40,3 +40,22 @@ func TestChangelogURL(t *testing.T) { t.Errorf("expected %s to create url %s but got %s", tag, url, result) } } + +func TestRemoteURLFormatting_no_config(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + result := formatRemoteURL(repoForkCmd, "OWNER/REPO") + eq(t, result, "https://github.com/OWNER/REPO.git") +} + +func TestRemoteURLFormatting_ssh_config(t *testing.T) { + cfg := `--- +hosts: + github.com: + user: OWNER + oauth_token: MUSTBEHIGHCUZIMATOKEN +git_protocol: ssh +` + initBlankContext(cfg, "OWNER/REPO", "master") + result := formatRemoteURL(repoForkCmd, "OWNER/REPO") + eq(t, result, "git@github.com:OWNER/REPO.git") +} diff --git a/git/remote.go b/git/remote.go index 6c8608da9..ab6cdb9fb 100644 --- a/git/remote.go +++ b/git/remote.go @@ -79,9 +79,19 @@ func AddRemote(name, u string) (*Remote, error) { return nil, err } - urlParsed, err := url.Parse(u) - if err != nil { - return nil, err + var urlParsed *url.URL + if strings.HasPrefix(u, "https") { + urlParsed, err = url.Parse(u) + if err != nil { + return nil, err + } + + } else { + urlParsed, err = ParseURL(u) + if err != nil { + return nil, err + } + } return &Remote{ From 9641eee38ac253d2468fd7e4f864eb2d1cf4cbd7 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Apr 2020 14:31:09 -0500 Subject: [PATCH 11/30] use more clear name --- command/title_body_survey.go | 4 ++-- pkg/surveyext/editor.go | 36 ++++++++++++++++++------------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/command/title_body_survey.go b/command/title_body_survey.go index 2f6270c36..c2b68ec8d 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -87,7 +87,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle, providedBody string, def if err != nil { return nil, fmt.Errorf("could not read config: %w", err) } - editorName, _ := cfg.Get(defaultHostname, "editor") + editorCommand, _ := cfg.Get(defaultHostname, "editor") var inProgress titleBody inProgress.Title = defs.Title @@ -116,7 +116,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle, providedBody string, def bodyQuestion := &survey.Question{ Name: "body", Prompt: &surveyext.GhEditor{ - EditorName: editorName, + EditorCommand: editorCommand, Editor: &survey.Editor{ Message: "Body", FileName: "*.md", diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index 4ed0c2844..3121ccf4d 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -37,15 +37,15 @@ func init() { // EXTENDED to enable different prompting behavior type GhEditor struct { *survey.Editor - EditorName string + EditorCommand string } -func (e *GhEditor) editorName() string { - if e.EditorName == "" { +func (e *GhEditor) editorCommand() string { + if e.EditorCommand == "" { return defaultEditor } - return e.EditorName + return e.EditorCommand } // EXTENDED to change prompt text @@ -58,17 +58,17 @@ var EditorQuestionTemplate = ` {{- else }} {{- if and .Help (not .ShowHelp)}}{{color "cyan"}}[{{ .Config.HelpInput }} for help]{{color "reset"}} {{end}} {{- if and .Default (not .HideDefault)}}{{color "white"}}({{.Default}}) {{color "reset"}}{{end}} - {{- color "cyan"}}[(e) to launch {{ .EditorName }}, enter to skip] {{color "reset"}} + {{- color "cyan"}}[(e) to launch {{ .EditorCommand }}, enter to skip] {{color "reset"}} {{- end}}` // EXTENDED to pass editor name (to use in prompt) type EditorTemplateData struct { survey.Editor - EditorName string - Answer string - ShowAnswer bool - ShowHelp bool - Config *survey.PromptConfig + EditorCommand string + Answer string + ShowAnswer bool + ShowHelp bool + Config *survey.PromptConfig } // EXTENDED to augment prompt text and keypress handling @@ -77,9 +77,9 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int EditorQuestionTemplate, // EXTENDED to support printing editor in prompt EditorTemplateData{ - Editor: *e.Editor, - EditorName: filepath.Base(e.editorName()), - Config: config, + Editor: *e.Editor, + EditorCommand: filepath.Base(e.editorCommand()), + Config: config, }, ) if err != nil { @@ -118,10 +118,10 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int EditorQuestionTemplate, EditorTemplateData{ // EXTENDED to support printing editor in prompt - Editor: *e.Editor, - EditorName: filepath.Base(e.editorName()), - ShowHelp: true, - Config: config, + Editor: *e.Editor, + EditorCommand: filepath.Base(e.editorCommand()), + ShowHelp: true, + Config: config, }, ) if err != nil { @@ -164,7 +164,7 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int stdio := e.Stdio() - args, err := shellquote.Split(e.editorName()) + args, err := shellquote.Split(e.editorCommand()) if err != nil { return "", err } From f26690adc9c7dcfcb1236f8d93136f55544ff130 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Apr 2020 14:33:56 -0500 Subject: [PATCH 12/30] support GH_EDITOR --- command/title_body_survey.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/command/title_body_survey.go b/command/title_body_survey.go index c2b68ec8d..397c27bef 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "os" "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/pkg/githubtemplate" @@ -82,12 +83,15 @@ func selectTemplate(templatePaths []string) (string, error) { } func titleBodySurvey(cmd *cobra.Command, providedTitle, providedBody string, defs defaults, templatePaths []string) (*titleBody, error) { - ctx := contextForCommand(cmd) - cfg, err := ctx.Config() - if err != nil { - return nil, fmt.Errorf("could not read config: %w", err) + editorCommand := os.Getenv("GH_EDITOR") + if editorCommand == "" { + ctx := contextForCommand(cmd) + cfg, err := ctx.Config() + if err != nil { + return nil, fmt.Errorf("could not read config: %w", err) + } + editorCommand, _ = cfg.Get(defaultHostname, "editor") } - editorCommand, _ := cfg.Get(defaultHostname, "editor") var inProgress titleBody inProgress.Title = defs.Title @@ -135,7 +139,7 @@ func titleBodySurvey(cmd *cobra.Command, providedTitle, providedBody string, def qs = append(qs, bodyQuestion) } - err = SurveyAsk(qs, &inProgress) + err := SurveyAsk(qs, &inProgress) if err != nil { return nil, fmt.Errorf("could not prompt: %w", err) } From b5299ef5d73596b66b06f701379ea188f01ee3fe Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:46:51 -0700 Subject: [PATCH 13/30] pointy brackets ftw Co-Authored-By: Amanda Pinsker --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 9c9b5e692..150a9292c 100644 --- a/command/root.go +++ b/command/root.go @@ -232,7 +232,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { command.Long}, {"USAGE", ` # most gh commands need to be run in a directory containing a GitHub repository. - gh [subcommand] [flags]`}, + gh [flags]`}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, {"FEEDBACK", ` From f1bee0c9b9c637e4bca16474ef7a7884a6aac420 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Apr 2020 15:49:16 -0500 Subject: [PATCH 14/30] use more clear name --- command/repo.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/command/repo.go b/command/repo.go index 3885f621f..5a278d612 100644 --- a/command/repo.go +++ b/command/repo.go @@ -333,7 +333,7 @@ func repoFork(cmd *cobra.Command, args []string) error { return fmt.Errorf("unable to create client: %w", err) } - var toFork ghrepo.Interface + var repoToFork ghrepo.Interface inParent := false // whether or not we're forking the repo we're currently "in" if len(args) == 0 { baseRepo, err := determineBaseRepo(cmd, ctx) @@ -341,7 +341,7 @@ func repoFork(cmd *cobra.Command, args []string) error { return fmt.Errorf("unable to determine base repository: %w", err) } inParent = true - toFork = baseRepo + repoToFork = baseRepo } else { repoArg := args[0] @@ -351,7 +351,7 @@ func repoFork(cmd *cobra.Command, args []string) error { return fmt.Errorf("did not understand argument: %w", err) } - toFork, err = ghrepo.FromURL(parsedURL) + repoToFork, err = ghrepo.FromURL(parsedURL) if err != nil { return fmt.Errorf("did not understand argument: %w", err) } @@ -361,13 +361,13 @@ func repoFork(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("did not understand argument: %w", err) } - toFork, err = ghrepo.FromURL(parsedURL) + repoToFork, err = ghrepo.FromURL(parsedURL) if err != nil { return fmt.Errorf("did not understand argument: %w", err) } } else { - toFork = ghrepo.FromFullName(repoArg) - if toFork.RepoName() == "" || toFork.RepoOwner() == "" { + repoToFork = ghrepo.FromFullName(repoArg) + if repoToFork.RepoName() == "" || repoToFork.RepoOwner() == "" { return fmt.Errorf("could not parse owner or repo name from %s", repoArg) } } @@ -376,12 +376,12 @@ func repoFork(cmd *cobra.Command, args []string) error { greenCheck := utils.Green("✓") out := colorableOut(cmd) s := utils.Spinner(out) - loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(toFork))) + utils.Gray("...") + loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(repoToFork))) + utils.Gray("...") s.Suffix = " " + loading s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading)) s.Start() - forkedRepo, err := api.ForkRepo(apiClient, toFork) + forkedRepo, err := api.ForkRepo(apiClient, repoToFork) if err != nil { s.Stop() return fmt.Errorf("failed to fork: %w", err) @@ -464,7 +464,7 @@ func repoFork(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to clone fork: %w", err) } - err = addUpstreamRemote(cmd, toFork, cloneDir) + err = addUpstreamRemote(cmd, repoToFork, cloneDir) if err != nil { return err } From 6d20283a7a66fa6622fd30cf1ca5e2b5439826fd Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:49:42 -0700 Subject: [PATCH 15/30] Move feedback to the bottom --- command/root.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/command/root.go b/command/root.go index 9c9b5e692..fbbb1b091 100644 --- a/command/root.go +++ b/command/root.go @@ -235,17 +235,17 @@ func rootHelpFunc(command *cobra.Command, s []string) { gh [subcommand] [flags]`}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, - {"FEEDBACK", ` - Fill out our feedback form - Open an issue using “gh issue create -R cli/cli`}, - {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, {"EXAMPLES", ` $ cd your-repository $ gh pr list $ gh pr checkout 321`}, {"LEARN MORE", ` Use "gh --help" for more information about a command. - Read the manual at `}, + Read the manual at `}, + {"FEEDBACK", ` + Fill out our feedback form + Open an issue using “gh issue create -R cli/cli`}, + {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, } out := colorableOut(command) From 833f8b4f2c8e6b6f540944aa168fad1c41095267 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:50:06 -0700 Subject: [PATCH 16/30] Update command/root.go Co-Authored-By: Amanda Pinsker --- command/root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/root.go b/command/root.go index 150a9292c..1c02550f1 100644 --- a/command/root.go +++ b/command/root.go @@ -240,8 +240,8 @@ func rootHelpFunc(command *cobra.Command, s []string) { Open an issue using “gh issue create -R cli/cli`}, {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, {"EXAMPLES", ` - $ cd your-repository - $ gh pr list + $ gh issue create + $ gh repo clone $ gh pr checkout 321`}, {"LEARN MORE", ` Use "gh --help" for more information about a command. From 0d792573126b1ae9ba197bc2d5826d87e88af633 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:50:52 -0700 Subject: [PATCH 17/30] use backets Co-Authored-By: Amanda Pinsker --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 1c02550f1..77d81f115 100644 --- a/command/root.go +++ b/command/root.go @@ -244,7 +244,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { $ gh repo clone $ gh pr checkout 321`}, {"LEARN MORE", ` - Use "gh --help" for more information about a command. + Use "gh [command] [subcommand] --help" for more information about a command. Read the manual at `}, } From c064b07e6db27b21a2357be70c19e7d1a9a46455 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:51:35 -0700 Subject: [PATCH 18/30] Add quote --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index fbbb1b091..988e12ff8 100644 --- a/command/root.go +++ b/command/root.go @@ -244,7 +244,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { Read the manual at `}, {"FEEDBACK", ` Fill out our feedback form - Open an issue using “gh issue create -R cli/cli`}, + Open an issue using “gh issue create -R cli/cli”`}, {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, } From c9d5935ee33b732e2914e72bc60598eb239a63d3 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 14:00:08 -0700 Subject: [PATCH 19/30] move to use --- command/root.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/root.go b/command/root.go index 6f2927a61..4096306b1 100644 --- a/command/root.go +++ b/command/root.go @@ -76,6 +76,7 @@ func (fe FlagError) Unwrap() error { // RootCmd is the entry point of command-line execution var RootCmd = &cobra.Command{ + Use: "gh [flags]", Short: "GitHub CLI", Long: `Work seamlessly with GitHub from the command line.`, @@ -246,9 +247,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { { "", command.Long}, - {"USAGE", ` - # most gh commands need to be run in a directory containing a GitHub repository. - gh [subcommand] [flags]`}, + {"USAGE", command.UsageString()}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, {"EXAMPLES", ` From e68be52ed0a11e5d488750713fc685f4a644e41b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 14:03:44 -0700 Subject: [PATCH 20/30] Use the correct field --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 611022d2d..84c421d46 100644 --- a/command/root.go +++ b/command/root.go @@ -247,7 +247,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { { "", command.Long}, - {"USAGE", command.UsageString()}, + {"USAGE", command.Use}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, From 03e60758d0974c304ba95ef8a7a56bef7dadf76a Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Apr 2020 16:08:47 -0500 Subject: [PATCH 21/30] missed a thing --- command/repo.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/command/repo.go b/command/repo.go index 5a278d612..b96a48164 100644 --- a/command/repo.go +++ b/command/repo.go @@ -459,7 +459,8 @@ func repoFork(cmd *cobra.Command, args []string) error { } } if cloneDesired { - cloneDir, err := runClone(forkedRepo.CloneURL, []string{}) + forkedRepoCloneURL := formatRemoteURL(cmd, ghrepo.FullName(forkedRepo)) + cloneDir, err := runClone(forkedRepoCloneURL, []string{}) if err != nil { return fmt.Errorf("failed to clone fork: %w", err) } From 97e3a5724478abc0461028fba9ad8212e93ce159 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Apr 2020 16:16:14 -0500 Subject: [PATCH 22/30] fix tests --- command/repo_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/command/repo_test.go b/command/repo_test.go index f50eb0cac..118e365d5 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -185,8 +185,8 @@ func TestRepoFork_outside_yes(t *testing.T) { eq(t, output.Stderr(), "") - eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/someone/repo.git") - eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C repo remote add -f upstream https://github.com/OWNER/REPO.git") + eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/someone/REPO.git") + eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git") test.ExpectLines(t, output.String(), "Created fork someone/REPO", @@ -218,8 +218,8 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { eq(t, output.Stderr(), "") - eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/someone/repo.git") - eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C repo remote add -f upstream https://github.com/OWNER/REPO.git") + eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/someone/REPO.git") + eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C REPO remote add -f upstream https://github.com/OWNER/REPO.git") test.ExpectLines(t, output.String(), "Created fork someone/REPO", From cd93e5643c46b2c5c1841840c861fcf2f12a8b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Apr 2020 13:04:40 +0200 Subject: [PATCH 23/30] Help goreleaser find the correct git tag during release --- .github/workflows/releases.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 2f9908167..67486f3aa 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -17,6 +17,7 @@ jobs: go-version: 1.13 - name: Generate changelog run: | + echo ::set-env name=GORELEASER_CURRENT_TAG::${GITHUB_REF#refs/tags/} git fetch --unshallow script/changelog | tee CHANGELOG.md - name: Run GoReleaser From 84ac3d412e42e28c491c655a10930e3b74c528b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Apr 2020 15:41:37 +0200 Subject: [PATCH 24/30] Fix gh updater mechanism for unauthenticated users Per code documentation: "BasicClient returns an API client that borrows from but does not depend on user configuration". This means that any errors while reading `oauth_token` should be tolerated. --- command/root.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/command/root.go b/command/root.go index 40be6a7e0..a6ea851ce 100644 --- a/command/root.go +++ b/command/root.go @@ -111,20 +111,11 @@ func BasicClient() (*api.Client, error) { } opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version))) - c, err := config.ParseDefaultConfig() - if err != nil { - return nil, err + if c, err := config.ParseDefaultConfig(); err == nil { + if token, _ := c.Get(defaultHostname, "oauth_token"); token != "" { + opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) + } } - - token, err := c.Get(defaultHostname, "oauth_token") - if err != nil { - return nil, err - } - if token == "" { - return nil, fmt.Errorf("no oauth_token set in config") - } - - opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) return api.NewClient(opts...), nil } From 3aaa231cc5388ecef05a218314f308cb49fcf7f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Apr 2020 18:15:20 +0200 Subject: [PATCH 25/30] Guide user through re-authorization flow if `read:org` scope is missing How this works for people with existing OAuth tokens: $ gh issue list -L1 Notice: additional authorization required Press Enter to open github.com in your browser... [auth flow in the browser...] Authentication complete. Press Enter to continue... Showing 1 of 132 issues in cli/cli ... Users of Personal Access Tokens get a different notice: Warning: gh now requires the `read:org` OAuth scope. Visit https://github.com/settings/tokens and edit your token to enable `read:org` or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN` --- api/client.go | 27 ++++++++++--------- command/root.go | 37 ++++++++++++++++++++++++-- internal/config/config_setup.go | 47 ++++++++++++++++++++++----------- 3 files changed, 81 insertions(+), 30 deletions(-) diff --git a/api/client.go b/api/client.go index e1fef7219..d7b981bee 100644 --- a/api/client.go +++ b/api/client.go @@ -7,7 +7,6 @@ import ( "io" "io/ioutil" "net/http" - "os" "regexp" "strings" @@ -38,6 +37,16 @@ func AddHeader(name, value string) ClientOption { } } +// AddHeaderFunc is an AddHeader that gets the string value from a function +func AddHeaderFunc(name string, value func() string) ClientOption { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + req.Header.Add(name, value()) + return tr.RoundTrip(req) + }} + } +} + // VerboseLog enables request/response logging within a RoundTripper func VerboseLog(out io.Writer, logTraffic bool, colorize bool) ClientOption { logger := &httpretty.Logger{ @@ -67,15 +76,15 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption { var issuedScopesWarning bool // CheckScopes checks whether an OAuth scope is present in a response -func CheckScopes(wantedScope string) ClientOption { +func CheckScopes(wantedScope string, cb func(string) error) ClientOption { 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 || issuedScopesWarning { + if err != nil || res.StatusCode > 299 || issuedScopesWarning { return res, err } - isApp := res.Header.Get("X-Oauth-Client-Id") != "" + appID := res.Header.Get("X-Oauth-Client-Id") hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",") hasWanted := false @@ -87,14 +96,8 @@ func CheckScopes(wantedScope string) ClientOption { } if !hasWanted { - fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") - // TODO: offer to take the person through the authentication flow again? - // TODO: retry the original request if it was a read? - if isApp { - fmt.Fprintln(os.Stderr, "To re-authenticate, please `rm ~/.config/gh/config.yml` and try again.") - } else { - // the person has pasted a Personal Access Token - fmt.Fprintln(os.Stderr, "Re-generate your token in `rm ~/.config/gh/config.yml` and try again.") + if err := cb(appID); err != nil { + return res, err } issuedScopesWarning = true } diff --git a/command/root.go b/command/root.go index 1624a57e0..8c0cbd046 100644 --- a/command/root.go +++ b/command/root.go @@ -142,13 +142,46 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { if err != nil { return nil, err } + var opts []api.ClientOption if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, apiVerboseLog()) } + + getAuthValue := func() string { + return fmt.Sprintf("token %s", token) + } + checkScopesFunc := func(appID string) error { + if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { + newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required") + if err != nil { + return err + } + cfg, err := ctx.Config() + if err != nil { + return err + } + _ = cfg.Set(defaultHostname, "oauth_token", newToken) + _ = cfg.Set(defaultHostname, "user", loginHandle) + // update config file on disk + err = cfg.Write() + if err != nil { + return err + } + // update configuration in memory + token = newToken + config.AuthFlowComplete() + } else { + fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") + fmt.Fprintln(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable `read:org`") + fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + } + return nil + } + opts = append(opts, - api.CheckScopes("read:org"), - api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), + api.CheckScopes("read:org", checkScopesFunc), + api.AddHeaderFunc("Authorization", getAuthValue), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), // antiope-preview: Checks api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), diff --git a/internal/config/config_setup.go b/internal/config/config_setup.go index 9564ecd4c..4d2a929cb 100644 --- a/internal/config/config_setup.go +++ b/internal/config/config_setup.go @@ -24,9 +24,14 @@ var ( oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) -// TODO: have a conversation about whether this belongs in the "context" package -// FIXME: make testable -func setupConfigFile(filename string) (Config, error) { +// IsGitHubApp reports whether an OAuth app is "GitHub CLI" or "GitHub CLI (dev)" +func IsGitHubApp(id string) bool { + // this intentionally doesn't use `oauthClientID` because that is a variable + // that can potentially be changed at build time via GH_OAUTH_CLIENT_ID + return id == "178c6fc778ccc68e1d6a" || id == "4d747ba5675d5d66553f" +} + +func AuthFlow(notice string) (string, string, error) { var verboseStream io.Writer if strings.Contains(os.Getenv("DEBUG"), "oauth") { verboseStream = os.Stderr @@ -43,15 +48,30 @@ func setupConfigFile(filename string) (Config, error) { VerboseStream: verboseStream, } - fmt.Fprintln(os.Stderr, "Notice: authentication required") + fmt.Fprintln(os.Stderr, notice) fmt.Fprintf(os.Stderr, "Press Enter to open %s in your browser... ", flow.Hostname) _ = waitForEnter(os.Stdin) token, err := flow.ObtainAccessToken() if err != nil { - return nil, err + return "", "", err } userLogin, err := getViewer(token) + if err != nil { + return "", "", err + } + + return token, userLogin, nil +} + +func AuthFlowComplete() { + fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") + _ = waitForEnter(os.Stdin) +} + +// FIXME: make testable +func setupConfigFile(filename string) (Config, error) { + token, userLogin, err := AuthFlow("Notice: authentication required") if err != nil { return nil, err } @@ -62,9 +82,9 @@ func setupConfigFile(filename string) (Config, error) { } yamlHosts := map[string]map[string]string{} - yamlHosts[flow.Hostname] = map[string]string{} - yamlHosts[flow.Hostname]["user"] = userLogin - yamlHosts[flow.Hostname]["oauth_token"] = token + yamlHosts[oauthHost] = map[string]string{} + yamlHosts[oauthHost]["user"] = userLogin + yamlHosts[oauthHost]["oauth_token"] = token defaultConfig := yamlConfig{ Hosts: yamlHosts, @@ -85,14 +105,9 @@ func setupConfigFile(filename string) (Config, error) { if err != nil { return nil, err } - n, err := cfgFile.Write(yamlData) - if err == nil && n < len(yamlData) { - err = io.ErrShortWrite - } - - if err == nil { - fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") - _ = waitForEnter(os.Stdin) + _, err = cfgFile.Write(yamlData) + if err != nil { + return nil, err } // TODO cleaner error handling? this "should" always work given that we /just/ wrote the file... From 700f86c5bce165a93aa3ecbbc850363ea8e530d8 Mon Sep 17 00:00:00 2001 From: Leonardo Melo Date: Fri, 24 Apr 2020 08:49:03 -0300 Subject: [PATCH 26/30] Update README Add missing `gh config` command usage to README. Also, remove a trailing space in the `Availability` section, hehe. :smiley: --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5c96a685f..c4c0e9a5d 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ the terminal next to where you are already working with `git` and your code. ## Availability -While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning support for GitHub Enterprise Server after GitHub CLI is out of beta (likely toward the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. +While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning support for GitHub Enterprise Server after GitHub CLI is out of beta (likely toward the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. ## We need your feedback @@ -22,6 +22,7 @@ And if you spot bugs or have features that you'd really like to see in `gh`, ple - `gh pr [status, list, view, checkout, create]` - `gh issue [status, list, view, create]` - `gh repo [view, create, clone, fork]` +- `gh config [get, set]` - `gh help` ## Documentation From fab18ed4a95266530c4fa539460699b367c73522 Mon Sep 17 00:00:00 2001 From: rista404 Date: Sat, 25 Apr 2020 22:57:03 +0200 Subject: [PATCH 27/30] Show awaiting triage when project col is empty --- command/issue.go | 6 +++++- command/pr.go | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/command/issue.go b/command/issue.go index 2f4e145af..f2cb4ba47 100644 --- a/command/issue.go +++ b/command/issue.go @@ -504,7 +504,11 @@ func issueProjectList(issue api.Issue) string { projectNames := make([]string, 0, len(issue.ProjectCards.Nodes)) for _, project := range issue.ProjectCards.Nodes { - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, project.Column.Name)) + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) } list := strings.Join(projectNames, ", ") diff --git a/command/pr.go b/command/pr.go index a5b1f0079..009548061 100644 --- a/command/pr.go +++ b/command/pr.go @@ -417,7 +417,11 @@ func prProjectList(pr api.PullRequest) string { projectNames := make([]string, 0, len(pr.ProjectCards.Nodes)) for _, project := range pr.ProjectCards.Nodes { - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, project.Column.Name)) + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) } list := strings.Join(projectNames, ", ") From a2a6036c6543973a4940a905263e87c63d742dc1 Mon Sep 17 00:00:00 2001 From: rista404 Date: Sat, 25 Apr 2020 23:12:51 +0200 Subject: [PATCH 28/30] Add tests --- command/issue_test.go | 2 +- command/pr_test.go | 2 +- test/fixtures/issueView_previewWithMetadata.json | 8 ++++++++ test/fixtures/prViewPreviewWithMetadataByNumber.json | 8 ++++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index b7f91031d..e9229ac24 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -312,7 +312,7 @@ func TestIssueView_Preview(t *testing.T) { `Open • marseilles opened about 292 years ago • 9 comments`, `Assignees: marseilles, monaco\n`, `Labels: one, two, three, four, five\n`, - `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone: uluru\n`, `bold story`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, diff --git a/command/pr_test.go b/command/pr_test.go index 074420deb..ba0989645 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -429,7 +429,7 @@ func TestPRView_Preview(t *testing.T) { `Open • nobody wants to merge 12 commits into master from blueberries`, `Assignees: marseilles, monaco\n`, `Labels: one, two, three, four, five\n`, - `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone: uluru\n`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, diff --git a/test/fixtures/issueView_previewWithMetadata.json b/test/fixtures/issueView_previewWithMetadata.json index a420eec66..246bbd77b 100644 --- a/test/fixtures/issueView_previewWithMetadata.json +++ b/test/fixtures/issueView_previewWithMetadata.json @@ -67,6 +67,14 @@ "column": { "name": "column C" } + }, + { + "project": { + "name": "Project 4" + }, + "column": { + "name": "" + } } ], "totalcount": 3 diff --git a/test/fixtures/prViewPreviewWithMetadataByNumber.json b/test/fixtures/prViewPreviewWithMetadataByNumber.json index de0da816a..e49bcdc85 100644 --- a/test/fixtures/prViewPreviewWithMetadataByNumber.json +++ b/test/fixtures/prViewPreviewWithMetadataByNumber.json @@ -66,6 +66,14 @@ "column": { "name": "column C" } + }, + { + "project": { + "name": "Project 4" + }, + "column": { + "name": "" + } } ], "totalcount": 3 From 3d2ee3c9b27ce1c940ef3659acc69393feced9dd Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 27 Apr 2020 10:07:47 -0700 Subject: [PATCH 29/30] fix whitespace --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 84c421d46..c567394a7 100644 --- a/command/root.go +++ b/command/root.go @@ -257,7 +257,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { $ gh pr checkout 321`}, {"LEARN MORE", ` Use "gh --help" for more information about a command. - Read the manual at `}, + Read the manual at `}, {"FEEDBACK", ` Fill out our feedback form Open an issue using “gh issue create -R cli/cli”`}, From fb7b5446d1e40ea43da09bd7ec85a0addaee3d4a Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 28 Apr 2020 20:19:52 -0500 Subject: [PATCH 30/30] Update command/root.go Co-Authored-By: Corey Johnson --- command/root.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command/root.go b/command/root.go index 8c0cbd046..fcb3dca2b 100644 --- a/command/root.go +++ b/command/root.go @@ -151,6 +151,7 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { getAuthValue := func() string { return fmt.Sprintf("token %s", token) } + checkScopesFunc := func(appID string) error { if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required")