From 8e86129e2eb37830fd88bb2e484a4c4d225e986c Mon Sep 17 00:00:00 2001 From: Ruslan Gilyazetdinov Date: Mon, 1 Feb 2021 18:49:26 +0300 Subject: [PATCH 01/22] remove gist description from single file raw view (#2886) --- pkg/cmd/gist/view/view.go | 10 +++++----- pkg/cmd/gist/view/view_test.go | 23 ++++++++++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 9d8b41110..31b48325f 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -89,11 +89,6 @@ func viewRun(opts *ViewOptions) error { return err } - cs := opts.IO.ColorScheme() - if gist.Description != "" { - fmt.Fprintf(opts.IO.Out, "%s\n", cs.Bold(gist.Description)) - } - if opts.Filename != "" { gistFile, ok := gist.Files[opts.Filename] if !ok { @@ -107,6 +102,11 @@ func viewRun(opts *ViewOptions) error { showFilenames := len(gist.Files) > 1 + cs := opts.IO.ColorScheme() + if gist.Description != "" && showFilenames { + fmt.Fprintf(opts.IO.Out, "%s\n", cs.Bold(gist.Description)) + } + outs := []string{} // to ensure consistent ordering for filename, gistFile := range gist.Files { diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index 0ddf55f49..832a9ae01 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -131,6 +131,27 @@ func Test_viewRun(t *testing.T) { }, wantOut: "bwhiizzzbwhuiiizzzz\n\n", }, + { + name: "filename selected, raw", + opts: &ViewOptions{ + Selector: "1234", + Filename: "cicada.txt", + Raw: true, + }, + gist: &shared.Gist{ + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "foo.md": { + Content: "# foo", + Type: "application/markdown", + }, + }, + }, + wantOut: "bwhiizzzbwhuiiizzzz\n\n", + }, { name: "multiple files, no description", opts: &ViewOptions{ @@ -171,7 +192,7 @@ func Test_viewRun(t *testing.T) { wantOut: "some files\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n \n • foo \n\n\n\n", }, { - name: "raw", + name: "multiple files, raw", opts: &ViewOptions{ Selector: "1234", Raw: true, From 232dc7b7fa6559de74bd6b4b5902fd7985a772fe Mon Sep 17 00:00:00 2001 From: Ruslan Gilyazetdinov Date: Tue, 2 Feb 2021 10:54:07 +0300 Subject: [PATCH 02/22] change condition for single file, remove empty lines in single file mode --- pkg/cmd/gist/view/view.go | 15 +++++++++------ pkg/cmd/gist/view/view_test.go | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 31b48325f..4f95af803 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -89,6 +89,11 @@ func viewRun(opts *ViewOptions) error { return err } + cs := opts.IO.ColorScheme() + if gist.Description != "" && opts.Filename == "" { + fmt.Fprintf(opts.IO.Out, "%s\n", cs.Bold(gist.Description)) + } + if opts.Filename != "" { gistFile, ok := gist.Files[opts.Filename] if !ok { @@ -102,11 +107,6 @@ func viewRun(opts *ViewOptions) error { showFilenames := len(gist.Files) > 1 - cs := opts.IO.ColorScheme() - if gist.Description != "" && showFilenames { - fmt.Fprintf(opts.IO.Out, "%s\n", cs.Bold(gist.Description)) - } - outs := []string{} // to ensure consistent ordering for filename, gistFile := range gist.Files { @@ -122,7 +122,10 @@ func viewRun(opts *ViewOptions) error { content = rendered } } - out += fmt.Sprintf("%s\n\n", content) + out += fmt.Sprintf("%s", content) + if opts.Filename == "" { + out += fmt.Sprintf("\n\n") + } outs = append(outs, out) } diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index 832a9ae01..4ecd86538 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -129,7 +129,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "bwhiizzzbwhuiiizzzz\n\n", + wantOut: "bwhiizzzbwhuiiizzzz", }, { name: "filename selected, raw", @@ -150,7 +150,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "bwhiizzzbwhuiiizzzz\n\n", + wantOut: "bwhiizzzbwhuiiizzzz", }, { name: "multiple files, no description", From bf4370bc3ac8498f238c2d788773c650c0125ae3 Mon Sep 17 00:00:00 2001 From: Ruslan Gilyazetdinov Date: Tue, 2 Feb 2021 10:56:29 +0300 Subject: [PATCH 03/22] linter fixes --- pkg/cmd/gist/view/view.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 4f95af803..69186ae59 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -122,9 +122,9 @@ func viewRun(opts *ViewOptions) error { content = rendered } } - out += fmt.Sprintf("%s", content) + out += content if opts.Filename == "" { - out += fmt.Sprintf("\n\n") + out += "\n\n" } outs = append(outs, out) From 3f7b1387e55377d1644e5f606bc18a9d057b9d49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 2 Feb 2021 13:18:23 +0100 Subject: [PATCH 04/22] Improve `gist view` rendering - Separate out logic to render a single file - Render directly to stdout instead of to string slice - Normalize whitespace between files; ensure no excessive trailing whitespace - Add terminal pager support - Sentence-case for flags --- pkg/cmd/gist/view/view.go | 85 ++++++++++++++++++++-------------- pkg/cmd/gist/view/view_test.go | 31 ++++++++++--- 2 files changed, 74 insertions(+), 42 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 69186ae59..e81bbeba6 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -49,9 +49,9 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman }, } - cmd.Flags().BoolVarP(&opts.Raw, "raw", "r", false, "do not try and render markdown") - cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "open gist in browser") - cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "display a single file of the gist") + cmd.Flags().BoolVarP(&opts.Raw, "raw", "r", false, "Print raw instead of rendered gist contents") + cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open gist in the browser") + cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "Display a single file from the gist") return cmd } @@ -89,51 +89,64 @@ func viewRun(opts *ViewOptions) error { return err } - cs := opts.IO.ColorScheme() - if gist.Description != "" && opts.Filename == "" { - fmt.Fprintf(opts.IO.Out, "%s\n", cs.Bold(gist.Description)) + theme := opts.IO.DetectTerminalTheme() + markdownStyle := markdown.GetStyle(theme) + if err := opts.IO.StartPager(); err != nil { + fmt.Fprintf(opts.IO.ErrOut, "starting pager failed: %v\n", err) + } + defer opts.IO.StopPager() + + render := func(gf *shared.GistFile) error { + if strings.Contains(gf.Type, "markdown") && !opts.Raw { + rendered, err := markdown.Render(gf.Content, markdownStyle, "") + if err != nil { + return err + } + _, err = fmt.Fprint(opts.IO.Out, rendered) + return err + } + + if _, err := fmt.Fprint(opts.IO.Out, gf.Content); err != nil { + return err + } + if !strings.HasSuffix(gf.Content, "\n") { + _, err := fmt.Fprint(opts.IO.Out, "\n") + return err + } + return nil } if opts.Filename != "" { gistFile, ok := gist.Files[opts.Filename] if !ok { - return fmt.Errorf("gist has no such file %q", opts.Filename) + return fmt.Errorf("gist has no such file: %q", opts.Filename) } + return render(gistFile) + } - gist.Files = map[string]*shared.GistFile{ - opts.Filename: gistFile, - } + cs := opts.IO.ColorScheme() + + if gist.Description != "" { + fmt.Fprintf(opts.IO.Out, "%s\n\n", cs.Bold(gist.Description)) } showFilenames := len(gist.Files) > 1 - - outs := []string{} // to ensure consistent ordering - - for filename, gistFile := range gist.Files { - out := "" - if showFilenames { - out += fmt.Sprintf("%s\n\n", cs.Gray(filename)) - } - content := gistFile.Content - if strings.Contains(gistFile.Type, "markdown") && !opts.Raw { - style := markdown.GetStyle(opts.IO.DetectTerminalTheme()) - rendered, err := markdown.Render(gistFile.Content, style, "") - if err == nil { - content = rendered - } - } - out += content - if opts.Filename == "" { - out += "\n\n" - } - - outs = append(outs, out) + filenames := make([]string, 0, len(gist.Files)) + for fn := range gist.Files { + filenames = append(filenames, fn) } + sort.Strings(filenames) - sort.Strings(outs) - - for _, out := range outs { - fmt.Fprint(opts.IO.Out, out) + for i, fn := range filenames { + if showFilenames { + fmt.Fprintf(opts.IO.Out, "%s\n\n", cs.Gray(fn)) + } + if err := render(gist.Files[fn]); err != nil { + return err + } + if i < len(filenames)-1 { + fmt.Fprint(opts.IO.Out, "\n") + } } return nil diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index 4ecd86538..a296a10fb 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -109,7 +109,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "bwhiizzzbwhuiiizzzz\n\n", + wantOut: "bwhiizzzbwhuiiizzzz\n", }, { name: "filename selected", @@ -129,7 +129,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "bwhiizzzbwhuiiizzzz", + wantOut: "bwhiizzzbwhuiiizzzz\n", }, { name: "filename selected, raw", @@ -150,7 +150,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "bwhiizzzbwhuiiizzzz", + wantOut: "bwhiizzzbwhuiiizzzz\n", }, { name: "multiple files, no description", @@ -169,7 +169,26 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n # foo \n\n\n\n", + wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n # foo \n\n", + }, + { + name: "multiple files, trailing newlines", + opts: &ViewOptions{ + Selector: "1234", + }, + gist: &shared.Gist{ + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz\n", + Type: "text/plain", + }, + "foo.txt": { + Content: "bar\n", + Type: "text/plain", + }, + }, + }, + wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.txt\n\nbar\n", }, { name: "multiple files, description", @@ -189,7 +208,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "some files\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n \n • foo \n\n\n\n", + wantOut: "some files\n\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n \n • foo \n\n", }, { name: "multiple files, raw", @@ -210,7 +229,7 @@ func Test_viewRun(t *testing.T) { }, }, }, - wantOut: "some files\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n- foo\n\n", + wantOut: "some files\n\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n- foo\n", }, } From ab21dbeaf04898d1bb0505c5f2be9278ebff5933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 3 Feb 2021 22:05:06 +0100 Subject: [PATCH 05/22] Improve `repo create` docs - Clarify what will happen when in the git directory vs. out; - List requirements for non-interactive use; - Demonstrate how to turn issues/wiki off. - Misc. formatting tweaks --- pkg/cmd/repo/create/create.go | 55 +++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index bccd6f9a4..2b1c1f088 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -49,10 +49,26 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "create []", Short: "Create a new repository", - Long: `Create a new GitHub repository.`, - Args: cobra.MaximumNArgs(1), + Long: heredoc.Docf(` + Create a new GitHub repository. + + When the current directory is a local git repository, the new repository will be added + as the "origin" git remote. Otherwise, the command will prompt to clone the new + repository into a sub-directory. + + To create a repository non-interactively, supply the following: + - the name argument; + - the %[1]s--confirm%[1]s flag; + - one of %[1]s--public%[1]s, %[1]s--private%[1]s, or %[1]s--internal%[1]s. + + To toggle off %[1]s--enable-issues%[1]s or %[1]s--enable-wiki%[1]s, which are enabled + by default, use the %[1]s--enable-issues=false%[1]s syntax. + `, "`"), + Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` # create a repository under your account using the current directory name + $ git init my-project + $ cd my-project $ gh repo create # create a repository with a specific name @@ -60,12 +76,16 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co # create a repository in an organization $ gh repo create cli/my-project + + # disable issues and wiki + $ gh repo create --enable-issues=false --enable-wiki=false `), Annotations: map[string]string{ - "help:arguments": heredoc.Doc( - `A repository can be supplied as an argument in any of the following formats: - - - - by URL, e.g. "https://github.com/OWNER/REPO"`), + "help:arguments": heredoc.Doc(` + A repository can be supplied as an argument in any of the following formats: + - "OWNER/REPO" + - by URL, e.g. "https://github.com/OWNER/REPO" + `), }, RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { @@ -78,32 +98,31 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co } if !opts.Internal && !opts.Private && !opts.Public { - return &cmdutil.FlagError{Err: errors.New("--public, --private, or --internal required when not running interactively")} + return &cmdutil.FlagError{Err: errors.New("`--public`, `--private`, or `--internal` required when not running interactively")} } } + if opts.Template != "" && (opts.Homepage != "" || opts.Team != "" || cmd.Flags().Changed("enable-issues") || cmd.Flags().Changed("enable-wiki")) { + return &cmdutil.FlagError{Err: errors.New("The `--template` option is not supported with `--homepage`, `--team`, `--enable-issues`, or `--enable-wiki`")} + } + if runF != nil { return runF(opts) } - - if opts.Template != "" && (opts.Homepage != "" || opts.Team != "" || !opts.EnableIssues || !opts.EnableWiki) { - return &cmdutil.FlagError{Err: errors.New(`The '--template' option is not supported with '--homepage, --team, --enable-issues or --enable-wiki'`)} - } - return createRun(opts) }, } - cmd.Flags().StringVarP(&opts.Description, "description", "d", "", "Description of repository") - cmd.Flags().StringVarP(&opts.Homepage, "homepage", "h", "", "Repository home page URL") - cmd.Flags().StringVarP(&opts.Team, "team", "t", "", "The name of the organization team to be granted access") - cmd.Flags().StringVarP(&opts.Template, "template", "p", "", "Make the new repository based on a template repository") + cmd.Flags().StringVarP(&opts.Description, "description", "d", "", "Description of the repository") + cmd.Flags().StringVarP(&opts.Homepage, "homepage", "h", "", "Repository home page `URL`") + cmd.Flags().StringVarP(&opts.Team, "team", "t", "", "The `name` of the organization team to be granted access") + cmd.Flags().StringVarP(&opts.Template, "template", "p", "", "Make the new repository based on a template `repository`") cmd.Flags().BoolVar(&opts.EnableIssues, "enable-issues", true, "Enable issues in the new repository") cmd.Flags().BoolVar(&opts.EnableWiki, "enable-wiki", true, "Enable wiki in the new repository") cmd.Flags().BoolVar(&opts.Public, "public", false, "Make the new repository public") cmd.Flags().BoolVar(&opts.Private, "private", false, "Make the new repository private") cmd.Flags().BoolVar(&opts.Internal, "internal", false, "Make the new repository internal") - cmd.Flags().BoolVarP(&opts.ConfirmSubmit, "confirm", "y", false, "Confirm the submission directly") + cmd.Flags().BoolVarP(&opts.ConfirmSubmit, "confirm", "y", false, "Skip the confirmation prompt") return cmd } @@ -139,7 +158,7 @@ func createRun(opts *CreateOptions) error { } if enabledFlagCount > 1 { - return fmt.Errorf("expected exactly one of --public, --private, or --internal to be true") + return fmt.Errorf("expected exactly one of `--public`, `--private`, or `--internal` to be true") } else if enabledFlagCount == 1 { isVisibilityPassed = true } From 47baf8fb10a416af8affad89f62af533d7f920e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 3 Feb 2021 22:17:31 +0100 Subject: [PATCH 06/22] `pr create`: explain how to link an issue --- pkg/cmd/pr/create/create.go | 29 ++++++++++++++++------------- pkg/cmd/pr/create/create_test.go | 2 +- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 9deb6b531..3818249a1 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -84,20 +84,23 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "create", Short: "Create a pull request", - Long: heredoc.Doc(` + Long: heredoc.Docf(` Create a pull request on GitHub. When the current branch isn't fully pushed to a git remote, a prompt will ask where - to push the branch and offer an option to fork the base repository. Use '--head' to + to push the branch and offer an option to fork the base repository. Use %[1]s--head%[1]s to explicitly skip any forking or pushing behavior. - A prompt will also ask for the title and the body of the pull request. Use '--title' - and '--body' to skip this, or use '--fill' to autofill these values from git commits. + A prompt will also ask for the title and the body of the pull request. Use %[1]s--title%[1]s + and %[1]s--body%[1]s to skip this, or use %[1]s--fill%[1]s to autofill these values from git commits. - By default users with write access to the base respository can add new commits to your branch. - If undesired, you may disable access of maintainers by using '--no-maintainer-edit' - You can always change this setting later via the web interface. - `), + Link an issue to the pull request by referencing the issue in the body of the pull + request. If the body text mentions %[1]sFixes #123%[1]s or %[1]sCloses #123%[1]s, the referenced issue + will automatically get closed when the pull request gets merged. + + By default, users with write access to the base respository can push new commits to the + head branch of the pull request. Disable this with %[1]s--no-maintainer-edit%[1]s. + `, "`"), Example: heredoc.Doc(` $ gh pr create --title "The bug is fixed" --body "Everything works again" $ gh pr create --reviewer monalisa,hubot --reviewer myorg/team-name @@ -113,21 +116,21 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts.MaintainerCanModify = !noMaintainerEdit if !opts.IO.CanPrompt() && opts.RecoverFile != "" { - return &cmdutil.FlagError{Err: errors.New("--recover only supported when running interactively")} + return &cmdutil.FlagError{Err: errors.New("`--recover` only supported when running interactively")} } if !opts.IO.CanPrompt() && !opts.WebMode && !opts.TitleProvided && !opts.Autofill { - return &cmdutil.FlagError{Err: errors.New("--title or --fill required when not running interactively")} + return &cmdutil.FlagError{Err: errors.New("`--title` or `--fill` required when not running interactively")} } if opts.IsDraft && opts.WebMode { - return errors.New("the --draft flag is not supported with --web") + return errors.New("the `--draft` flag is not supported with `--web`") } if len(opts.Reviewers) > 0 && opts.WebMode { - return errors.New("the --reviewer flag is not supported with --web") + return errors.New("the `--reviewer` flag is not supported with `--web`") } if cmd.Flags().Changed("no-maintainer-edit") && opts.WebMode { - return errors.New("the --no-maintainer-edit flag is not supported with --web") + return errors.New("the `--no-maintainer-edit` flag is not supported with `--web`") } if runF != nil { diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index e1a5183f2..1dbf4c68a 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -120,7 +120,7 @@ func TestPRCreate_nontty_insufficient_flags(t *testing.T) { defer http.Verify(t) output, err := runCommand(http, nil, "feature", false, "") - assert.EqualError(t, err, "--title or --fill required when not running interactively") + assert.EqualError(t, err, "`--title` or `--fill` required when not running interactively") assert.Equal(t, "", output.String()) } From 9dcf47d5d17213bf43fdb85a5b3c1ef889bb1145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 3 Feb 2021 22:33:37 +0100 Subject: [PATCH 07/22] `release create`: clarify handling of git tags --- pkg/cmd/release/create/create.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 9b3eb0fb7..9b87b8845 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -61,18 +61,30 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co var notesFile string cmd := &cobra.Command{ + DisableFlagsInUseLine: true, + Use: "create [...]", Short: "Create a new release", - Long: heredoc.Doc(` + Long: heredoc.Docf(` Create a new GitHub Release for a repository. A list of asset files may be given to upload to the new release. To define a - display label for an asset, append text starting with '#' after the file name. - `), + display label for an asset, append text starting with %[1]s#%[1]s after the file name. + + If a matching git tag does not yet exist, one will automatically get created + from the latest state of the default branch. Use %[1]s--target%[1]s to override this. + To fetch the new tag locally after the release, do %[1]sgit fetch --tags origin%[1]s. + + To create a release from an annotated git tag, first create one locally with + git, push the tag to GitHub, then run this command. + `, "`"), Example: heredoc.Doc(` # use release notes from a file $ gh release create v1.2.3 -F changelog.md + # upload all tarballs in a directory as release assets + $ gh release create v1.2.3 ./dist/*.tgz + # upload a release asset with a display label $ gh release create v1.2.3 '/path/to/asset.zip#My display label' `), @@ -116,7 +128,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().BoolVarP(&opts.Draft, "draft", "d", false, "Save the release as a draft instead of publishing it") cmd.Flags().BoolVarP(&opts.Prerelease, "prerelease", "p", false, "Mark the release as a prerelease") - cmd.Flags().StringVar(&opts.Target, "target", "", "Target `branch` or commit SHA (default: main branch)") + cmd.Flags().StringVar(&opts.Target, "target", "", "Target `branch` or full commit SHA (default: main branch)") cmd.Flags().StringVarP(&opts.Name, "title", "t", "", "Release title") cmd.Flags().StringVarP(&opts.Body, "notes", "n", "", "Release notes") cmd.Flags().StringVarP(¬esFile, "notes-file", "F", "", "Read release notes from `file`") From 907524f459135df0abb6180d37973ac754a10e25 Mon Sep 17 00:00:00 2001 From: Ruslan Gilyazetdinov Date: Tue, 2 Feb 2021 19:17:23 +0300 Subject: [PATCH 08/22] add --files to list filenames in gist (#2885) --- pkg/cmd/gist/view/view.go | 19 +++++-- pkg/cmd/gist/view/view_test.go | 99 +++++++++++++++++++++++++++------- 2 files changed, 94 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index e81bbeba6..95f8223e3 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -19,10 +19,11 @@ type ViewOptions struct { IO *iostreams.IOStreams HttpClient func() (*http.Client, error) - Selector string - Filename string - Raw bool - Web bool + Selector string + Filename string + Raw bool + Web bool + ListFiles bool } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -51,6 +52,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVarP(&opts.Raw, "raw", "r", false, "Print raw instead of rendered gist contents") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open gist in the browser") + cmd.Flags().BoolVarP(&opts.ListFiles, "files", "", false, "Display filenames list from the gist") cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "Display a single file from the gist") return cmd @@ -126,7 +128,7 @@ func viewRun(opts *ViewOptions) error { cs := opts.IO.ColorScheme() - if gist.Description != "" { + if gist.Description != "" && !opts.ListFiles { fmt.Fprintf(opts.IO.Out, "%s\n\n", cs.Bold(gist.Description)) } @@ -137,6 +139,13 @@ func viewRun(opts *ViewOptions) error { } sort.Strings(filenames) + if opts.ListFiles { + for _, fn := range filenames { + fmt.Fprintln(opts.IO.Out, fn) + } + return nil + } + for i, fn := range filenames { if showFilenames { fmt.Fprintf(opts.IO.Out, "%s\n\n", cs.Gray(fn)) diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index a296a10fb..23571e837 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -25,16 +25,18 @@ func TestNewCmdView(t *testing.T) { tty: true, cli: "123", wants: ViewOptions{ - Raw: false, - Selector: "123", + Raw: false, + Selector: "123", + ListFiles: false, }, }, { name: "nontty no arguments", cli: "123", wants: ViewOptions{ - Raw: true, - Selector: "123", + Raw: true, + Selector: "123", + ListFiles: false, }, }, { @@ -42,9 +44,20 @@ func TestNewCmdView(t *testing.T) { cli: "-fcool.txt 123", tty: true, wants: ViewOptions{ - Raw: false, - Selector: "123", - Filename: "cool.txt", + Raw: false, + Selector: "123", + Filename: "cool.txt", + ListFiles: false, + }, + }, + { + name: "files passed", + cli: "--files 123", + tty: true, + wants: ViewOptions{ + Raw: false, + Selector: "123", + ListFiles: true, }, }, } @@ -92,14 +105,16 @@ func Test_viewRun(t *testing.T) { { name: "no such gist", opts: &ViewOptions{ - Selector: "1234", + Selector: "1234", + ListFiles: false, }, wantErr: true, }, { name: "one file", opts: &ViewOptions{ - Selector: "1234", + Selector: "1234", + ListFiles: false, }, gist: &shared.Gist{ Files: map[string]*shared.GistFile{ @@ -114,8 +129,9 @@ func Test_viewRun(t *testing.T) { { name: "filename selected", opts: &ViewOptions{ - Selector: "1234", - Filename: "cicada.txt", + Selector: "1234", + Filename: "cicada.txt", + ListFiles: false, }, gist: &shared.Gist{ Files: map[string]*shared.GistFile{ @@ -134,9 +150,10 @@ func Test_viewRun(t *testing.T) { { name: "filename selected, raw", opts: &ViewOptions{ - Selector: "1234", - Filename: "cicada.txt", - Raw: true, + Selector: "1234", + Filename: "cicada.txt", + Raw: true, + ListFiles: false, }, gist: &shared.Gist{ Files: map[string]*shared.GistFile{ @@ -155,7 +172,8 @@ func Test_viewRun(t *testing.T) { { name: "multiple files, no description", opts: &ViewOptions{ - Selector: "1234", + Selector: "1234", + ListFiles: false, }, gist: &shared.Gist{ Files: map[string]*shared.GistFile{ @@ -174,7 +192,8 @@ func Test_viewRun(t *testing.T) { { name: "multiple files, trailing newlines", opts: &ViewOptions{ - Selector: "1234", + Selector: "1234", + ListFiles: false, }, gist: &shared.Gist{ Files: map[string]*shared.GistFile{ @@ -193,7 +212,8 @@ func Test_viewRun(t *testing.T) { { name: "multiple files, description", opts: &ViewOptions{ - Selector: "1234", + Selector: "1234", + ListFiles: false, }, gist: &shared.Gist{ Description: "some files", @@ -213,8 +233,9 @@ func Test_viewRun(t *testing.T) { { name: "multiple files, raw", opts: &ViewOptions{ - Selector: "1234", - Raw: true, + Selector: "1234", + Raw: true, + ListFiles: false, }, gist: &shared.Gist{ Description: "some files", @@ -231,6 +252,46 @@ func Test_viewRun(t *testing.T) { }, wantOut: "some files\n\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n- foo\n", }, + { + name: "one file, list files", + opts: &ViewOptions{ + Selector: "1234", + Raw: false, + ListFiles: true, + }, + gist: &shared.Gist{ + Description: "some files", + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + }, + wantOut: "cicada.txt\n", + }, + { + name: "multiple file, list files", + opts: &ViewOptions{ + Selector: "1234", + Raw: false, + ListFiles: true, + }, + gist: &shared.Gist{ + Description: "some files", + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "foo.md": { + Content: "- foo", + Type: "application/markdown", + }, + }, + }, + wantOut: "cicada.txt\nfoo.md\n", + }, } for _, tt := range tests { From 622317ee89a45eda6ba91c2b6b9e5d08ebfbc02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 3 Feb 2021 22:39:16 +0100 Subject: [PATCH 09/22] Tweak gist docs --- pkg/cmd/gist/delete/delete.go | 2 +- pkg/cmd/gist/edit/edit.go | 4 ++-- pkg/cmd/gist/view/view.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/gist/delete/delete.go b/pkg/cmd/gist/delete/delete.go index b9e3c20cf..54f745030 100644 --- a/pkg/cmd/gist/delete/delete.go +++ b/pkg/cmd/gist/delete/delete.go @@ -27,7 +27,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co } cmd := &cobra.Command{ - Use: "delete { | }", + Use: "delete { | }", Short: "Delete a gist", Args: cmdutil.MinimumArgs(1, "cannot delete: gist argument required"), RunE: func(c *cobra.Command, args []string) error { diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 5188fe7be..7cba14800 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -47,7 +47,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } cmd := &cobra.Command{ - Use: "edit { | }", + Use: "edit { | }", Short: "Edit one of your gists", Args: cmdutil.MinimumArgs(1, "cannot edit: gist argument required"), RunE: func(c *cobra.Command, args []string) error { @@ -60,7 +60,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman return editRun(&opts) }, } - cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "a specific file to edit") + cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "Select a file to edit") return cmd } diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 95f8223e3..70bb0a238 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -33,7 +33,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } cmd := &cobra.Command{ - Use: "view { | }", + Use: "view { | }", Short: "View a gist", Args: cmdutil.MinimumArgs(1, "cannot view: gist argument required"), RunE: func(cmd *cobra.Command, args []string) error { @@ -52,7 +52,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVarP(&opts.Raw, "raw", "r", false, "Print raw instead of rendered gist contents") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open gist in the browser") - cmd.Flags().BoolVarP(&opts.ListFiles, "files", "", false, "Display filenames list from the gist") + cmd.Flags().BoolVarP(&opts.ListFiles, "files", "", false, "List file names from the gist") cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "Display a single file from the gist") return cmd From b366802aa1b9a770f3b30d3d1f8605090a9769d1 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 27 Jan 2021 14:12:47 -0800 Subject: [PATCH 10/22] Edit issue command --- api/queries_issue.go | 88 +++++--- api/queries_pr.go | 22 +- pkg/cmd/issue/edit/edit.go | 236 +++++++++++++++++++++ pkg/cmd/issue/edit/edit_test.go | 349 ++++++++++++++++++++++++++++++++ pkg/cmd/issue/issue.go | 2 + pkg/cmd/pr/shared/editable.go | 310 ++++++++++++++++++++++++++++ 6 files changed, 964 insertions(+), 43 deletions(-) create mode 100644 pkg/cmd/issue/edit/edit.go create mode 100644 pkg/cmd/issue/edit/edit_test.go create mode 100644 pkg/cmd/pr/shared/editable.go diff --git a/api/queries_issue.go b/api/queries_issue.go index 22f08287b..11c169e0e 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -24,44 +24,52 @@ type IssuesAndTotalCount struct { } type Issue struct { - ID string - Number int - Title string - URL string - State string - Closed bool - Body string - CreatedAt time.Time - UpdatedAt time.Time - Comments Comments - Author Author - Assignees struct { - Nodes []struct { - Login string - } - TotalCount int + ID string + Number int + Title string + URL string + State string + Closed bool + Body string + CreatedAt time.Time + UpdatedAt time.Time + Comments Comments + Author Author + Assignees Assignees + Labels Labels + ProjectCards ProjectCards + Milestone Milestone + ReactionGroups ReactionGroups +} + +type Assignees struct { + Nodes []struct { + Login string } - Labels struct { - Nodes []struct { + TotalCount int +} + +type Labels struct { + Nodes []struct { + Name string + } + TotalCount int +} + +type ProjectCards struct { + Nodes []struct { + Project struct { Name string } - TotalCount int - } - ProjectCards struct { - Nodes []struct { - Project struct { - Name string - } - Column struct { - Name string - } + Column struct { + Name string } - TotalCount int } - Milestone struct { - Title string - } - ReactionGroups ReactionGroups + TotalCount int +} + +type Milestone struct { + Title string } type IssuesDisabledError struct { @@ -488,6 +496,20 @@ func IssueDelete(client *Client, repo ghrepo.Interface, issue Issue) error { return err } +func IssueUpdate(client *Client, repo ghrepo.Interface, params githubv4.UpdateIssueInput) error { + var mutation struct { + UpdateIssue struct { + Issue struct { + ID string + } + } `graphql:"updateIssue(input: $input)"` + } + variables := map[string]interface{}{"input": params} + gql := graphQLClient(client.http, repo.RepoHost()) + err := gql.MutateNamed(context.Background(), "IssueUpdate", &mutation, variables) + return err +} + // milestoneNodeIdToDatabaseId extracts the REST Database ID from the GraphQL Node ID // This conversion is necessary since the GraphQL API requires the use of the milestone's database ID // for querying the related issues. diff --git a/api/queries_pr.go b/api/queries_pr.go index 8b526bdc6..53935ccbe 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -80,16 +80,6 @@ type PullRequest struct { } } } - ReviewRequests struct { - Nodes []struct { - RequestedReviewer struct { - TypeName string `json:"__typename"` - Login string - Name string - } - } - TotalCount int - } Assignees struct { Nodes []struct { Login string @@ -119,6 +109,18 @@ type PullRequest struct { Comments Comments ReactionGroups ReactionGroups Reviews PullRequestReviews + ReviewRequests ReviewRequests +} + +type ReviewRequests struct { + Nodes []struct { + RequestedReviewer struct { + TypeName string `json:"__typename"` + Login string + Name string + } + } + TotalCount int } type NotFoundError struct { diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go new file mode 100644 index 000000000..cfb0ad23e --- /dev/null +++ b/pkg/cmd/issue/edit/edit.go @@ -0,0 +1,236 @@ +package edit + +import ( + "errors" + "fmt" + "net/http" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghrepo" + shared "github.com/cli/cli/pkg/cmd/issue/shared" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/shurcooL/githubv4" + "github.com/spf13/cobra" +) + +type EditOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + OpenInBrowser func(string) error + DetermineEditor func() (string, error) + FieldsToEditSurvey func(*prShared.EditableOptions) error + EditableSurvey func(string, *prShared.EditableOptions) error + FetchOptions func(*api.Client, ghrepo.Interface, *prShared.EditableOptions) error + + SelectorArg string + Interactive bool + WebMode bool + + prShared.EditableOptions +} + +func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { + opts := &EditOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + OpenInBrowser: utils.OpenInBrowser, + DetermineEditor: func() (string, error) { return cmdutil.DetermineEditor(f.Config) }, + FieldsToEditSurvey: prShared.FieldsToEditSurvey, + EditableSurvey: prShared.EditableSurvey, + FetchOptions: prShared.FetchOptions, + } + + cmd := &cobra.Command{ + Use: "edit { | }", + Short: "Edit an issue", + Example: heredoc.Doc(` + $ gh issue edit 23 --title "I found a bug" --body "Nothing works" + $ gh issue edit 23 --label "bug,help wanted" + $ gh issue edit 23 --label bug --label "help wanted" + $ gh issue edit 23 --assignee monalisa,hubot + $ gh issue edit 23 --assignee @me + $ gh issue edit 23 --project "Roadmap" + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + opts.SelectorArg = args[0] + + flags := cmd.Flags() + if flags.Changed("title") { + opts.EditableOptions.TitleEdited = true + } + if flags.Changed("body") { + opts.EditableOptions.BodyEdited = true + } + if flags.Changed("assignee") { + opts.EditableOptions.AssigneesEdited = true + } + if flags.Changed("label") { + opts.EditableOptions.LabelsEdited = true + } + if flags.Changed("project") { + opts.EditableOptions.ProjectsEdited = true + } + if flags.Changed("milestone") { + opts.EditableOptions.MilestoneEdited = true + } + + if !opts.EditableOptions.Dirty() { + opts.Interactive = true + } + + if opts.Interactive && !opts.IO.CanPrompt() { + return &cmdutil.FlagError{Err: errors.New("--tile, --body, --assignee, --label, --project, --milestone, or --web required when not running interactively")} + } + + if runF != nil { + return runF(opts) + } + + return editRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to create an issue") + cmd.Flags().StringVarP(&opts.EditableOptions.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.") + cmd.Flags().StringVarP(&opts.EditableOptions.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.") + cmd.Flags().StringSliceVarP(&opts.EditableOptions.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.") + cmd.Flags().StringSliceVarP(&opts.EditableOptions.Labels, "label", "l", nil, "Add labels by `name`") + cmd.Flags().StringSliceVarP(&opts.EditableOptions.Projects, "project", "p", nil, "Add the issue to projects by `name`") + cmd.Flags().StringVarP(&opts.EditableOptions.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`") + + return cmd +} + +func editRun(opts *EditOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + issue, repo, err := shared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) + if err != nil { + return err + } + + if opts.WebMode { + openURL := issue.URL + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) + } + return opts.OpenInBrowser(openURL) + } + + editOptions := opts.EditableOptions + editOptions.TitleDefault = issue.Title + editOptions.BodyDefault = issue.Body + editOptions.AssigneesDefault = issue.Assignees + editOptions.LabelsDefault = issue.Labels + editOptions.ProjectsDefault = issue.ProjectCards + editOptions.MilestoneDefault = issue.Milestone + + if opts.Interactive { + err = opts.FieldsToEditSurvey(&editOptions) + if err != nil { + return err + } + } + + opts.IO.StartProgressIndicator() + err = opts.FetchOptions(apiClient, repo, &editOptions) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + + if opts.Interactive { + editorCommand, err := opts.DetermineEditor() + if err != nil { + return err + } + err = opts.EditableSurvey(editorCommand, &editOptions) + if err != nil { + return err + } + } + + opts.IO.StartProgressIndicator() + err = updateIssue(apiClient, repo, issue.ID, editOptions) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + + fmt.Fprintln(opts.IO.Out, issue.URL) + + return nil +} + +func updateIssue(client *api.Client, repo ghrepo.Interface, id string, options prShared.EditableOptions) error { + params := githubv4.UpdateIssueInput{ID: id} + if options.TitleEdited { + title := githubv4.String(options.Title) + params.Title = &title + } + if options.BodyEdited { + body := githubv4.String(options.Body) + params.Body = &body + } + if options.AssigneesEdited { + meReplacer := prShared.NewMeReplacer(client, repo.RepoHost()) + assignees, err := meReplacer.ReplaceSlice(options.Assignees) + if err != nil { + return err + } + ids, err := options.Metadata.MembersToIDs(assignees) + if err != nil { + return err + } + assigneeIDs := make([]githubv4.ID, len(ids)) + for i, v := range ids { + assigneeIDs[i] = v + } + params.AssigneeIDs = &assigneeIDs + } + if options.LabelsEdited { + ids, err := options.Metadata.LabelsToIDs(options.Labels) + if err != nil { + return err + } + labelIDs := make([]githubv4.ID, len(ids)) + for i, v := range ids { + labelIDs[i] = v + } + params.LabelIDs = &labelIDs + } + if options.ProjectsEdited { + ids, err := options.Metadata.ProjectsToIDs(options.Projects) + if err != nil { + return err + } + projectIDs := make([]githubv4.ID, len(ids)) + for i, v := range ids { + projectIDs[i] = v + } + params.ProjectIDs = &projectIDs + } + if options.MilestoneEdited { + id, err := options.Metadata.MilestoneToID(options.Milestone) + if err != nil { + return err + } + milestoneID := githubv4.ID(id) + params.MilestoneID = &milestoneID + } + return api.IssueUpdate(client, repo, params) +} diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go new file mode 100644 index 000000000..15616d008 --- /dev/null +++ b/pkg/cmd/issue/edit/edit_test.go @@ -0,0 +1,349 @@ +package edit + +import ( + "bytes" + "net/http" + "testing" + + "github.com/cli/cli/internal/ghrepo" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdEdit(t *testing.T) { + tests := []struct { + name string + input string + output EditOptions + wantsErr bool + }{ + { + name: "no argument", + input: "", + output: EditOptions{}, + wantsErr: true, + }, + { + name: "issue number argument", + input: "23", + output: EditOptions{ + SelectorArg: "23", + Interactive: true, + }, + wantsErr: false, + }, + { + name: "web flag", + input: "23 --web", + output: EditOptions{ + SelectorArg: "23", + Interactive: true, + WebMode: true, + }, + wantsErr: false, + }, + { + name: "title flag", + input: "23 --title test", + output: EditOptions{ + SelectorArg: "23", + EditableOptions: prShared.EditableOptions{ + Title: "test", + TitleEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "body flag", + input: "23 --body test", + output: EditOptions{ + SelectorArg: "23", + EditableOptions: prShared.EditableOptions{ + Body: "test", + BodyEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "assignee flag", + input: "23 --assignee monalisa,hubot", + output: EditOptions{ + SelectorArg: "23", + EditableOptions: prShared.EditableOptions{ + Assignees: []string{"monalisa", "hubot"}, + AssigneesEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "label flag", + input: "23 --label feature,TODO,bug", + output: EditOptions{ + SelectorArg: "23", + EditableOptions: prShared.EditableOptions{ + Labels: []string{"feature", "TODO", "bug"}, + LabelsEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "project flag", + input: "23 --project Cleanup,Roadmap", + output: EditOptions{ + SelectorArg: "23", + EditableOptions: prShared.EditableOptions{ + Projects: []string{"Cleanup", "Roadmap"}, + ProjectsEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "milestone flag", + input: "23 --milestone GA", + output: EditOptions{ + SelectorArg: "23", + EditableOptions: prShared.EditableOptions{ + Milestone: "GA", + MilestoneEdited: true, + }, + }, + wantsErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(true) + io.SetStdinTTY(true) + io.SetStderrTTY(true) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + + var gotOpts *EditOptions + cmd := NewCmdEdit(f, func(opts *EditOptions) error { + gotOpts = opts + return nil + }) + cmd.Flags().BoolP("help", "x", false, "") + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg) + assert.Equal(t, tt.output.WebMode, gotOpts.WebMode) + assert.Equal(t, tt.output.Interactive, gotOpts.Interactive) + assert.Equal(t, tt.output.EditableOptions, gotOpts.EditableOptions) + }) + } +} + +func Test_editRun(t *testing.T) { + tests := []struct { + name string + input *EditOptions + httpStubs func(*testing.T, *httpmock.Registry) + stdout string + stderr string + }{ + { + name: "web mode", + input: &EditOptions{ + SelectorArg: "123", + WebMode: true, + OpenInBrowser: func(string) error { return nil }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockIssueGet(t, reg) + }, + stderr: "Opening github.com/OWNER/REPO/issue/123 in your browser.\n", + }, + { + name: "non-interactive", + input: &EditOptions{ + SelectorArg: "123", + Interactive: false, + EditableOptions: prShared.EditableOptions{ + Title: "new title", + TitleEdited: true, + Body: "new body", + BodyEdited: true, + Assignees: []string{"monalisa", "hubot"}, + AssigneesEdited: true, + Labels: []string{"feature", "TODO", "bug"}, + LabelsEdited: true, + Projects: []string{"Cleanup", "Roadmap"}, + ProjectsEdited: true, + Milestone: "GA", + MilestoneEdited: true, + }, + FetchOptions: prShared.FetchOptions, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockIssueGet(t, reg) + mockRepoMetadata(t, reg) + mockIssueUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/issue/123\n", + }, + { + name: "interactive", + input: &EditOptions{ + SelectorArg: "123", + Interactive: true, + FieldsToEditSurvey: func(eo *prShared.EditableOptions) error { + eo.TitleEdited = true + eo.BodyEdited = true + eo.AssigneesEdited = true + eo.LabelsEdited = true + eo.ProjectsEdited = true + eo.MilestoneEdited = true + return nil + }, + EditableSurvey: func(_ string, eo *prShared.EditableOptions) error { + eo.Title = "new title" + eo.Body = "new body" + eo.Assignees = []string{"monalisa", "hubot"} + eo.Labels = []string{"feature", "TODO", "bug"} + eo.Projects = []string{"Cleanup", "Roadmap"} + eo.Milestone = "GA" + return nil + }, + FetchOptions: prShared.FetchOptions, + DetermineEditor: func() (string, error) { return "vim", nil }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockIssueGet(t, reg) + mockRepoMetadata(t, reg) + mockIssueUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/issue/123\n", + }, + } + for _, tt := range tests { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(true) + io.SetStdinTTY(true) + io.SetStderrTTY(true) + + reg := &httpmock.Registry{} + defer reg.Verify(t) + tt.httpStubs(t, reg) + + httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } + baseRepo := func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } + + tt.input.IO = io + tt.input.HttpClient = httpClient + tt.input.BaseRepo = baseRepo + + t.Run(tt.name, func(t *testing.T) { + err := editRun(tt.input) + assert.NoError(t, err) + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + }) + } +} + +func mockIssueGet(_ *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 123, + "url": "https://github.com/OWNER/REPO/issue/123" + } } } }`), + ) +} + +func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), + httpmock.StringResponse(` + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "hubot", "id": "HUBOTID" }, + { "login": "MonaLisa", "id": "MONAID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryLabelList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "feature", "id": "FEATUREID" }, + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryMilestoneList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "milestones": { + "nodes": [ + { "title": "GA", "id": "GAID" }, + { "title": "Big One.oh", "id": "BIGONEID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryProjectList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "projects": { + "nodes": [ + { "name": "Cleanup", "id": "CLEANUPID" }, + { "name": "Roadmap", "id": "ROADMAPID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query OrganizationProjectList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "projects": { + "nodes": [ + { "name": "Triage", "id": "TRIAGEID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) +} + +func mockIssueUpdate(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation IssueUpdate\b`), + httpmock.GraphQLMutation(` + { "data": { "updateIssue": { "issue": { + "id": "123" + } } } }`, + func(inputs map[string]interface{}) {}), + ) +} diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go index 24b96fb1b..d2af35c0a 100644 --- a/pkg/cmd/issue/issue.go +++ b/pkg/cmd/issue/issue.go @@ -6,6 +6,7 @@ import ( cmdComment "github.com/cli/cli/pkg/cmd/issue/comment" cmdCreate "github.com/cli/cli/pkg/cmd/issue/create" cmdDelete "github.com/cli/cli/pkg/cmd/issue/delete" + cmdEdit "github.com/cli/cli/pkg/cmd/issue/edit" cmdList "github.com/cli/cli/pkg/cmd/issue/list" cmdReopen "github.com/cli/cli/pkg/cmd/issue/reopen" cmdStatus "github.com/cli/cli/pkg/cmd/issue/status" @@ -44,6 +45,7 @@ func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil)) + cmd.AddCommand(cmdEdit.NewCmdEdit(f, nil)) return cmd } diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go new file mode 100644 index 000000000..36bd0387c --- /dev/null +++ b/pkg/cmd/pr/shared/editable.go @@ -0,0 +1,310 @@ +package shared + +import ( + "fmt" + + "github.com/AlecAivazis/survey/v2" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/surveyext" +) + +type EditableOptions struct { + Title string + TitleDefault string + TitleEdited bool + + Body string + BodyDefault string + BodyEdited bool + + Reviewers []string + ReviewersDefault api.ReviewRequests + ReviewersOptions []string + ReviewersEdited bool + ReviewersAllowed bool + + Assignees []string + AssigneesDefault api.Assignees + AssigneesOptions []string + AssigneesEdited bool + + Labels []string + LabelsDefault api.Labels + LabelsOptions []string + LabelsEdited bool + + Projects []string + ProjectsDefault api.ProjectCards + ProjectsOptions []string + ProjectsEdited bool + + Milestone string + MilestoneDefault api.Milestone + MilestoneOptions []string + MilestoneEdited bool + + Metadata api.RepoMetadataResult +} + +func (e EditableOptions) Dirty() bool { + return e.TitleEdited || + e.BodyEdited || + e.ReviewersEdited || + e.AssigneesEdited || + e.LabelsEdited || + e.ProjectsEdited || + e.MilestoneEdited +} + +func EditableSurvey(editorCommand string, options *EditableOptions) error { + if options.TitleEdited { + title, err := titleSurvey(options.TitleDefault) + if err != nil { + return err + } + options.Title = title + } + if options.BodyEdited { + body, err := bodySurvey(options.BodyDefault, editorCommand) + if err != nil { + return err + } + options.Body = body + } + if options.AssigneesEdited { + assignees, err := assigneesSurvey(options.AssigneesDefault, options.AssigneesOptions) + if err != nil { + return err + } + options.Assignees = assignees + } + if options.LabelsEdited { + labels, err := labelsSurvey(options.LabelsDefault, options.LabelsOptions) + if err != nil { + return err + } + options.Labels = labels + } + if options.ProjectsEdited { + projects, err := projectsSurvey(options.ProjectsDefault, options.ProjectsOptions) + if err != nil { + return err + } + options.Projects = projects + } + if options.MilestoneEdited { + milestone, err := milestoneSurvey(options.MilestoneDefault, options.MilestoneOptions) + if err != nil { + return err + } + options.Milestone = milestone + } + confirm, err := confirmSurvey() + if err != nil { + return err + } + if !confirm { + return fmt.Errorf("Discarding...") + } + + return nil +} + +func FieldsToEditSurvey(options *EditableOptions) error { + contains := func(s []string, str string) bool { + for _, v := range s { + if v == str { + return true + } + } + return false + } + + results := []string{} + opts := []string{"Title", "Body"} + if options.ReviewersAllowed { + opts = append(opts, "Reviewers") + } + opts = append(opts, "Assignees", "Labels", "Projects", "Milestone") + q := &survey.MultiSelect{ + Message: "What would you like to edit?", + Options: opts, + } + err := survey.AskOne(q, &results) + if err != nil { + return err + } + + if contains(results, "Title") { + options.TitleEdited = true + } + if contains(results, "Body") { + options.BodyEdited = true + } + if contains(results, "Reviewers") { + options.ReviewersEdited = true + } + if contains(results, "Assignees") { + options.AssigneesEdited = true + } + if contains(results, "Labels") { + options.LabelsEdited = true + } + if contains(results, "Projects") { + options.ProjectsEdited = true + } + if contains(results, "Milestone") { + options.MilestoneEdited = true + } + + return nil +} + +func FetchOptions(client *api.Client, repo ghrepo.Interface, options *EditableOptions) error { + input := api.RepoMetadataInput{ + Reviewers: options.ReviewersEdited, + Assignees: options.AssigneesEdited, + Labels: options.LabelsEdited, + Projects: options.ProjectsEdited, + Milestones: options.MilestoneEdited, + } + metadata, err := api.RepoMetadata(client, repo, input) + if err != nil { + return err + } + + var users []string + for _, u := range metadata.AssignableUsers { + users = append(users, u.Login) + } + var teams []string + for _, t := range metadata.Teams { + teams = append(teams, fmt.Sprintf("%s/%s", repo.RepoOwner(), t.Slug)) + } + var labels []string + for _, l := range metadata.Labels { + labels = append(labels, l.Name) + } + var projects []string + for _, l := range metadata.Projects { + projects = append(projects, l.Name) + } + milestones := []string{noMilestone} + for _, m := range metadata.Milestones { + milestones = append(milestones, m.Title) + } + + options.Metadata = *metadata + options.ReviewersOptions = append(users, teams...) + options.AssigneesOptions = users + options.LabelsOptions = labels + options.ProjectsOptions = projects + options.MilestoneOptions = milestones + + return nil +} + +func titleSurvey(title string) (string, error) { + var result string + q := &survey.Input{ + Message: "Title", + Default: title, + } + err := survey.AskOne(q, &result) + return result, err +} + +func bodySurvey(body, editorCommand string) (string, error) { + var result string + q := &surveyext.GhEditor{ + BlankAllowed: true, + EditorCommand: editorCommand, + Editor: &survey.Editor{Message: "Body", + FileName: "*.md", + Default: body, + HideDefault: true, + AppendDefault: true, + }, + } + err := survey.AskOne(q, &result) + return result, err +} + +func assigneesSurvey(assignees api.Assignees, assigneesOpts []string) ([]string, error) { + if len(assigneesOpts) == 0 { + return nil, nil + } + logins := []string{} + for _, a := range assignees.Nodes { + logins = append(logins, a.Login) + } + var results []string + q := &survey.MultiSelect{ + Message: "Assignees", + Options: assigneesOpts, + Default: logins, + } + err := survey.AskOne(q, &results) + return results, err +} + +func labelsSurvey(labels api.Labels, labelOpts []string) ([]string, error) { + if len(labelOpts) == 0 { + return nil, nil + } + names := []string{} + for _, l := range labels.Nodes { + names = append(names, l.Name) + } + var results []string + q := &survey.MultiSelect{ + Message: "Labels", + Options: labelOpts, + Default: names, + } + err := survey.AskOne(q, &results) + return results, err +} + +func projectsSurvey(projectCards api.ProjectCards, projectsOpts []string) ([]string, error) { + if len(projectsOpts) == 0 { + return nil, nil + } + names := []string{} + for _, c := range projectCards.Nodes { + names = append(names, c.Project.Name) + } + var results []string + q := &survey.MultiSelect{ + Message: "Projects", + Options: projectsOpts, + Default: names, + } + err := survey.AskOne(q, &results) + return results, err +} + +func milestoneSurvey(milestone api.Milestone, milestoneOpts []string) (string, error) { + if len(milestoneOpts) == 0 { + return "", nil + } + var result string + q := &survey.Select{ + Message: "Milestone", + Options: milestoneOpts, + Default: milestone.Title, + } + err := survey.AskOne(q, &result) + return result, err +} + +func confirmSurvey() (bool, error) { + var result bool + q := &survey.Confirm{ + Message: "Submit?", + Default: true, + } + err := survey.AskOne(q, &result) + return result, err +} From 9920ea97f6b51f84d3376dbb30193c64370c1f9c Mon Sep 17 00:00:00 2001 From: xvqxy <76832739+xvqxy@users.noreply.github.com> Date: Sat, 6 Feb 2021 09:49:43 +0100 Subject: [PATCH 11/22] Display output of build commands. This fixes #2920. Print out output of executed command to stdout/stderr. --- script/build.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/build.go b/script/build.go index 3b1a08ea5..3d26ca7cb 100644 --- a/script/build.go +++ b/script/build.go @@ -165,6 +165,8 @@ func run(args ...string) error { } announce(args...) cmd := exec.Command(exe, args[1:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr return cmd.Run() } From feb4acc2c00ce42d5ed67252ae1ec66addeb786d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 8 Feb 2021 13:57:08 +0100 Subject: [PATCH 12/22] Suggest `brew upgrade gh` when new version detected When the update notifier is enabled and a new version was detected, show a Homebrew upgrade notice if: - the release was at least 24 hours ago; and - the current `gh` binary is under the Homebrew prefix. --- cmd/gh/main.go | 34 ++++++++++++++++++++++++++++++---- internal/update/update.go | 5 +++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index f1e72f1b5..a0eba09a5 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -8,7 +8,9 @@ import ( "os" "os/exec" "path" + "path/filepath" "strings" + "time" surveyCore "github.com/AlecAivazis/survey/v2/core" "github.com/cli/cli/api" @@ -161,13 +163,16 @@ func main() { newRelease := <-updateMessageChan if newRelease != nil { - msg := fmt.Sprintf("%s %s → %s\n%s", + ghExe, _ := os.Executable() + fmt.Fprintf(stderr, "\n\n%s %s → %s\n", ansi.Color("A new release of gh is available:", "yellow"), ansi.Color(buildVersion, "cyan"), - ansi.Color(newRelease.Version, "cyan"), + ansi.Color(newRelease.Version, "cyan")) + if suggestBrewUpgrade(newRelease, ghExe) { + fmt.Fprintf(stderr, "To upgrade, run: %s\n", "brew update && brew upgrade gh") + } + fmt.Fprintf(stderr, "%s\n\n", ansi.Color(newRelease.URL, "yellow")) - - fmt.Fprintf(stderr, "\n\n%s\n\n", msg) } } @@ -259,3 +264,24 @@ func apiVerboseLog() api.ClientOption { colorize := utils.IsTerminal(os.Stderr) return api.VerboseLog(colorable.NewColorable(os.Stderr), logTraffic, colorize) } + +// Suggest to `brew upgrade gh` only if gh was found under homebrew prefix and when the release was +// published over 24h ago, allowing homebrew-core ample time to merge the formula bump. +func suggestBrewUpgrade(rel *update.ReleaseInfo, ghBinary string) bool { + if rel.PublishedAt.IsZero() || time.Since(rel.PublishedAt) < time.Duration(time.Hour*24) { + return false + } + + brewExe, err := safeexec.LookPath("brew") + if err != nil { + return false + } + + brewPrefixBytes, err := exec.Command(brewExe, "--prefix").Output() + if err != nil { + return false + } + + brewBinPrefix := filepath.Join(strings.TrimSpace(string(brewPrefixBytes)), "bin") + string(filepath.Separator) + return strings.HasPrefix(ghBinary, brewBinPrefix) +} diff --git a/internal/update/update.go b/internal/update/update.go index b647d5a5c..024fd2377 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -18,8 +18,9 @@ var gitDescribeSuffixRE = regexp.MustCompile(`\d+-\d+-g[a-f0-9]{8}$`) // ReleaseInfo stores information about a release type ReleaseInfo struct { - Version string `json:"tag_name"` - URL string `json:"html_url"` + Version string `json:"tag_name"` + URL string `json:"html_url"` + PublishedAt time.Time `json:"published_at"` } type StateEntry struct { From 68f71d82a0f69607bb6f9e2bd3513d7f19da166c Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 8 Feb 2021 09:17:04 -0800 Subject: [PATCH 13/22] Remove webmode --- pkg/cmd/issue/edit/edit.go | 12 +----------- pkg/cmd/issue/edit/edit_test.go | 23 ----------------------- 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index cfb0ad23e..a53e2eacb 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -30,7 +30,6 @@ type EditOptions struct { SelectorArg string Interactive bool - WebMode bool prShared.EditableOptions } @@ -89,7 +88,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } if opts.Interactive && !opts.IO.CanPrompt() { - return &cmdutil.FlagError{Err: errors.New("--tile, --body, --assignee, --label, --project, --milestone, or --web required when not running interactively")} + return &cmdutil.FlagError{Err: errors.New("--tile, --body, --assignee, --label, --project, or --milestone required when not running interactively")} } if runF != nil { @@ -100,7 +99,6 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman }, } - cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to create an issue") cmd.Flags().StringVarP(&opts.EditableOptions.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.") cmd.Flags().StringVarP(&opts.EditableOptions.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.") cmd.Flags().StringSliceVarP(&opts.EditableOptions.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.") @@ -123,14 +121,6 @@ func editRun(opts *EditOptions) error { return err } - if opts.WebMode { - openURL := issue.URL - if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) - } - return opts.OpenInBrowser(openURL) - } - editOptions := opts.EditableOptions editOptions.TitleDefault = issue.Title editOptions.BodyDefault = issue.Body diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 15616d008..a5d090c51 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -36,16 +36,6 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, - { - name: "web flag", - input: "23 --web", - output: EditOptions{ - SelectorArg: "23", - Interactive: true, - WebMode: true, - }, - wantsErr: false, - }, { name: "title flag", input: "23 --title test", @@ -153,7 +143,6 @@ func TestNewCmdEdit(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg) - assert.Equal(t, tt.output.WebMode, gotOpts.WebMode) assert.Equal(t, tt.output.Interactive, gotOpts.Interactive) assert.Equal(t, tt.output.EditableOptions, gotOpts.EditableOptions) }) @@ -168,18 +157,6 @@ func Test_editRun(t *testing.T) { stdout string stderr string }{ - { - name: "web mode", - input: &EditOptions{ - SelectorArg: "123", - WebMode: true, - OpenInBrowser: func(string) error { return nil }, - }, - httpStubs: func(t *testing.T, reg *httpmock.Registry) { - mockIssueGet(t, reg) - }, - stderr: "Opening github.com/OWNER/REPO/issue/123 in your browser.\n", - }, { name: "non-interactive", input: &EditOptions{ From 4ed94c2a069843b554e61ddc98e695c5fcf91d6f Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 8 Feb 2021 09:59:30 -0800 Subject: [PATCH 14/22] Fix up flag descriptions --- pkg/cmd/issue/edit/edit.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index a53e2eacb..cb6e55afd 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -99,12 +99,12 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman }, } - cmd.Flags().StringVarP(&opts.EditableOptions.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.") - cmd.Flags().StringVarP(&opts.EditableOptions.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.") - cmd.Flags().StringSliceVarP(&opts.EditableOptions.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.") - cmd.Flags().StringSliceVarP(&opts.EditableOptions.Labels, "label", "l", nil, "Add labels by `name`") - cmd.Flags().StringSliceVarP(&opts.EditableOptions.Projects, "project", "p", nil, "Add the issue to projects by `name`") - cmd.Flags().StringVarP(&opts.EditableOptions.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`") + cmd.Flags().StringVarP(&opts.EditableOptions.Title, "title", "t", "", "Revise the issue title.") + cmd.Flags().StringVarP(&opts.EditableOptions.Body, "body", "b", "", "Revise the issue body.") + cmd.Flags().StringSliceVarP(&opts.EditableOptions.Assignees, "assignee", "a", nil, "Set assigned people by their `login`. Use \"@me\" to self-assign.") + cmd.Flags().StringSliceVarP(&opts.EditableOptions.Labels, "label", "l", nil, "Set the issue labels by `name`") + cmd.Flags().StringSliceVarP(&opts.EditableOptions.Projects, "project", "p", nil, "Set the projects the issue belongs to by `name`") + cmd.Flags().StringVarP(&opts.EditableOptions.Milestone, "milestone", "m", "", "Set the milestone the issue belongs to by `name`") return cmd } From cd9f211826c7f6a35d218401f81b8f5db750b7b7 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 9 Feb 2021 09:48:58 -0800 Subject: [PATCH 15/22] Remove unused code --- pkg/cmd/issue/edit/edit.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index cb6e55afd..856537e3e 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -12,7 +12,6 @@ import ( prShared "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/shurcooL/githubv4" "github.com/spf13/cobra" ) @@ -22,7 +21,6 @@ type EditOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) - OpenInBrowser func(string) error DetermineEditor func() (string, error) FieldsToEditSurvey func(*prShared.EditableOptions) error EditableSurvey func(string, *prShared.EditableOptions) error @@ -38,7 +36,6 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman opts := &EditOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, - OpenInBrowser: utils.OpenInBrowser, DetermineEditor: func() (string, error) { return cmdutil.DetermineEditor(f.Config) }, FieldsToEditSurvey: prShared.FieldsToEditSurvey, EditableSurvey: prShared.EditableSurvey, From 335f0117c0d0be4af63692ccab866b9cc9a227fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 11 Feb 2021 19:16:11 +0100 Subject: [PATCH 16/22] Add more examples to `api` docs - Clarify that fields need to be in "key=value" format - Headers need to be in "key:value" format - Contrast POST vs GET requests with params in examples - Add an example of how to add HTTP headers - Use backticks where applicable --- pkg/cmd/api/api.go | 80 +++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 63437937c..8ebdd3bc7 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -55,44 +55,57 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command cmd := &cobra.Command{ Use: "api ", Short: "Make an authenticated GitHub API request", - Long: `Makes an authenticated HTTP request to the GitHub API and prints the response. + Long: heredoc.Docf(` + Makes an authenticated HTTP request to the GitHub API and prints the response. -The endpoint argument should either be a path of a GitHub API v3 endpoint, or -"graphql" to access the GitHub API v4. + The endpoint argument should either be a path of a GitHub API v3 endpoint, or + "graphql" to access the GitHub API v4. -Placeholder values ":owner", ":repo", and ":branch" in the endpoint argument will -get replaced with values from the repository of the current directory. + Placeholder values ":owner", ":repo", and ":branch" in the endpoint argument will + get replaced with values from the repository of the current directory. -The default HTTP request method is "GET" normally and "POST" if any parameters -were added. Override the method with '--method'. + The default HTTP request method is "GET" normally and "POST" if any parameters + were added. Override the method with %[1]s--method%[1]s. -Pass one or more '--raw-field' values in "key=value" format to add -JSON-encoded string parameters to the POST body. + Pass one or more %[1]s--raw-field%[1]s values in "key=value" format to add + JSON-encoded string parameters to the POST body. -The '--field' flag behaves like '--raw-field' with magic type conversion based -on the format of the value: + The %[1]s--field%[1]s flag behaves like %[1]s--raw-field%[1]s with magic type conversion based + on the format of the value: -- literal values "true", "false", "null", and integer numbers get converted to - appropriate JSON types; -- placeholder values ":owner", ":repo", and ":branch" get populated with values - from the repository of the current directory; -- if the value starts with "@", the rest of the value is interpreted as a - filename to read the value from. Pass "-" to read from standard input. + - literal values "true", "false", "null", and integer numbers get converted to + appropriate JSON types; + - placeholder values ":owner", ":repo", and ":branch" get populated with values + from the repository of the current directory; + - if the value starts with "@", the rest of the value is interpreted as a + filename to read the value from. Pass "-" to read from standard input. -For GraphQL requests, all fields other than "query" and "operationName" are -interpreted as GraphQL variables. + For GraphQL requests, all fields other than "query" and "operationName" are + interpreted as GraphQL variables. -Raw request body may be passed from the outside via a file specified by '--input'. -Pass "-" to read from standard input. In this mode, parameters specified via -'--field' flags are serialized into URL query parameters. + Raw request body may be passed from the outside via a file specified by %[1]s--input%[1]s. + Pass "-" to read from standard input. In this mode, parameters specified via + %[1]s--field%[1]s flags are serialized into URL query parameters. -In '--paginate' mode, all pages of results will sequentially be requested until -there are no more pages of results. For GraphQL requests, this requires that the -original query accepts an '$endCursor: String' variable and that it fetches the -'pageInfo{ hasNextPage, endCursor }' set of fields from a collection.`, + In %[1]s--paginate%[1]s mode, all pages of results will sequentially be requested until + there are no more pages of results. For GraphQL requests, this requires that the + original query accepts an %[1]s$endCursor: String%[1]s variable and that it fetches the + %[1]spageInfo{ hasNextPage, endCursor }%[1]s set of fields from a collection. + `, "`"), Example: heredoc.Doc(` + # list releases in the current repository $ gh api repos/:owner/:repo/releases + # post an issue comment + $ gh api repos/:owner/:repo/issues/123/comments -f body='Hi from CLI' + + # add parameters to a GET request + $ gh api -X GET search/issues -f q='repo:cli/cli is:open remote' + + # set a custom HTTP header + $ gh api -H 'Accept: application/vnd.github.XYZ-preview+json' ... + + # list releases with GraphQL $ gh api graphql -F owner=':owner' -F name=':repo' -f query=' query($name: String!, $owner: String!) { repository(owner: $owner, name: $name) { @@ -103,6 +116,7 @@ original query accepts an '$endCursor: String' variable and that it fetches the } ' + # list all repositories for a user $ gh api graphql --paginate -f query=' query($endCursor: String) { viewer { @@ -119,9 +133,11 @@ original query accepts an '$endCursor: String' variable and that it fetches the `), Annotations: map[string]string{ "help:environment": heredoc.Doc(` - GH_TOKEN, GITHUB_TOKEN (in order of precedence): an authentication token for github.com API requests. + GH_TOKEN, GITHUB_TOKEN (in order of precedence): an authentication token for + github.com API requests. - GH_ENTERPRISE_TOKEN, GITHUB_ENTERPRISE_TOKEN (in order of precedence): an authentication token for API requests to GitHub Enterprise. + GH_ENTERPRISE_TOKEN, GITHUB_ENTERPRISE_TOKEN (in order of precedence): an + authentication token for API requests to GitHub Enterprise. GH_HOST: make the request to a GitHub host other than github.com. `), @@ -153,12 +169,12 @@ original query accepts an '$endCursor: String' variable and that it fetches the cmd.Flags().StringVar(&opts.Hostname, "hostname", "", "The GitHub hostname for the request (default \"github.com\")") cmd.Flags().StringVarP(&opts.RequestMethod, "method", "X", "GET", "The HTTP method for the request") - cmd.Flags().StringArrayVarP(&opts.MagicFields, "field", "F", nil, "Add a parameter of inferred type") - cmd.Flags().StringArrayVarP(&opts.RawFields, "raw-field", "f", nil, "Add a string parameter") - cmd.Flags().StringArrayVarP(&opts.RequestHeaders, "header", "H", nil, "Add an additional HTTP request header") + cmd.Flags().StringArrayVarP(&opts.MagicFields, "field", "F", nil, "Add a typed parameter in `key=value` format") + cmd.Flags().StringArrayVarP(&opts.RawFields, "raw-field", "f", nil, "Add a string parameter in `key=value` format") + cmd.Flags().StringArrayVarP(&opts.RequestHeaders, "header", "H", nil, "Add a HTTP request header in `key:value` format") cmd.Flags().BoolVarP(&opts.ShowResponseHeaders, "include", "i", false, "Include HTTP response headers in the output") cmd.Flags().BoolVar(&opts.Paginate, "paginate", false, "Make additional HTTP requests to fetch all pages of results") - cmd.Flags().StringVar(&opts.RequestInputFile, "input", "", "The file to use as body for the HTTP request") + cmd.Flags().StringVar(&opts.RequestInputFile, "input", "", "The `file` to use as body for the HTTP request") cmd.Flags().BoolVar(&opts.Silent, "silent", false, "Do not print the response body") return cmd } From 0d55f8648c0540c4a0aaaf568a39967fbbff93ed Mon Sep 17 00:00:00 2001 From: Michael Neeley Date: Thu, 11 Feb 2021 16:27:23 -0500 Subject: [PATCH 17/22] adds merge state status --- api/client.go | 1 + api/queries_pr.go | 2 ++ pkg/cmd/pr/status/status.go | 10 ++++++++++ 3 files changed, 13 insertions(+) diff --git a/api/client.go b/api/client.go index 09195181b..17534b8b2 100644 --- a/api/client.go +++ b/api/client.go @@ -239,6 +239,7 @@ func (c Client) GraphQL(hostname string, query string, variables map[string]inte } req.Header.Set("Content-Type", "application/json; charset=utf-8") + req.Header.Set("Accept", "application/vnd.github.merge-info-preview+json") resp, err := c.http.Do(req) if err != nil { diff --git a/api/queries_pr.go b/api/queries_pr.go index 8b526bdc6..898d3297a 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -38,6 +38,7 @@ type PullRequest struct { HeadRefName string Body string Mergeable string + MergeStateStatus string Author struct { Login string @@ -353,6 +354,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu state url headRefName + mergeStateStatus headRepositoryOwner { login } diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index bec49f259..3758a9d67 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -227,6 +227,16 @@ func printPrs(io *iostreams.IOStreams, totalCount int, prs ...api.PullRequest) { } else if reviews.Approved { fmt.Fprint(w, cs.Green("✓ Approved")) } + + // add padding between reviews & merge status + fmt.Fprint(w, " ") + + if pr.MergeStateStatus == "BEHIND" { + fmt.Fprint(w, cs.Yellow("- Not up to date")) + } else { + fmt.Fprint(w, cs.Green("✓ Up to date")) + } + } else { fmt.Fprintf(w, " - %s", shared.StateTitleWithColor(cs, pr)) } From 8511365afb56a3ea24b3c7f17768ef2080b519ad Mon Sep 17 00:00:00 2001 From: Michael Neeley Date: Thu, 11 Feb 2021 16:46:16 -0500 Subject: [PATCH 18/22] linter --- api/queries_pr.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 898d3297a..273f95f40 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -28,17 +28,17 @@ type PullRequestAndTotalCount struct { } type PullRequest struct { - ID string - Number int - Title string - State string - Closed bool - URL string - BaseRefName string - HeadRefName string - Body string - Mergeable string - MergeStateStatus string + ID string + Number int + Title string + State string + Closed bool + URL string + BaseRefName string + HeadRefName string + Body string + Mergeable string + MergeStateStatus string Author struct { Login string From 9be9229a48c2fca10942eb707b94fcc16286cd05 Mon Sep 17 00:00:00 2001 From: Michael Neeley Date: Fri, 12 Feb 2021 08:51:47 -0500 Subject: [PATCH 19/22] adds strict status checks --- api/queries_pr.go | 33 ++++++++++++++++++++++++--------- pkg/cmd/pr/status/status.go | 23 +++++++++++++---------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 273f95f40..b40b9a32e 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -16,10 +16,11 @@ import ( ) type PullRequestsPayload struct { - ViewerCreated PullRequestAndTotalCount - ReviewRequested PullRequestAndTotalCount - CurrentPR *PullRequest - DefaultBranch string + ViewerCreated PullRequestAndTotalCount + ReviewRequested PullRequestAndTotalCount + CurrentPR *PullRequest + DefaultBranch string + StrictProtection bool } type PullRequestAndTotalCount struct { @@ -302,7 +303,10 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu type response struct { Repository struct { DefaultBranchRef struct { - Name string + Name string + BranchProtectionRule struct { + RequiresStrictStatusChecks bool + } } PullRequests edges PullRequest *PullRequest @@ -371,7 +375,12 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu queryPrefix := ` query PullRequestStatus($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { - defaultBranchRef { name } + defaultBranchRef { + name + branchProtectionRule { + requiresStrictStatusChecks + } + } pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) { totalCount edges { @@ -386,7 +395,12 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu queryPrefix = ` query PullRequestStatus($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { - defaultBranchRef { name } + defaultBranchRef { + name + branchProtectionRule { + requiresStrictStatusChecks + } + } pullRequest(number: $number) { ...prWithReviews } @@ -473,8 +487,9 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu PullRequests: reviewRequested, TotalCount: resp.ReviewRequested.TotalCount, }, - CurrentPR: currentPR, - DefaultBranch: resp.Repository.DefaultBranchRef.Name, + CurrentPR: currentPR, + DefaultBranch: resp.Repository.DefaultBranchRef.Name, + StrictProtection: resp.Repository.DefaultBranchRef.BranchProtectionRule.RequiresStrictStatusChecks, } return &payload, nil diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index 3758a9d67..5a210941a 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -114,7 +114,7 @@ func statusRun(opts *StatusOptions) error { currentPR = nil } if currentPR != nil { - printPrs(opts.IO, 1, *currentPR) + printPrs(opts.IO, 1, prPayload.StrictProtection, *currentPR) } else if currentPRHeadRef == "" { shared.PrintMessage(opts.IO, " There is no current branch") } else { @@ -124,7 +124,7 @@ func statusRun(opts *StatusOptions) error { shared.PrintHeader(opts.IO, "Created by you") if prPayload.ViewerCreated.TotalCount > 0 { - printPrs(opts.IO, prPayload.ViewerCreated.TotalCount, prPayload.ViewerCreated.PullRequests...) + printPrs(opts.IO, prPayload.ViewerCreated.TotalCount, prPayload.StrictProtection, prPayload.ViewerCreated.PullRequests...) } else { shared.PrintMessage(opts.IO, " You have no open pull requests") } @@ -132,7 +132,7 @@ func statusRun(opts *StatusOptions) error { shared.PrintHeader(opts.IO, "Requesting a code review from you") if prPayload.ReviewRequested.TotalCount > 0 { - printPrs(opts.IO, prPayload.ReviewRequested.TotalCount, prPayload.ReviewRequested.PullRequests...) + printPrs(opts.IO, prPayload.ReviewRequested.TotalCount, prPayload.StrictProtection, prPayload.ReviewRequested.PullRequests...) } else { shared.PrintMessage(opts.IO, " You have no pull requests to review") } @@ -178,7 +178,7 @@ func prSelectorForCurrentBranch(baseRepo ghrepo.Interface, prHeadRef string, rem return } -func printPrs(io *iostreams.IOStreams, totalCount int, prs ...api.PullRequest) { +func printPrs(io *iostreams.IOStreams, totalCount int, strictProtection bool, prs ...api.PullRequest) { w := io.Out cs := io.ColorScheme() @@ -228,13 +228,16 @@ func printPrs(io *iostreams.IOStreams, totalCount int, prs ...api.PullRequest) { fmt.Fprint(w, cs.Green("✓ Approved")) } - // add padding between reviews & merge status - fmt.Fprint(w, " ") + // only check if the "up to date" setting is checked in repo settings + if strictProtection { + // add padding between reviews & merge status + fmt.Fprint(w, " ") - if pr.MergeStateStatus == "BEHIND" { - fmt.Fprint(w, cs.Yellow("- Not up to date")) - } else { - fmt.Fprint(w, cs.Green("✓ Up to date")) + if pr.MergeStateStatus == "BEHIND" { + fmt.Fprint(w, cs.Yellow("- Not up to date")) + } else { + fmt.Fprint(w, cs.Green("✓ Up to date")) + } } } else { From a47ee660a79cc578e3c860015e2329fcecfe84be Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 5 Feb 2021 11:46:58 -0800 Subject: [PATCH 20/22] Pr edit command --- api/queries_pr.go | 28 ++ pkg/cmd/issue/edit/edit.go | 127 ++++------ pkg/cmd/issue/edit/edit_test.go | 20 +- pkg/cmd/pr/edit/edit.go | 259 +++++++++++++++++++ pkg/cmd/pr/edit/edit_test.go | 435 ++++++++++++++++++++++++++++++++ pkg/cmd/pr/pr.go | 2 + pkg/cmd/pr/shared/editable.go | 254 ++++++++++++++----- 7 files changed, 967 insertions(+), 158 deletions(-) create mode 100644 pkg/cmd/pr/edit/edit.go create mode 100644 pkg/cmd/pr/edit/edit_test.go diff --git a/api/queries_pr.go b/api/queries_pr.go index 53935ccbe..e91216479 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -835,6 +835,34 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter return pr, nil } +func UpdatePullRequest(client *Client, repo ghrepo.Interface, params githubv4.UpdatePullRequestInput) error { + var mutation struct { + UpdatePullRequest struct { + PullRequest struct { + ID string + } + } `graphql:"updatePullRequest(input: $input)"` + } + variables := map[string]interface{}{"input": params} + gql := graphQLClient(client.http, repo.RepoHost()) + err := gql.MutateNamed(context.Background(), "PullRequestUpdate", &mutation, variables) + return err +} + +func UpdatePullRequestReviews(client *Client, repo ghrepo.Interface, params githubv4.RequestReviewsInput) error { + var mutation struct { + RequestReviews struct { + PullRequest struct { + ID string + } + } `graphql:"requestReviews(input: $input)"` + } + variables := map[string]interface{}{"input": params} + gql := graphQLClient(client.http, repo.RepoHost()) + err := gql.MutateNamed(context.Background(), "PullRequestUpdateRequestReviews", &mutation, variables) + return err +} + func isBlank(v interface{}) bool { switch vv := v.(type) { case string: diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 856537e3e..c209c8fc7 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -22,14 +22,14 @@ type EditOptions struct { BaseRepo func() (ghrepo.Interface, error) DetermineEditor func() (string, error) - FieldsToEditSurvey func(*prShared.EditableOptions) error - EditableSurvey func(string, *prShared.EditableOptions) error - FetchOptions func(*api.Client, ghrepo.Interface, *prShared.EditableOptions) error + FieldsToEditSurvey func(*prShared.Editable) error + EditFieldsSurvey func(*prShared.Editable, string) error + FetchOptions func(*api.Client, ghrepo.Interface, *prShared.Editable) error SelectorArg string Interactive bool - prShared.EditableOptions + prShared.Editable } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -38,7 +38,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman HttpClient: f.HttpClient, DetermineEditor: func() (string, error) { return cmdutil.DetermineEditor(f.Config) }, FieldsToEditSurvey: prShared.FieldsToEditSurvey, - EditableSurvey: prShared.EditableSurvey, + EditFieldsSurvey: prShared.EditFieldsSurvey, FetchOptions: prShared.FetchOptions, } @@ -62,25 +62,25 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman flags := cmd.Flags() if flags.Changed("title") { - opts.EditableOptions.TitleEdited = true + opts.Editable.TitleEdited = true } if flags.Changed("body") { - opts.EditableOptions.BodyEdited = true + opts.Editable.BodyEdited = true } if flags.Changed("assignee") { - opts.EditableOptions.AssigneesEdited = true + opts.Editable.AssigneesEdited = true } if flags.Changed("label") { - opts.EditableOptions.LabelsEdited = true + opts.Editable.LabelsEdited = true } if flags.Changed("project") { - opts.EditableOptions.ProjectsEdited = true + opts.Editable.ProjectsEdited = true } if flags.Changed("milestone") { - opts.EditableOptions.MilestoneEdited = true + opts.Editable.MilestoneEdited = true } - if !opts.EditableOptions.Dirty() { + if !opts.Editable.Dirty() { opts.Interactive = true } @@ -96,12 +96,12 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman }, } - cmd.Flags().StringVarP(&opts.EditableOptions.Title, "title", "t", "", "Revise the issue title.") - cmd.Flags().StringVarP(&opts.EditableOptions.Body, "body", "b", "", "Revise the issue body.") - cmd.Flags().StringSliceVarP(&opts.EditableOptions.Assignees, "assignee", "a", nil, "Set assigned people by their `login`. Use \"@me\" to self-assign.") - cmd.Flags().StringSliceVarP(&opts.EditableOptions.Labels, "label", "l", nil, "Set the issue labels by `name`") - cmd.Flags().StringSliceVarP(&opts.EditableOptions.Projects, "project", "p", nil, "Set the projects the issue belongs to by `name`") - cmd.Flags().StringVarP(&opts.EditableOptions.Milestone, "milestone", "m", "", "Set the milestone the issue belongs to by `name`") + cmd.Flags().StringVarP(&opts.Editable.Title, "title", "t", "", "Revise the issue title.") + cmd.Flags().StringVarP(&opts.Editable.Body, "body", "b", "", "Revise the issue body.") + cmd.Flags().StringSliceVarP(&opts.Editable.Assignees, "assignee", "a", nil, "Set assigned people by their `login`. Use \"@me\" to self-assign.") + cmd.Flags().StringSliceVarP(&opts.Editable.Labels, "label", "l", nil, "Set the issue labels by `name`") + cmd.Flags().StringSliceVarP(&opts.Editable.Projects, "project", "p", nil, "Set the projects the issue belongs to by `name`") + cmd.Flags().StringVarP(&opts.Editable.Milestone, "milestone", "m", "", "Set the milestone the issue belongs to by `name`") return cmd } @@ -118,23 +118,23 @@ func editRun(opts *EditOptions) error { return err } - editOptions := opts.EditableOptions - editOptions.TitleDefault = issue.Title - editOptions.BodyDefault = issue.Body - editOptions.AssigneesDefault = issue.Assignees - editOptions.LabelsDefault = issue.Labels - editOptions.ProjectsDefault = issue.ProjectCards - editOptions.MilestoneDefault = issue.Milestone + editable := opts.Editable + editable.TitleDefault = issue.Title + editable.BodyDefault = issue.Body + editable.AssigneesDefault = issue.Assignees + editable.LabelsDefault = issue.Labels + editable.ProjectsDefault = issue.ProjectCards + editable.MilestoneDefault = issue.Milestone if opts.Interactive { - err = opts.FieldsToEditSurvey(&editOptions) + err = opts.FieldsToEditSurvey(&editable) if err != nil { return err } } opts.IO.StartProgressIndicator() - err = opts.FetchOptions(apiClient, repo, &editOptions) + err = opts.FetchOptions(apiClient, repo, &editable) opts.IO.StopProgressIndicator() if err != nil { return err @@ -145,14 +145,14 @@ func editRun(opts *EditOptions) error { if err != nil { return err } - err = opts.EditableSurvey(editorCommand, &editOptions) + err = opts.EditFieldsSurvey(&editable, editorCommand) if err != nil { return err } } opts.IO.StartProgressIndicator() - err = updateIssue(apiClient, repo, issue.ID, editOptions) + err = updateIssue(apiClient, repo, issue.ID, editable) opts.IO.StopProgressIndicator() if err != nil { return err @@ -163,61 +163,28 @@ func editRun(opts *EditOptions) error { return nil } -func updateIssue(client *api.Client, repo ghrepo.Interface, id string, options prShared.EditableOptions) error { - params := githubv4.UpdateIssueInput{ID: id} - if options.TitleEdited { - title := githubv4.String(options.Title) - params.Title = &title +func updateIssue(client *api.Client, repo ghrepo.Interface, id string, options prShared.Editable) error { + var err error + params := githubv4.UpdateIssueInput{ + ID: id, + Title: options.TitleParam(), + Body: options.BodyParam(), } - if options.BodyEdited { - body := githubv4.String(options.Body) - params.Body = &body + params.AssigneeIDs, err = options.AssigneesParam(client, repo) + if err != nil { + return err } - if options.AssigneesEdited { - meReplacer := prShared.NewMeReplacer(client, repo.RepoHost()) - assignees, err := meReplacer.ReplaceSlice(options.Assignees) - if err != nil { - return err - } - ids, err := options.Metadata.MembersToIDs(assignees) - if err != nil { - return err - } - assigneeIDs := make([]githubv4.ID, len(ids)) - for i, v := range ids { - assigneeIDs[i] = v - } - params.AssigneeIDs = &assigneeIDs + params.LabelIDs, err = options.LabelsParam() + if err != nil { + return err } - if options.LabelsEdited { - ids, err := options.Metadata.LabelsToIDs(options.Labels) - if err != nil { - return err - } - labelIDs := make([]githubv4.ID, len(ids)) - for i, v := range ids { - labelIDs[i] = v - } - params.LabelIDs = &labelIDs + params.ProjectIDs, err = options.ProjectsParam() + if err != nil { + return err } - if options.ProjectsEdited { - ids, err := options.Metadata.ProjectsToIDs(options.Projects) - if err != nil { - return err - } - projectIDs := make([]githubv4.ID, len(ids)) - for i, v := range ids { - projectIDs[i] = v - } - params.ProjectIDs = &projectIDs - } - if options.MilestoneEdited { - id, err := options.Metadata.MilestoneToID(options.Milestone) - if err != nil { - return err - } - milestoneID := githubv4.ID(id) - params.MilestoneID = &milestoneID + params.MilestoneID, err = options.MilestoneParam() + if err != nil { + return err } return api.IssueUpdate(client, repo, params) } diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index a5d090c51..b132db26f 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -41,7 +41,7 @@ func TestNewCmdEdit(t *testing.T) { input: "23 --title test", output: EditOptions{ SelectorArg: "23", - EditableOptions: prShared.EditableOptions{ + Editable: prShared.Editable{ Title: "test", TitleEdited: true, }, @@ -53,7 +53,7 @@ func TestNewCmdEdit(t *testing.T) { input: "23 --body test", output: EditOptions{ SelectorArg: "23", - EditableOptions: prShared.EditableOptions{ + Editable: prShared.Editable{ Body: "test", BodyEdited: true, }, @@ -65,7 +65,7 @@ func TestNewCmdEdit(t *testing.T) { input: "23 --assignee monalisa,hubot", output: EditOptions{ SelectorArg: "23", - EditableOptions: prShared.EditableOptions{ + Editable: prShared.Editable{ Assignees: []string{"monalisa", "hubot"}, AssigneesEdited: true, }, @@ -77,7 +77,7 @@ func TestNewCmdEdit(t *testing.T) { input: "23 --label feature,TODO,bug", output: EditOptions{ SelectorArg: "23", - EditableOptions: prShared.EditableOptions{ + Editable: prShared.Editable{ Labels: []string{"feature", "TODO", "bug"}, LabelsEdited: true, }, @@ -89,7 +89,7 @@ func TestNewCmdEdit(t *testing.T) { input: "23 --project Cleanup,Roadmap", output: EditOptions{ SelectorArg: "23", - EditableOptions: prShared.EditableOptions{ + Editable: prShared.Editable{ Projects: []string{"Cleanup", "Roadmap"}, ProjectsEdited: true, }, @@ -101,7 +101,7 @@ func TestNewCmdEdit(t *testing.T) { input: "23 --milestone GA", output: EditOptions{ SelectorArg: "23", - EditableOptions: prShared.EditableOptions{ + Editable: prShared.Editable{ Milestone: "GA", MilestoneEdited: true, }, @@ -144,7 +144,7 @@ func TestNewCmdEdit(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg) assert.Equal(t, tt.output.Interactive, gotOpts.Interactive) - assert.Equal(t, tt.output.EditableOptions, gotOpts.EditableOptions) + assert.Equal(t, tt.output.Editable, gotOpts.Editable) }) } } @@ -162,7 +162,7 @@ func Test_editRun(t *testing.T) { input: &EditOptions{ SelectorArg: "123", Interactive: false, - EditableOptions: prShared.EditableOptions{ + Editable: prShared.Editable{ Title: "new title", TitleEdited: true, Body: "new body", @@ -190,7 +190,7 @@ func Test_editRun(t *testing.T) { input: &EditOptions{ SelectorArg: "123", Interactive: true, - FieldsToEditSurvey: func(eo *prShared.EditableOptions) error { + FieldsToEditSurvey: func(eo *prShared.Editable) error { eo.TitleEdited = true eo.BodyEdited = true eo.AssigneesEdited = true @@ -199,7 +199,7 @@ func Test_editRun(t *testing.T) { eo.MilestoneEdited = true return nil }, - EditableSurvey: func(_ string, eo *prShared.EditableOptions) error { + EditFieldsSurvey: func(eo *prShared.Editable, _ string) error { eo.Title = "new title" eo.Body = "new body" eo.Assignees = []string{"monalisa", "hubot"} diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go new file mode 100644 index 000000000..0bd5ce826 --- /dev/null +++ b/pkg/cmd/pr/edit/edit.go @@ -0,0 +1,259 @@ +package edit + +import ( + "errors" + "fmt" + "net/http" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + shared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/shurcooL/githubv4" + "github.com/spf13/cobra" +) + +type EditOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Remotes func() (context.Remotes, error) + Branch func() (string, error) + + Surveyor Surveyor + Fetcher EditableOptionsFetcher + EditorRetriever EditorRetriever + + SelectorArg string + Interactive bool + + shared.Editable +} + +func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { + opts := &EditOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Remotes: f.Remotes, + Branch: f.Branch, + Surveyor: surveyor{}, + Fetcher: fetcher{}, + EditorRetriever: editorRetriever{config: f.Config}, + } + + cmd := &cobra.Command{ + Use: "edit { | }", + Short: "Edit a pull request", + Example: heredoc.Doc(` + $ gh pr edit 23 --title "I found a bug" --body "Nothing works" + $ gh pr edit 23 --label "bug,help wanted" + $ gh pr edit 23 --label bug --label "help wanted" + $ gh pr edit 23 --reviewer monalisa,hubot --reviewer myorg/team-name + $ gh pr edit 23 --assignee monalisa,hubot + $ gh pr edit 23 --assignee @me + $ gh pr edit 23 --project "Roadmap" + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + opts.SelectorArg = args[0] + + flags := cmd.Flags() + if flags.Changed("title") { + opts.Editable.TitleEdited = true + } + if flags.Changed("body") { + opts.Editable.BodyEdited = true + } + if flags.Changed("reviewer") { + opts.Editable.ReviewersEdited = true + } + if flags.Changed("assignee") { + opts.Editable.AssigneesEdited = true + } + if flags.Changed("label") { + opts.Editable.LabelsEdited = true + } + if flags.Changed("project") { + opts.Editable.ProjectsEdited = true + } + if flags.Changed("milestone") { + opts.Editable.MilestoneEdited = true + } + + if !opts.Editable.Dirty() { + opts.Interactive = true + } + + if opts.Interactive && !opts.IO.CanPrompt() { + return &cmdutil.FlagError{Err: errors.New("--tile, --body, --reviewer, --assignee, --label, --project, or --milestone required when not running interactively")} + } + + if runF != nil { + return runF(opts) + } + + return editRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Editable.Title, "title", "t", "", "Revise the pr title.") + cmd.Flags().StringVarP(&opts.Editable.Body, "body", "b", "", "Revise the pr body.") + cmd.Flags().StringSliceVarP(&opts.Editable.Reviewers, "reviewer", "r", nil, "Request reviews from people or teams by their `handle`") + cmd.Flags().StringSliceVarP(&opts.Editable.Assignees, "assignee", "a", nil, "Set assigned people by their `login`. Use \"@me\" to self-assign.") + cmd.Flags().StringSliceVarP(&opts.Editable.Labels, "label", "l", nil, "Set the pr labels by `name`") + cmd.Flags().StringSliceVarP(&opts.Editable.Projects, "project", "p", nil, "Set the projects the pr belongs to by `name`") + cmd.Flags().StringVarP(&opts.Editable.Milestone, "milestone", "m", "", "Set the milestone the pr belongs to by `name`") + + return cmd +} + +func editRun(opts *EditOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + pr, repo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + if err != nil { + return err + } + + editable := opts.Editable + editable.ReviewersAllowed = true + editable.TitleDefault = pr.Title + editable.BodyDefault = pr.Body + editable.ReviewersDefault = pr.ReviewRequests + editable.AssigneesDefault = pr.Assignees + editable.LabelsDefault = pr.Labels + editable.ProjectsDefault = pr.ProjectCards + editable.MilestoneDefault = pr.Milestone + + if opts.Interactive { + err = opts.Surveyor.FieldsToEdit(&editable) + if err != nil { + return err + } + } + + opts.IO.StartProgressIndicator() + err = opts.Fetcher.EditableOptionsFetch(apiClient, repo, &editable) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + + if opts.Interactive { + editorCommand, err := opts.EditorRetriever.Retrieve() + if err != nil { + return err + } + err = opts.Surveyor.EditFields(&editable, editorCommand) + if err != nil { + return err + } + } + + opts.IO.StartProgressIndicator() + err = updatePullRequest(apiClient, repo, pr.ID, editable) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + + fmt.Fprintln(opts.IO.Out, pr.URL) + + return nil +} + +func updatePullRequest(client *api.Client, repo ghrepo.Interface, id string, editable shared.Editable) error { + var err error + params := githubv4.UpdatePullRequestInput{ + PullRequestID: id, + Title: editable.TitleParam(), + Body: editable.BodyParam(), + } + params.AssigneeIDs, err = editable.AssigneesParam(client, repo) + if err != nil { + return err + } + params.LabelIDs, err = editable.LabelsParam() + if err != nil { + return err + } + params.ProjectIDs, err = editable.ProjectsParam() + if err != nil { + return err + } + params.MilestoneID, err = editable.MilestoneParam() + if err != nil { + return err + } + err = api.UpdatePullRequest(client, repo, params) + if err != nil { + return err + } + return updatePullRequestReviews(client, repo, id, editable) +} + +func updatePullRequestReviews(client *api.Client, repo ghrepo.Interface, id string, editable shared.Editable) error { + if !editable.ReviewersEdited { + return nil + } + userIds, teamIds, err := editable.ReviewersParams() + if err != nil { + return err + } + union := githubv4.Boolean(false) + reviewsRequestParams := githubv4.RequestReviewsInput{ + PullRequestID: id, + Union: &union, + UserIDs: userIds, + TeamIDs: teamIds, + } + return api.UpdatePullRequestReviews(client, repo, reviewsRequestParams) +} + +type Surveyor interface { + FieldsToEdit(*shared.Editable) error + EditFields(*shared.Editable, string) error +} + +type surveyor struct{} + +func (s surveyor) FieldsToEdit(editable *shared.Editable) error { + return shared.FieldsToEditSurvey(editable) +} + +func (s surveyor) EditFields(editable *shared.Editable, editorCmd string) error { + return shared.EditFieldsSurvey(editable, editorCmd) +} + +type EditableOptionsFetcher interface { + EditableOptionsFetch(*api.Client, ghrepo.Interface, *shared.Editable) error +} + +type fetcher struct{} + +func (f fetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable) error { + return shared.FetchOptions(client, repo, opts) +} + +type EditorRetriever interface { + Retrieve() (string, error) +} + +type editorRetriever struct { + config func() (config.Config, error) +} + +func (e editorRetriever) Retrieve() (string, error) { + return cmdutil.DetermineEditor(e.config) +} diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go new file mode 100644 index 000000000..1351ff29a --- /dev/null +++ b/pkg/cmd/pr/edit/edit_test.go @@ -0,0 +1,435 @@ +package edit + +import ( + "bytes" + "net/http" + "testing" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghrepo" + shared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdEdit(t *testing.T) { + tests := []struct { + name string + input string + output EditOptions + wantsErr bool + }{ + { + name: "no argument", + input: "", + output: EditOptions{}, + wantsErr: true, + }, + { + name: "issue number argument", + input: "23", + output: EditOptions{ + SelectorArg: "23", + Interactive: true, + }, + wantsErr: false, + }, + { + name: "title flag", + input: "23 --title test", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Title: "test", + TitleEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "body flag", + input: "23 --body test", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Body: "test", + BodyEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "reviewer flag", + input: "23 --reviewer owner/team,monalisa", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Reviewers: []string{"owner/team", "monalisa"}, + ReviewersEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "assignee flag", + input: "23 --assignee monalisa,hubot", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Assignees: []string{"monalisa", "hubot"}, + AssigneesEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "label flag", + input: "23 --label feature,TODO,bug", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Labels: []string{"feature", "TODO", "bug"}, + LabelsEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "project flag", + input: "23 --project Cleanup,Roadmap", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Projects: []string{"Cleanup", "Roadmap"}, + ProjectsEdited: true, + }, + }, + wantsErr: false, + }, + { + name: "milestone flag", + input: "23 --milestone GA", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Milestone: "GA", + MilestoneEdited: true, + }, + }, + wantsErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(true) + io.SetStdinTTY(true) + io.SetStderrTTY(true) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + + var gotOpts *EditOptions + cmd := NewCmdEdit(f, func(opts *EditOptions) error { + gotOpts = opts + return nil + }) + cmd.Flags().BoolP("help", "x", false, "") + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg) + assert.Equal(t, tt.output.Interactive, gotOpts.Interactive) + assert.Equal(t, tt.output.Editable, gotOpts.Editable) + }) + } +} + +func Test_editRun(t *testing.T) { + tests := []struct { + name string + input *EditOptions + httpStubs func(*testing.T, *httpmock.Registry) + stdout string + stderr string + }{ + { + name: "non-interactive", + input: &EditOptions{ + SelectorArg: "123", + Interactive: false, + Editable: shared.Editable{ + Title: "new title", + TitleEdited: true, + Body: "new body", + BodyEdited: true, + Reviewers: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot"}, + ReviewersEdited: true, + Assignees: []string{"monalisa", "hubot"}, + AssigneesEdited: true, + Labels: []string{"feature", "TODO", "bug"}, + LabelsEdited: true, + Projects: []string{"Cleanup", "Roadmap"}, + ProjectsEdited: true, + Milestone: "GA", + MilestoneEdited: true, + }, + Fetcher: testFetcher{}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockPullRequestGet(t, reg) + mockRepoMetadata(t, reg, false) + mockPullRequestUpdate(t, reg) + mockPullRequestReviewersUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123\n", + }, + { + name: "non-interactive skip reviewers", + input: &EditOptions{ + SelectorArg: "123", + Interactive: false, + Editable: shared.Editable{ + Title: "new title", + TitleEdited: true, + Body: "new body", + BodyEdited: true, + Assignees: []string{"monalisa", "hubot"}, + AssigneesEdited: true, + Labels: []string{"feature", "TODO", "bug"}, + LabelsEdited: true, + Projects: []string{"Cleanup", "Roadmap"}, + ProjectsEdited: true, + Milestone: "GA", + MilestoneEdited: true, + }, + Fetcher: testFetcher{}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockPullRequestGet(t, reg) + mockRepoMetadata(t, reg, true) + mockPullRequestUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123\n", + }, + { + name: "interactive", + input: &EditOptions{ + SelectorArg: "123", + Interactive: true, + Surveyor: testSurveyor{}, + Fetcher: testFetcher{}, + EditorRetriever: testEditorRetriever{}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockPullRequestGet(t, reg) + mockRepoMetadata(t, reg, false) + mockPullRequestUpdate(t, reg) + mockPullRequestReviewersUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123\n", + }, + { + name: "interactive skip reviewers", + input: &EditOptions{ + SelectorArg: "123", + Interactive: true, + Surveyor: testSurveyor{skipReviewers: true}, + Fetcher: testFetcher{}, + EditorRetriever: testEditorRetriever{}, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockPullRequestGet(t, reg) + mockRepoMetadata(t, reg, true) + mockPullRequestUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123\n", + }, + } + for _, tt := range tests { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(true) + io.SetStdinTTY(true) + io.SetStderrTTY(true) + + reg := &httpmock.Registry{} + defer reg.Verify(t) + tt.httpStubs(t, reg) + + httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } + baseRepo := func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } + + tt.input.IO = io + tt.input.HttpClient = httpClient + tt.input.BaseRepo = baseRepo + + t.Run(tt.name, func(t *testing.T) { + err := editRun(tt.input) + assert.NoError(t, err) + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + }) + } +} + +func mockPullRequestGet(_ *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "456", + "number": 123, + "url": "https://github.com/OWNER/REPO/pull/123" + } } } }`), + ) +} + +func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry, skipReviewers bool) { + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), + httpmock.StringResponse(` + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "hubot", "id": "HUBOTID" }, + { "login": "MonaLisa", "id": "MONAID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryLabelList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "feature", "id": "FEATUREID" }, + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryMilestoneList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "milestones": { + "nodes": [ + { "title": "GA", "id": "GAID" }, + { "title": "Big One.oh", "id": "BIGONEID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryProjectList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "projects": { + "nodes": [ + { "name": "Cleanup", "id": "CLEANUPID" }, + { "name": "Roadmap", "id": "ROADMAPID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query OrganizationProjectList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "projects": { + "nodes": [ + { "name": "Triage", "id": "TRIAGEID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + if !skipReviewers { + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "external", "id": "EXTERNALID" }, + { "slug": "core", "id": "COREID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + } +} + +func mockPullRequestUpdate(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation PullRequestUpdate\b`), + httpmock.GraphQLMutation(` + { "data": { "updatePullRequest": { "pullRequest": { + "id": "456" + } } } }`, + func(inputs map[string]interface{}) {}), + ) +} + +func mockPullRequestReviewersUpdate(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation PullRequestUpdateRequestReviews\b`), + httpmock.GraphQLMutation(` + { "data": { "requestReviews": { "pullRequest": { + "id": "456" + } } } }`, + func(inputs map[string]interface{}) {}), + ) +} + +type testFetcher struct{} +type testSurveyor struct { + skipReviewers bool +} +type testEditorRetriever struct{} + +func (f testFetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable) error { + return shared.FetchOptions(client, repo, opts) +} + +func (s testSurveyor) FieldsToEdit(e *shared.Editable) error { + e.TitleEdited = true + e.BodyEdited = true + if !s.skipReviewers { + e.ReviewersEdited = true + } + e.AssigneesEdited = true + e.LabelsEdited = true + e.ProjectsEdited = true + e.MilestoneEdited = true + return nil +} + +func (s testSurveyor) EditFields(e *shared.Editable, _ string) error { + e.Title = "new title" + e.Body = "new body" + if !s.skipReviewers { + e.Reviewers = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} + } + e.Assignees = []string{"monalisa", "hubot"} + e.Labels = []string{"feature", "TODO", "bug"} + e.Projects = []string{"Cleanup", "Roadmap"} + e.Milestone = "GA" + return nil +} + +func (t testEditorRetriever) Retrieve() (string, error) { + return "vim", nil +} diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index f1981fabe..3be067636 100644 --- a/pkg/cmd/pr/pr.go +++ b/pkg/cmd/pr/pr.go @@ -8,6 +8,7 @@ import ( cmdComment "github.com/cli/cli/pkg/cmd/pr/comment" cmdCreate "github.com/cli/cli/pkg/cmd/pr/create" cmdDiff "github.com/cli/cli/pkg/cmd/pr/diff" + cmdEdit "github.com/cli/cli/pkg/cmd/pr/edit" cmdList "github.com/cli/cli/pkg/cmd/pr/list" cmdMerge "github.com/cli/cli/pkg/cmd/pr/merge" cmdReady "github.com/cli/cli/pkg/cmd/pr/ready" @@ -55,6 +56,7 @@ func NewCmdPR(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdChecks.NewCmdChecks(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) + cmd.AddCommand(cmdEdit.NewCmdEdit(f, nil)) return cmd } diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 36bd0387c..885d1459d 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -2,14 +2,16 @@ package shared import ( "fmt" + "strings" "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/surveyext" + "github.com/shurcooL/githubv4" ) -type EditableOptions struct { +type Editable struct { Title string TitleDefault string TitleEdited bool @@ -47,7 +49,7 @@ type EditableOptions struct { Metadata api.RepoMetadataResult } -func (e EditableOptions) Dirty() bool { +func (e Editable) Dirty() bool { return e.TitleEdited || e.BodyEdited || e.ReviewersEdited || @@ -57,48 +59,125 @@ func (e EditableOptions) Dirty() bool { e.MilestoneEdited } -func EditableSurvey(editorCommand string, options *EditableOptions) error { - if options.TitleEdited { - title, err := titleSurvey(options.TitleDefault) - if err != nil { - return err - } - options.Title = title +func (e Editable) TitleParam() *githubv4.String { + if !e.TitleEdited { + return nil } - if options.BodyEdited { - body, err := bodySurvey(options.BodyDefault, editorCommand) - if err != nil { - return err - } - options.Body = body + s := githubv4.String(e.Title) + return &s +} + +func (e Editable) BodyParam() *githubv4.String { + if !e.BodyEdited { + return nil } - if options.AssigneesEdited { - assignees, err := assigneesSurvey(options.AssigneesDefault, options.AssigneesOptions) - if err != nil { - return err - } - options.Assignees = assignees + s := githubv4.String(e.Body) + return &s +} + +func (e Editable) ReviewersParams() (*[]githubv4.ID, *[]githubv4.ID, error) { + if !e.ReviewersEdited { + return nil, nil, nil } - if options.LabelsEdited { - labels, err := labelsSurvey(options.LabelsDefault, options.LabelsOptions) - if err != nil { - return err + var userReviewers []string + var teamReviewers []string + for _, r := range e.Reviewers { + if strings.ContainsRune(r, '/') { + teamReviewers = append(teamReviewers, r) + } else { + userReviewers = append(userReviewers, r) } - options.Labels = labels } - if options.ProjectsEdited { - projects, err := projectsSurvey(options.ProjectsDefault, options.ProjectsOptions) - if err != nil { - return err - } - options.Projects = projects + userIds, err := toParams(userReviewers, e.Metadata.MembersToIDs) + if err != nil { + return nil, nil, err } - if options.MilestoneEdited { - milestone, err := milestoneSurvey(options.MilestoneDefault, options.MilestoneOptions) + teamIds, err := toParams(teamReviewers, e.Metadata.TeamsToIDs) + if err != nil { + return nil, nil, err + } + return userIds, teamIds, nil +} + +func (e Editable) AssigneesParam(client *api.Client, repo ghrepo.Interface) (*[]githubv4.ID, error) { + if !e.AssigneesEdited { + return nil, nil + } + meReplacer := NewMeReplacer(client, repo.RepoHost()) + assignees, err := meReplacer.ReplaceSlice(e.Assignees) + if err != nil { + return nil, err + } + return toParams(assignees, e.Metadata.MembersToIDs) +} + +func (e Editable) LabelsParam() (*[]githubv4.ID, error) { + if !e.LabelsEdited { + return nil, nil + } + return toParams(e.Labels, e.Metadata.LabelsToIDs) +} + +func (e Editable) ProjectsParam() (*[]githubv4.ID, error) { + if !e.ProjectsEdited { + return nil, nil + } + return toParams(e.Projects, e.Metadata.ProjectsToIDs) +} + +func (e Editable) MilestoneParam() (*githubv4.ID, error) { + if !e.MilestoneEdited { + return nil, nil + } + if e.Milestone == noMilestone || e.Milestone == "" { + return githubv4.NewID(nil), nil + } + return toParam(e.Milestone, e.Metadata.MilestoneToID) +} + +func EditFieldsSurvey(editable *Editable, editorCommand string) error { + var err error + if editable.TitleEdited { + editable.Title, err = titleSurvey(editable.TitleDefault) + if err != nil { + return err + } + } + if editable.BodyEdited { + editable.Body, err = bodySurvey(editable.BodyDefault, editorCommand) + if err != nil { + return err + } + } + if editable.ReviewersEdited { + editable.Reviewers, err = reviewersSurvey(editable.ReviewersDefault, editable.ReviewersOptions) + if err != nil { + return err + } + } + if editable.AssigneesEdited { + editable.Assignees, err = assigneesSurvey(editable.AssigneesDefault, editable.AssigneesOptions) + if err != nil { + return err + } + } + if editable.LabelsEdited { + editable.Labels, err = labelsSurvey(editable.LabelsDefault, editable.LabelsOptions) + if err != nil { + return err + } + } + if editable.ProjectsEdited { + editable.Projects, err = projectsSurvey(editable.ProjectsDefault, editable.ProjectsOptions) + if err != nil { + return err + } + } + if editable.MilestoneEdited { + editable.Milestone, err = milestoneSurvey(editable.MilestoneDefault, editable.MilestoneOptions) if err != nil { return err } - options.Milestone = milestone } confirm, err := confirmSurvey() if err != nil { @@ -111,7 +190,7 @@ func EditableSurvey(editorCommand string, options *EditableOptions) error { return nil } -func FieldsToEditSurvey(options *EditableOptions) error { +func FieldsToEditSurvey(editable *Editable) error { contains := func(s []string, str string) bool { for _, v := range s { if v == str { @@ -123,7 +202,7 @@ func FieldsToEditSurvey(options *EditableOptions) error { results := []string{} opts := []string{"Title", "Body"} - if options.ReviewersAllowed { + if editable.ReviewersAllowed { opts = append(opts, "Reviewers") } opts = append(opts, "Assignees", "Labels", "Projects", "Milestone") @@ -137,37 +216,37 @@ func FieldsToEditSurvey(options *EditableOptions) error { } if contains(results, "Title") { - options.TitleEdited = true + editable.TitleEdited = true } if contains(results, "Body") { - options.BodyEdited = true + editable.BodyEdited = true } if contains(results, "Reviewers") { - options.ReviewersEdited = true + editable.ReviewersEdited = true } if contains(results, "Assignees") { - options.AssigneesEdited = true + editable.AssigneesEdited = true } if contains(results, "Labels") { - options.LabelsEdited = true + editable.LabelsEdited = true } if contains(results, "Projects") { - options.ProjectsEdited = true + editable.ProjectsEdited = true } if contains(results, "Milestone") { - options.MilestoneEdited = true + editable.MilestoneEdited = true } return nil } -func FetchOptions(client *api.Client, repo ghrepo.Interface, options *EditableOptions) error { +func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) error { input := api.RepoMetadataInput{ - Reviewers: options.ReviewersEdited, - Assignees: options.AssigneesEdited, - Labels: options.LabelsEdited, - Projects: options.ProjectsEdited, - Milestones: options.MilestoneEdited, + Reviewers: editable.ReviewersEdited, + Assignees: editable.AssigneesEdited, + Labels: editable.LabelsEdited, + Projects: editable.ProjectsEdited, + Milestones: editable.MilestoneEdited, } metadata, err := api.RepoMetadata(client, repo, input) if err != nil { @@ -195,12 +274,12 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, options *EditableOp milestones = append(milestones, m.Title) } - options.Metadata = *metadata - options.ReviewersOptions = append(users, teams...) - options.AssigneesOptions = users - options.LabelsOptions = labels - options.ProjectsOptions = projects - options.MilestoneOptions = milestones + editable.Metadata = *metadata + editable.ReviewersOptions = append(users, teams...) + editable.AssigneesOptions = users + editable.LabelsOptions = labels + editable.ProjectsOptions = projects + editable.MilestoneOptions = milestones return nil } @@ -231,8 +310,26 @@ func bodySurvey(body, editorCommand string) (string, error) { return result, err } -func assigneesSurvey(assignees api.Assignees, assigneesOpts []string) ([]string, error) { - if len(assigneesOpts) == 0 { +func reviewersSurvey(reviewers api.ReviewRequests, opts []string) ([]string, error) { + if len(opts) == 0 { + return nil, nil + } + logins := []string{} + for _, a := range reviewers.Nodes { + logins = append(logins, a.RequestedReviewer.Login) + } + var results []string + q := &survey.MultiSelect{ + Message: "Reviewers", + Options: opts, + Default: logins, + } + err := survey.AskOne(q, &results) + return results, err +} + +func assigneesSurvey(assignees api.Assignees, opts []string) ([]string, error) { + if len(opts) == 0 { return nil, nil } logins := []string{} @@ -242,15 +339,15 @@ func assigneesSurvey(assignees api.Assignees, assigneesOpts []string) ([]string, var results []string q := &survey.MultiSelect{ Message: "Assignees", - Options: assigneesOpts, + Options: opts, Default: logins, } err := survey.AskOne(q, &results) return results, err } -func labelsSurvey(labels api.Labels, labelOpts []string) ([]string, error) { - if len(labelOpts) == 0 { +func labelsSurvey(labels api.Labels, opts []string) ([]string, error) { + if len(opts) == 0 { return nil, nil } names := []string{} @@ -260,15 +357,15 @@ func labelsSurvey(labels api.Labels, labelOpts []string) ([]string, error) { var results []string q := &survey.MultiSelect{ Message: "Labels", - Options: labelOpts, + Options: opts, Default: names, } err := survey.AskOne(q, &results) return results, err } -func projectsSurvey(projectCards api.ProjectCards, projectsOpts []string) ([]string, error) { - if len(projectsOpts) == 0 { +func projectsSurvey(projectCards api.ProjectCards, opts []string) ([]string, error) { + if len(opts) == 0 { return nil, nil } names := []string{} @@ -278,21 +375,21 @@ func projectsSurvey(projectCards api.ProjectCards, projectsOpts []string) ([]str var results []string q := &survey.MultiSelect{ Message: "Projects", - Options: projectsOpts, + Options: opts, Default: names, } err := survey.AskOne(q, &results) return results, err } -func milestoneSurvey(milestone api.Milestone, milestoneOpts []string) (string, error) { - if len(milestoneOpts) == 0 { +func milestoneSurvey(milestone api.Milestone, opts []string) (string, error) { + if len(opts) == 0 { return "", nil } var result string q := &survey.Select{ Message: "Milestone", - Options: milestoneOpts, + Options: opts, Default: milestone.Title, } err := survey.AskOne(q, &result) @@ -308,3 +405,24 @@ func confirmSurvey() (bool, error) { err := survey.AskOne(q, &result) return result, err } + +func toParams(s []string, mapper func([]string) ([]string, error)) (*[]githubv4.ID, error) { + ids, err := mapper(s) + if err != nil { + return nil, err + } + gIds := make([]githubv4.ID, len(ids)) + for i, v := range ids { + gIds[i] = v + } + return &gIds, nil +} + +func toParam(s string, mapper func(string) (string, error)) (*githubv4.ID, error) { + id, err := mapper(s) + if err != nil { + return nil, err + } + gId := githubv4.ID(id) + return &gId, nil +} From 4fdf28d8a43c39013ce974d65f079240dce3796d Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 10 Feb 2021 15:13:59 -0800 Subject: [PATCH 21/22] Change behavior of slice flags for issue edit and pr edit commands --- api/queries_issue.go | 24 +++ api/queries_pr.go | 39 ++-- api/queries_repo.go | 3 +- pkg/cmd/issue/edit/edit.go | 103 ++++++---- pkg/cmd/issue/edit/edit_test.go | 153 ++++++++++---- pkg/cmd/pr/edit/edit.go | 122 ++++++++---- pkg/cmd/pr/edit/edit_test.go | 229 +++++++++++++++------ pkg/cmd/pr/shared/editable.go | 339 +++++++++++++------------------- pkg/set/string_set.go | 46 +++++ 9 files changed, 651 insertions(+), 407 deletions(-) create mode 100644 pkg/set/string_set.go diff --git a/api/queries_issue.go b/api/queries_issue.go index 11c169e0e..bd3f15555 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -49,6 +49,14 @@ type Assignees struct { TotalCount int } +func (a Assignees) Logins() []string { + logins := make([]string, len(a.Nodes)) + for i, a := range a.Nodes { + logins[i] = a.Login + } + return logins +} + type Labels struct { Nodes []struct { Name string @@ -56,6 +64,14 @@ type Labels struct { TotalCount int } +func (l Labels) Names() []string { + names := make([]string, len(l.Nodes)) + for i, l := range l.Nodes { + names[i] = l.Name + } + return names +} + type ProjectCards struct { Nodes []struct { Project struct { @@ -68,6 +84,14 @@ type ProjectCards struct { TotalCount int } +func (p ProjectCards) ProjectNames() []string { + names := make([]string, len(p.Nodes)) + for i, c := range p.Nodes { + names[i] = c.Project.Name + } + return names +} + type Milestone struct { Title string } diff --git a/api/queries_pr.go b/api/queries_pr.go index e91216479..4c35a153a 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -80,32 +80,10 @@ type PullRequest struct { } } } - Assignees struct { - Nodes []struct { - Login string - } - TotalCount int - } - Labels struct { - Nodes []struct { - Name string - } - TotalCount int - } - ProjectCards struct { - Nodes []struct { - Project struct { - Name string - } - Column struct { - Name string - } - } - TotalCount int - } - Milestone struct { - Title string - } + Assignees Assignees + Labels Labels + ProjectCards ProjectCards + Milestone Milestone Comments Comments ReactionGroups ReactionGroups Reviews PullRequestReviews @@ -123,6 +101,14 @@ type ReviewRequests struct { TotalCount int } +func (r ReviewRequests) Logins() []string { + logins := make([]string, len(r.Nodes)) + for i, a := range r.Nodes { + logins[i] = a.RequestedReviewer.Login + } + return logins +} + type NotFoundError struct { error } @@ -816,6 +802,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter reviewParams["teamIds"] = ids } + //TODO: How much work to extract this into own method and use for create and edit? if len(reviewParams) > 0 { reviewQuery := ` mutation PullRequestCreateRequestReviews($input: RequestReviewsInput!) { diff --git a/api/queries_repo.go b/api/queries_repo.go index 852664b7c..90d3949a5 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "net/http" "sort" @@ -497,7 +496,7 @@ func (m *RepoMetadataResult) MilestoneToID(title string) (string, error) { return m.ID, nil } } - return "", errors.New("not found") + return "", fmt.Errorf("'%s' not found", title) } func (m *RepoMetadataResult) Merge(m2 *RepoMetadataResult) { diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index c209c8fc7..e83784359 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -47,11 +47,10 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman Short: "Edit an issue", Example: heredoc.Doc(` $ gh issue edit 23 --title "I found a bug" --body "Nothing works" - $ gh issue edit 23 --label "bug,help wanted" - $ gh issue edit 23 --label bug --label "help wanted" - $ gh issue edit 23 --assignee monalisa,hubot - $ gh issue edit 23 --assignee @me - $ gh issue edit 23 --project "Roadmap" + $ gh issue edit 23 --add-label "bug,help wanted" --remove-label "core" + $ gh issue edit 23 --add-assignee @me --remove-assignee monalisa,hubot + $ gh issue edit 23 --add-project "Roadmap" --remove-project v1,v2 + $ gh issue edit 23 --milestone "Version 1" `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -62,22 +61,22 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman flags := cmd.Flags() if flags.Changed("title") { - opts.Editable.TitleEdited = true + opts.Editable.Title.Edited = true } if flags.Changed("body") { - opts.Editable.BodyEdited = true + opts.Editable.Body.Edited = true } - if flags.Changed("assignee") { - opts.Editable.AssigneesEdited = true + if flags.Changed("add-assignee") || flags.Changed("remove-assignee") { + opts.Editable.Assignees.Edited = true } - if flags.Changed("label") { - opts.Editable.LabelsEdited = true + if flags.Changed("add-label") || flags.Changed("remove-label") { + opts.Editable.Labels.Edited = true } - if flags.Changed("project") { - opts.Editable.ProjectsEdited = true + if flags.Changed("add-project") || flags.Changed("remove-project") { + opts.Editable.Projects.Edited = true } if flags.Changed("milestone") { - opts.Editable.MilestoneEdited = true + opts.Editable.Milestone.Edited = true } if !opts.Editable.Dirty() { @@ -85,7 +84,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } if opts.Interactive && !opts.IO.CanPrompt() { - return &cmdutil.FlagError{Err: errors.New("--tile, --body, --assignee, --label, --project, or --milestone required when not running interactively")} + return &cmdutil.FlagError{Err: errors.New("field to edit flag required when not running interactively")} } if runF != nil { @@ -96,12 +95,15 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman }, } - cmd.Flags().StringVarP(&opts.Editable.Title, "title", "t", "", "Revise the issue title.") - cmd.Flags().StringVarP(&opts.Editable.Body, "body", "b", "", "Revise the issue body.") - cmd.Flags().StringSliceVarP(&opts.Editable.Assignees, "assignee", "a", nil, "Set assigned people by their `login`. Use \"@me\" to self-assign.") - cmd.Flags().StringSliceVarP(&opts.Editable.Labels, "label", "l", nil, "Set the issue labels by `name`") - cmd.Flags().StringSliceVarP(&opts.Editable.Projects, "project", "p", nil, "Set the projects the issue belongs to by `name`") - cmd.Flags().StringVarP(&opts.Editable.Milestone, "milestone", "m", "", "Set the milestone the issue belongs to by `name`") + cmd.Flags().StringVarP(&opts.Editable.Title.Value, "title", "t", "", "Set the new title.") + cmd.Flags().StringVarP(&opts.Editable.Body.Value, "body", "b", "", "Set the new body.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself.") + cmd.Flags().StringSliceVar(&opts.Editable.Labels.Add, "add-label", nil, "Add labels by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Labels.Remove, "remove-label", nil, "Remove labels by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the issue to projects by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Remove, "remove-project", nil, "Remove the issue from projects by `name`") + cmd.Flags().StringVarP(&opts.Editable.Milestone.Value, "milestone", "m", "", "Edit the milestone the issue belongs to by `name`") return cmd } @@ -119,12 +121,12 @@ func editRun(opts *EditOptions) error { } editable := opts.Editable - editable.TitleDefault = issue.Title - editable.BodyDefault = issue.Body - editable.AssigneesDefault = issue.Assignees - editable.LabelsDefault = issue.Labels - editable.ProjectsDefault = issue.ProjectCards - editable.MilestoneDefault = issue.Milestone + editable.Title.Default = issue.Title + editable.Body.Default = issue.Body + editable.Assignees.Default = issue.Assignees.Logins() + editable.Labels.Default = issue.Labels.Names() + editable.Projects.Default = issue.ProjectCards.ProjectNames() + editable.Milestone.Default = issue.Milestone.Title if opts.Interactive { err = opts.FieldsToEditSurvey(&editable) @@ -167,24 +169,59 @@ func updateIssue(client *api.Client, repo ghrepo.Interface, id string, options p var err error params := githubv4.UpdateIssueInput{ ID: id, - Title: options.TitleParam(), - Body: options.BodyParam(), + Title: ghString(options.TitleValue()), + Body: ghString(options.BodyValue()), } - params.AssigneeIDs, err = options.AssigneesParam(client, repo) + assigneeIds, err := options.AssigneeIds(client, repo) if err != nil { return err } - params.LabelIDs, err = options.LabelsParam() + params.AssigneeIDs = ghIds(assigneeIds) + labelIds, err := options.LabelIds() if err != nil { return err } - params.ProjectIDs, err = options.ProjectsParam() + params.LabelIDs = ghIds(labelIds) + projectIds, err := options.ProjectIds() if err != nil { return err } - params.MilestoneID, err = options.MilestoneParam() + params.ProjectIDs = ghIds(projectIds) + milestoneId, err := options.MilestoneId() if err != nil { return err } + params.MilestoneID = ghId(milestoneId) return api.IssueUpdate(client, repo, params) } + +func ghIds(s *[]string) *[]githubv4.ID { + if s == nil { + return nil + } + ids := make([]githubv4.ID, len(*s)) + for i, v := range *s { + ids[i] = v + } + return &ids +} + +func ghId(s *string) *githubv4.ID { + if s == nil { + return nil + } + if *s == "" { + r := githubv4.ID(nil) + return &r + } + r := githubv4.ID(*s) + return &r +} + +func ghString(s *string) *githubv4.String { + if s == nil { + return nil + } + r := githubv4.String(*s) + return &r +} diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index b132db26f..d21997516 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -42,8 +42,10 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: prShared.Editable{ - Title: "test", - TitleEdited: true, + Title: prShared.EditableString{ + Value: "test", + Edited: true, + }, }, }, wantsErr: false, @@ -54,44 +56,94 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: prShared.Editable{ - Body: "test", - BodyEdited: true, + Body: prShared.EditableString{ + Value: "test", + Edited: true, + }, }, }, wantsErr: false, }, { - name: "assignee flag", - input: "23 --assignee monalisa,hubot", + name: "add-assignee flag", + input: "23 --add-assignee monalisa,hubot", output: EditOptions{ SelectorArg: "23", Editable: prShared.Editable{ - Assignees: []string{"monalisa", "hubot"}, - AssigneesEdited: true, + Assignees: prShared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Edited: true, + }, }, }, wantsErr: false, }, { - name: "label flag", - input: "23 --label feature,TODO,bug", + name: "remove-assignee flag", + input: "23 --remove-assignee monalisa,hubot", output: EditOptions{ SelectorArg: "23", Editable: prShared.Editable{ - Labels: []string{"feature", "TODO", "bug"}, - LabelsEdited: true, + Assignees: prShared.EditableSlice{ + Remove: []string{"monalisa", "hubot"}, + Edited: true, + }, }, }, wantsErr: false, }, { - name: "project flag", - input: "23 --project Cleanup,Roadmap", + name: "add-label flag", + input: "23 --add-label feature,TODO,bug", output: EditOptions{ SelectorArg: "23", Editable: prShared.Editable{ - Projects: []string{"Cleanup", "Roadmap"}, - ProjectsEdited: true, + Labels: prShared.EditableSlice{ + Add: []string{"feature", "TODO", "bug"}, + Edited: true, + }, + }, + }, + wantsErr: false, + }, + { + name: "remove-label flag", + input: "23 --remove-label feature,TODO,bug", + output: EditOptions{ + SelectorArg: "23", + Editable: prShared.Editable{ + Labels: prShared.EditableSlice{ + Remove: []string{"feature", "TODO", "bug"}, + Edited: true, + }, + }, + }, + wantsErr: false, + }, + { + name: "add-project flag", + input: "23 --add-project Cleanup,Roadmap", + output: EditOptions{ + SelectorArg: "23", + Editable: prShared.Editable{ + Projects: prShared.EditableSlice{ + Add: []string{"Cleanup", "Roadmap"}, + Edited: true, + }, + }, + }, + wantsErr: false, + }, + { + name: "remove-project flag", + input: "23 --remove-project Cleanup,Roadmap", + output: EditOptions{ + SelectorArg: "23", + Editable: prShared.Editable{ + Projects: prShared.EditableSlice{ + Remove: []string{"Cleanup", "Roadmap"}, + Edited: true, + }, }, }, wantsErr: false, @@ -102,8 +154,10 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: prShared.Editable{ - Milestone: "GA", - MilestoneEdited: true, + Milestone: prShared.EditableString{ + Value: "GA", + Edited: true, + }, }, }, wantsErr: false, @@ -163,18 +217,33 @@ func Test_editRun(t *testing.T) { SelectorArg: "123", Interactive: false, Editable: prShared.Editable{ - Title: "new title", - TitleEdited: true, - Body: "new body", - BodyEdited: true, - Assignees: []string{"monalisa", "hubot"}, - AssigneesEdited: true, - Labels: []string{"feature", "TODO", "bug"}, - LabelsEdited: true, - Projects: []string{"Cleanup", "Roadmap"}, - ProjectsEdited: true, - Milestone: "GA", - MilestoneEdited: true, + Title: prShared.EditableString{ + Value: "new title", + Edited: true, + }, + Body: prShared.EditableString{ + Value: "new body", + Edited: true, + }, + Assignees: prShared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, + Labels: prShared.EditableSlice{ + Add: []string{"feature", "TODO", "bug"}, + Remove: []string{"docs"}, + Edited: true, + }, + Projects: prShared.EditableSlice{ + Add: []string{"Cleanup", "Roadmap"}, + Remove: []string{"Features"}, + Edited: true, + }, + Milestone: prShared.EditableString{ + Value: "GA", + Edited: true, + }, }, FetchOptions: prShared.FetchOptions, }, @@ -191,21 +260,21 @@ func Test_editRun(t *testing.T) { SelectorArg: "123", Interactive: true, FieldsToEditSurvey: func(eo *prShared.Editable) error { - eo.TitleEdited = true - eo.BodyEdited = true - eo.AssigneesEdited = true - eo.LabelsEdited = true - eo.ProjectsEdited = true - eo.MilestoneEdited = true + eo.Title.Edited = true + eo.Body.Edited = true + eo.Assignees.Edited = true + eo.Labels.Edited = true + eo.Projects.Edited = true + eo.Milestone.Edited = true return nil }, EditFieldsSurvey: func(eo *prShared.Editable, _ string) error { - eo.Title = "new title" - eo.Body = "new body" - eo.Assignees = []string{"monalisa", "hubot"} - eo.Labels = []string{"feature", "TODO", "bug"} - eo.Projects = []string{"Cleanup", "Roadmap"} - eo.Milestone = "GA" + eo.Title.Value = "new title" + eo.Body.Value = "new body" + eo.Assignees.Value = []string{"monalisa", "hubot"} + eo.Labels.Value = []string{"feature", "TODO", "bug"} + eo.Projects.Value = []string{"Cleanup", "Roadmap"} + eo.Milestone.Value = "GA" return nil }, FetchOptions: prShared.FetchOptions, diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 0bd5ce826..6c1592b4c 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -50,12 +50,11 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman Short: "Edit a pull request", Example: heredoc.Doc(` $ gh pr edit 23 --title "I found a bug" --body "Nothing works" - $ gh pr edit 23 --label "bug,help wanted" - $ gh pr edit 23 --label bug --label "help wanted" - $ gh pr edit 23 --reviewer monalisa,hubot --reviewer myorg/team-name - $ gh pr edit 23 --assignee monalisa,hubot - $ gh pr edit 23 --assignee @me - $ gh pr edit 23 --project "Roadmap" + $ gh pr edit 23 --add-label "bug,help wanted" --remove-label "core" + $ gh pr edit 23 --add-reviewer monalisa,hubot --remove-reviewer myorg/team-name + $ gh pr edit 23 --add-assignee @me --remove-assignee monalisa,hubot + $ gh pr edit 23 --add-project "Roadmap" --remove-project v1,v2 + $ gh pr edit 23 --milestone "Version 1" `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -66,25 +65,25 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman flags := cmd.Flags() if flags.Changed("title") { - opts.Editable.TitleEdited = true + opts.Editable.Title.Edited = true } if flags.Changed("body") { - opts.Editable.BodyEdited = true + opts.Editable.Body.Edited = true } - if flags.Changed("reviewer") { - opts.Editable.ReviewersEdited = true + if flags.Changed("add-reviewer") || flags.Changed("remove-reviewer") { + opts.Editable.Reviewers.Edited = true } - if flags.Changed("assignee") { - opts.Editable.AssigneesEdited = true + if flags.Changed("add-assignee") || flags.Changed("remove-assignee") { + opts.Editable.Assignees.Edited = true } - if flags.Changed("label") { - opts.Editable.LabelsEdited = true + if flags.Changed("add-label") || flags.Changed("remove-label") { + opts.Editable.Labels.Edited = true } - if flags.Changed("project") { - opts.Editable.ProjectsEdited = true + if flags.Changed("add-project") || flags.Changed("remove-project") { + opts.Editable.Projects.Edited = true } if flags.Changed("milestone") { - opts.Editable.MilestoneEdited = true + opts.Editable.Milestone.Edited = true } if !opts.Editable.Dirty() { @@ -103,13 +102,17 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman }, } - cmd.Flags().StringVarP(&opts.Editable.Title, "title", "t", "", "Revise the pr title.") - cmd.Flags().StringVarP(&opts.Editable.Body, "body", "b", "", "Revise the pr body.") - cmd.Flags().StringSliceVarP(&opts.Editable.Reviewers, "reviewer", "r", nil, "Request reviews from people or teams by their `handle`") - cmd.Flags().StringSliceVarP(&opts.Editable.Assignees, "assignee", "a", nil, "Set assigned people by their `login`. Use \"@me\" to self-assign.") - cmd.Flags().StringSliceVarP(&opts.Editable.Labels, "label", "l", nil, "Set the pr labels by `name`") - cmd.Flags().StringSliceVarP(&opts.Editable.Projects, "project", "p", nil, "Set the projects the pr belongs to by `name`") - cmd.Flags().StringVarP(&opts.Editable.Milestone, "milestone", "m", "", "Set the milestone the pr belongs to by `name`") + cmd.Flags().StringVarP(&opts.Editable.Title.Value, "title", "t", "", "Set the new title.") + cmd.Flags().StringVarP(&opts.Editable.Body.Value, "body", "b", "", "Set the new body.") + cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Add, "add-reviewer", nil, "Add reviewers by their `login`.") + cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Remove, "remove-reviewer", nil, "Remove reviewers by their `login`.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself.") + cmd.Flags().StringSliceVar(&opts.Editable.Labels.Add, "add-label", nil, "Add labels by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Labels.Remove, "remove-label", nil, "Remove labels by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the pull request to projects by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Remove, "remove-project", nil, "Remove the pull request from projects by `name`") + cmd.Flags().StringVarP(&opts.Editable.Milestone.Value, "milestone", "m", "", "Edit the milestone the pull request belongs to by `name`") return cmd } @@ -127,14 +130,14 @@ func editRun(opts *EditOptions) error { } editable := opts.Editable - editable.ReviewersAllowed = true - editable.TitleDefault = pr.Title - editable.BodyDefault = pr.Body - editable.ReviewersDefault = pr.ReviewRequests - editable.AssigneesDefault = pr.Assignees - editable.LabelsDefault = pr.Labels - editable.ProjectsDefault = pr.ProjectCards - editable.MilestoneDefault = pr.Milestone + editable.Reviewers.Allowed = true + editable.Title.Default = pr.Title + editable.Body.Default = pr.Body + editable.Reviewers.Default = pr.ReviewRequests.Logins() + editable.Assignees.Default = pr.Assignees.Logins() + editable.Labels.Default = pr.Labels.Names() + editable.Projects.Default = pr.ProjectCards.ProjectNames() + editable.Milestone.Default = pr.Milestone.Title if opts.Interactive { err = opts.Surveyor.FieldsToEdit(&editable) @@ -177,25 +180,29 @@ func updatePullRequest(client *api.Client, repo ghrepo.Interface, id string, edi var err error params := githubv4.UpdatePullRequestInput{ PullRequestID: id, - Title: editable.TitleParam(), - Body: editable.BodyParam(), + Title: ghString(editable.TitleValue()), + Body: ghString(editable.BodyValue()), } - params.AssigneeIDs, err = editable.AssigneesParam(client, repo) + assigneeIds, err := editable.AssigneeIds(client, repo) if err != nil { return err } - params.LabelIDs, err = editable.LabelsParam() + params.AssigneeIDs = ghIds(assigneeIds) + labelIds, err := editable.LabelIds() if err != nil { return err } - params.ProjectIDs, err = editable.ProjectsParam() + params.LabelIDs = ghIds(labelIds) + projectIds, err := editable.ProjectIds() if err != nil { return err } - params.MilestoneID, err = editable.MilestoneParam() + params.ProjectIDs = ghIds(projectIds) + milestoneId, err := editable.MilestoneId() if err != nil { return err } + params.MilestoneID = ghId(milestoneId) err = api.UpdatePullRequest(client, repo, params) if err != nil { return err @@ -204,10 +211,10 @@ func updatePullRequest(client *api.Client, repo ghrepo.Interface, id string, edi } func updatePullRequestReviews(client *api.Client, repo ghrepo.Interface, id string, editable shared.Editable) error { - if !editable.ReviewersEdited { + if !editable.Reviewers.Edited { return nil } - userIds, teamIds, err := editable.ReviewersParams() + userIds, teamIds, err := editable.ReviewerIds() if err != nil { return err } @@ -215,8 +222,8 @@ func updatePullRequestReviews(client *api.Client, repo ghrepo.Interface, id stri reviewsRequestParams := githubv4.RequestReviewsInput{ PullRequestID: id, Union: &union, - UserIDs: userIds, - TeamIDs: teamIds, + UserIDs: ghIds(userIds), + TeamIDs: ghIds(teamIds), } return api.UpdatePullRequestReviews(client, repo, reviewsRequestParams) } @@ -257,3 +264,34 @@ type editorRetriever struct { func (e editorRetriever) Retrieve() (string, error) { return cmdutil.DetermineEditor(e.config) } + +func ghIds(s *[]string) *[]githubv4.ID { + if s == nil { + return nil + } + ids := make([]githubv4.ID, len(*s)) + for i, v := range *s { + ids[i] = v + } + return &ids +} + +func ghId(s *string) *githubv4.ID { + if s == nil { + return nil + } + if *s == "" { + r := githubv4.ID(nil) + return &r + } + r := githubv4.ID(*s) + return &r +} + +func ghString(s *string) *githubv4.String { + if s == nil { + return nil + } + r := githubv4.String(*s) + return &r +} diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 1351ff29a..35d8278dc 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -29,7 +29,7 @@ func TestNewCmdEdit(t *testing.T) { wantsErr: true, }, { - name: "issue number argument", + name: "pull request number argument", input: "23", output: EditOptions{ SelectorArg: "23", @@ -43,8 +43,10 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Title: "test", - TitleEdited: true, + Title: shared.EditableString{ + Value: "test", + Edited: true, + }, }, }, wantsErr: false, @@ -55,56 +57,122 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Body: "test", - BodyEdited: true, + Body: shared.EditableString{ + Value: "test", + Edited: true, + }, }, }, wantsErr: false, }, { - name: "reviewer flag", - input: "23 --reviewer owner/team,monalisa", + name: "add-reviewer flag", + input: "23 --add-reviewer monalisa,owner/core", output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Reviewers: []string{"owner/team", "monalisa"}, - ReviewersEdited: true, + Reviewers: shared.EditableSlice{ + Add: []string{"monalisa", "owner/core"}, + Edited: true, + }, }, }, wantsErr: false, }, { - name: "assignee flag", - input: "23 --assignee monalisa,hubot", + name: "remove-reviewer flag", + input: "23 --remove-reviewer monalisa,owner/core", output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Assignees: []string{"monalisa", "hubot"}, - AssigneesEdited: true, + Reviewers: shared.EditableSlice{ + Remove: []string{"monalisa", "owner/core"}, + Edited: true, + }, }, }, wantsErr: false, }, { - name: "label flag", - input: "23 --label feature,TODO,bug", + name: "add-assignee flag", + input: "23 --add-assignee monalisa,hubot", output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Labels: []string{"feature", "TODO", "bug"}, - LabelsEdited: true, + Assignees: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Edited: true, + }, }, }, wantsErr: false, }, { - name: "project flag", - input: "23 --project Cleanup,Roadmap", + name: "remove-assignee flag", + input: "23 --remove-assignee monalisa,hubot", output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Projects: []string{"Cleanup", "Roadmap"}, - ProjectsEdited: true, + Assignees: shared.EditableSlice{ + Remove: []string{"monalisa", "hubot"}, + Edited: true, + }, + }, + }, + wantsErr: false, + }, + { + name: "add-label flag", + input: "23 --add-label feature,TODO,bug", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Labels: shared.EditableSlice{ + Add: []string{"feature", "TODO", "bug"}, + Edited: true, + }, + }, + }, + wantsErr: false, + }, + { + name: "remove-label flag", + input: "23 --remove-label feature,TODO,bug", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Labels: shared.EditableSlice{ + Remove: []string{"feature", "TODO", "bug"}, + Edited: true, + }, + }, + }, + wantsErr: false, + }, + { + name: "add-project flag", + input: "23 --add-project Cleanup,Roadmap", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Projects: shared.EditableSlice{ + Add: []string{"Cleanup", "Roadmap"}, + Edited: true, + }, + }, + }, + wantsErr: false, + }, + { + name: "remove-project flag", + input: "23 --remove-project Cleanup,Roadmap", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Projects: shared.EditableSlice{ + Remove: []string{"Cleanup", "Roadmap"}, + Edited: true, + }, }, }, wantsErr: false, @@ -115,8 +183,10 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Milestone: "GA", - MilestoneEdited: true, + Milestone: shared.EditableString{ + Value: "GA", + Edited: true, + }, }, }, wantsErr: false, @@ -176,20 +246,38 @@ func Test_editRun(t *testing.T) { SelectorArg: "123", Interactive: false, Editable: shared.Editable{ - Title: "new title", - TitleEdited: true, - Body: "new body", - BodyEdited: true, - Reviewers: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot"}, - ReviewersEdited: true, - Assignees: []string{"monalisa", "hubot"}, - AssigneesEdited: true, - Labels: []string{"feature", "TODO", "bug"}, - LabelsEdited: true, - Projects: []string{"Cleanup", "Roadmap"}, - ProjectsEdited: true, - Milestone: "GA", - MilestoneEdited: true, + Title: shared.EditableString{ + Value: "new title", + Edited: true, + }, + Body: shared.EditableString{ + Value: "new body", + Edited: true, + }, + Reviewers: shared.EditableSlice{ + Add: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot"}, + Remove: []string{"dependabot"}, + Edited: true, + }, + Assignees: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, + Labels: shared.EditableSlice{ + Add: []string{"feature", "TODO", "bug"}, + Remove: []string{"docs"}, + Edited: true, + }, + Projects: shared.EditableSlice{ + Add: []string{"Cleanup", "Roadmap"}, + Remove: []string{"Features"}, + Edited: true, + }, + Milestone: shared.EditableString{ + Value: "GA", + Edited: true, + }, }, Fetcher: testFetcher{}, }, @@ -207,18 +295,33 @@ func Test_editRun(t *testing.T) { SelectorArg: "123", Interactive: false, Editable: shared.Editable{ - Title: "new title", - TitleEdited: true, - Body: "new body", - BodyEdited: true, - Assignees: []string{"monalisa", "hubot"}, - AssigneesEdited: true, - Labels: []string{"feature", "TODO", "bug"}, - LabelsEdited: true, - Projects: []string{"Cleanup", "Roadmap"}, - ProjectsEdited: true, - Milestone: "GA", - MilestoneEdited: true, + Title: shared.EditableString{ + Value: "new title", + Edited: true, + }, + Body: shared.EditableString{ + Value: "new body", + Edited: true, + }, + Assignees: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, + Labels: shared.EditableSlice{ + Value: []string{"feature", "TODO", "bug"}, + Remove: []string{"docs"}, + Edited: true, + }, + Projects: shared.EditableSlice{ + Value: []string{"Cleanup", "Roadmap"}, + Remove: []string{"Features"}, + Edited: true, + }, + Milestone: shared.EditableString{ + Value: "GA", + Edited: true, + }, }, Fetcher: testFetcher{}, }, @@ -405,28 +508,28 @@ func (f testFetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interf } func (s testSurveyor) FieldsToEdit(e *shared.Editable) error { - e.TitleEdited = true - e.BodyEdited = true + e.Title.Edited = true + e.Body.Edited = true if !s.skipReviewers { - e.ReviewersEdited = true + e.Reviewers.Edited = true } - e.AssigneesEdited = true - e.LabelsEdited = true - e.ProjectsEdited = true - e.MilestoneEdited = true + e.Assignees.Edited = true + e.Labels.Edited = true + e.Projects.Edited = true + e.Milestone.Edited = true return nil } func (s testSurveyor) EditFields(e *shared.Editable, _ string) error { - e.Title = "new title" - e.Body = "new body" + e.Title.Value = "new title" + e.Body.Value = "new body" if !s.skipReviewers { - e.Reviewers = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} + e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} } - e.Assignees = []string{"monalisa", "hubot"} - e.Labels = []string{"feature", "TODO", "bug"} - e.Projects = []string{"Cleanup", "Roadmap"} - e.Milestone = "GA" + e.Assignees.Value = []string{"monalisa", "hubot"} + e.Labels.Value = []string{"feature", "TODO", "bug"} + e.Projects.Value = []string{"Cleanup", "Roadmap"} + e.Milestone.Value = "GA" return nil } diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 885d1459d..16faca4de 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -7,174 +7,199 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/set" "github.com/cli/cli/pkg/surveyext" - "github.com/shurcooL/githubv4" ) type Editable struct { - Title string - TitleDefault string - TitleEdited bool + Title EditableString + Body EditableString + Reviewers EditableSlice + Assignees EditableSlice + Labels EditableSlice + Projects EditableSlice + Milestone EditableString + Metadata api.RepoMetadataResult +} - Body string - BodyDefault string - BodyEdited bool +type EditableString struct { + Value string + Default string + Options []string + Edited bool +} - Reviewers []string - ReviewersDefault api.ReviewRequests - ReviewersOptions []string - ReviewersEdited bool - ReviewersAllowed bool - - Assignees []string - AssigneesDefault api.Assignees - AssigneesOptions []string - AssigneesEdited bool - - Labels []string - LabelsDefault api.Labels - LabelsOptions []string - LabelsEdited bool - - Projects []string - ProjectsDefault api.ProjectCards - ProjectsOptions []string - ProjectsEdited bool - - Milestone string - MilestoneDefault api.Milestone - MilestoneOptions []string - MilestoneEdited bool - - Metadata api.RepoMetadataResult +type EditableSlice struct { + Value []string + Add []string + Remove []string + Default []string + Options []string + Edited bool + Allowed bool } func (e Editable) Dirty() bool { - return e.TitleEdited || - e.BodyEdited || - e.ReviewersEdited || - e.AssigneesEdited || - e.LabelsEdited || - e.ProjectsEdited || - e.MilestoneEdited + return e.Title.Edited || + e.Body.Edited || + e.Reviewers.Edited || + e.Assignees.Edited || + e.Labels.Edited || + e.Projects.Edited || + e.Milestone.Edited } -func (e Editable) TitleParam() *githubv4.String { - if !e.TitleEdited { +func (e Editable) TitleValue() *string { + if !e.Title.Edited { return nil } - s := githubv4.String(e.Title) - return &s + return &e.Title.Value } -func (e Editable) BodyParam() *githubv4.String { - if !e.BodyEdited { +func (e Editable) BodyValue() *string { + if !e.Body.Edited { return nil } - s := githubv4.String(e.Body) - return &s + return &e.Body.Value } -func (e Editable) ReviewersParams() (*[]githubv4.ID, *[]githubv4.ID, error) { - if !e.ReviewersEdited { +func (e Editable) ReviewerIds() (*[]string, *[]string, error) { + if !e.Reviewers.Edited { return nil, nil, nil } + if len(e.Reviewers.Add) != 0 || len(e.Reviewers.Remove) != 0 { + s := set.NewStringSet() + s.AddValues(e.Reviewers.Default) + s.AddValues(e.Reviewers.Add) + s.RemoveValues(e.Reviewers.Remove) + e.Reviewers.Value = s.ToSlice() + } var userReviewers []string var teamReviewers []string - for _, r := range e.Reviewers { + for _, r := range e.Reviewers.Value { if strings.ContainsRune(r, '/') { teamReviewers = append(teamReviewers, r) } else { userReviewers = append(userReviewers, r) } } - userIds, err := toParams(userReviewers, e.Metadata.MembersToIDs) + userIds, err := e.Metadata.MembersToIDs(userReviewers) if err != nil { return nil, nil, err } - teamIds, err := toParams(teamReviewers, e.Metadata.TeamsToIDs) + teamIds, err := e.Metadata.TeamsToIDs(teamReviewers) if err != nil { return nil, nil, err } - return userIds, teamIds, nil + return &userIds, &teamIds, nil } -func (e Editable) AssigneesParam(client *api.Client, repo ghrepo.Interface) (*[]githubv4.ID, error) { - if !e.AssigneesEdited { +func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]string, error) { + if !e.Assignees.Edited { return nil, nil } - meReplacer := NewMeReplacer(client, repo.RepoHost()) - assignees, err := meReplacer.ReplaceSlice(e.Assignees) - if err != nil { - return nil, err + if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 { + meReplacer := NewMeReplacer(client, repo.RepoHost()) + s := set.NewStringSet() + s.AddValues(e.Assignees.Default) + add, err := meReplacer.ReplaceSlice(e.Assignees.Add) + if err != nil { + return nil, err + } + s.AddValues(add) + remove, err := meReplacer.ReplaceSlice(e.Assignees.Remove) + if err != nil { + return nil, err + } + s.RemoveValues(remove) + e.Assignees.Value = s.ToSlice() } - return toParams(assignees, e.Metadata.MembersToIDs) + a, err := e.Metadata.MembersToIDs(e.Assignees.Value) + return &a, err } -func (e Editable) LabelsParam() (*[]githubv4.ID, error) { - if !e.LabelsEdited { +func (e Editable) LabelIds() (*[]string, error) { + if !e.Labels.Edited { return nil, nil } - return toParams(e.Labels, e.Metadata.LabelsToIDs) + if len(e.Labels.Add) != 0 || len(e.Labels.Remove) != 0 { + s := set.NewStringSet() + s.AddValues(e.Labels.Default) + s.AddValues(e.Labels.Add) + s.RemoveValues(e.Labels.Remove) + e.Labels.Value = s.ToSlice() + } + l, err := e.Metadata.LabelsToIDs(e.Labels.Value) + return &l, err } -func (e Editable) ProjectsParam() (*[]githubv4.ID, error) { - if !e.ProjectsEdited { +func (e Editable) ProjectIds() (*[]string, error) { + if !e.Projects.Edited { return nil, nil } - return toParams(e.Projects, e.Metadata.ProjectsToIDs) + if len(e.Projects.Add) != 0 || len(e.Projects.Remove) != 0 { + s := set.NewStringSet() + s.AddValues(e.Projects.Default) + s.AddValues(e.Projects.Add) + s.RemoveValues(e.Projects.Remove) + e.Projects.Value = s.ToSlice() + } + p, err := e.Metadata.ProjectsToIDs(e.Projects.Value) + return &p, err } -func (e Editable) MilestoneParam() (*githubv4.ID, error) { - if !e.MilestoneEdited { +func (e Editable) MilestoneId() (*string, error) { + if !e.Milestone.Edited { return nil, nil } - if e.Milestone == noMilestone || e.Milestone == "" { - return githubv4.NewID(nil), nil + if e.Milestone.Value == noMilestone || e.Milestone.Value == "" { + s := "" + return &s, nil } - return toParam(e.Milestone, e.Metadata.MilestoneToID) + m, err := e.Metadata.MilestoneToID(e.Milestone.Value) + return &m, err } func EditFieldsSurvey(editable *Editable, editorCommand string) error { var err error - if editable.TitleEdited { - editable.Title, err = titleSurvey(editable.TitleDefault) + if editable.Title.Edited { + editable.Title.Value, err = titleSurvey(editable.Title.Default) if err != nil { return err } } - if editable.BodyEdited { - editable.Body, err = bodySurvey(editable.BodyDefault, editorCommand) + if editable.Body.Edited { + editable.Body.Value, err = bodySurvey(editable.Body.Default, editorCommand) if err != nil { return err } } - if editable.ReviewersEdited { - editable.Reviewers, err = reviewersSurvey(editable.ReviewersDefault, editable.ReviewersOptions) + if editable.Reviewers.Edited { + editable.Reviewers.Value, err = multiSelectSurvey("Reviewers", editable.Reviewers.Default, editable.Reviewers.Options) if err != nil { return err } } - if editable.AssigneesEdited { - editable.Assignees, err = assigneesSurvey(editable.AssigneesDefault, editable.AssigneesOptions) + if editable.Assignees.Edited { + editable.Assignees.Value, err = multiSelectSurvey("Assignees", editable.Assignees.Default, editable.Assignees.Options) if err != nil { return err } } - if editable.LabelsEdited { - editable.Labels, err = labelsSurvey(editable.LabelsDefault, editable.LabelsOptions) + if editable.Labels.Edited { + editable.Labels.Value, err = multiSelectSurvey("Labels", editable.Labels.Default, editable.Labels.Options) if err != nil { return err } } - if editable.ProjectsEdited { - editable.Projects, err = projectsSurvey(editable.ProjectsDefault, editable.ProjectsOptions) + if editable.Projects.Edited { + editable.Projects.Value, err = multiSelectSurvey("Projects", editable.Projects.Default, editable.Projects.Options) if err != nil { return err } } - if editable.MilestoneEdited { - editable.Milestone, err = milestoneSurvey(editable.MilestoneDefault, editable.MilestoneOptions) + if editable.Milestone.Edited { + editable.Milestone.Value, err = milestoneSurvey(editable.Milestone.Default, editable.Milestone.Options) if err != nil { return err } @@ -200,41 +225,36 @@ func FieldsToEditSurvey(editable *Editable) error { return false } - results := []string{} opts := []string{"Title", "Body"} - if editable.ReviewersAllowed { + if editable.Reviewers.Allowed { opts = append(opts, "Reviewers") } opts = append(opts, "Assignees", "Labels", "Projects", "Milestone") - q := &survey.MultiSelect{ - Message: "What would you like to edit?", - Options: opts, - } - err := survey.AskOne(q, &results) + results, err := multiSelectSurvey("What would you like to edit?", []string{}, opts) if err != nil { return err } if contains(results, "Title") { - editable.TitleEdited = true + editable.Title.Edited = true } if contains(results, "Body") { - editable.BodyEdited = true + editable.Body.Edited = true } if contains(results, "Reviewers") { - editable.ReviewersEdited = true + editable.Reviewers.Edited = true } if contains(results, "Assignees") { - editable.AssigneesEdited = true + editable.Assignees.Edited = true } if contains(results, "Labels") { - editable.LabelsEdited = true + editable.Labels.Edited = true } if contains(results, "Projects") { - editable.ProjectsEdited = true + editable.Projects.Edited = true } if contains(results, "Milestone") { - editable.MilestoneEdited = true + editable.Milestone.Edited = true } return nil @@ -242,11 +262,11 @@ func FieldsToEditSurvey(editable *Editable) error { func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) error { input := api.RepoMetadataInput{ - Reviewers: editable.ReviewersEdited, - Assignees: editable.AssigneesEdited, - Labels: editable.LabelsEdited, - Projects: editable.ProjectsEdited, - Milestones: editable.MilestoneEdited, + Reviewers: editable.Reviewers.Edited, + Assignees: editable.Assignees.Edited, + Labels: editable.Labels.Edited, + Projects: editable.Projects.Edited, + Milestones: editable.Milestone.Edited, } metadata, err := api.RepoMetadata(client, repo, input) if err != nil { @@ -275,11 +295,11 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) } editable.Metadata = *metadata - editable.ReviewersOptions = append(users, teams...) - editable.AssigneesOptions = users - editable.LabelsOptions = labels - editable.ProjectsOptions = projects - editable.MilestoneOptions = milestones + editable.Reviewers.Options = append(users, teams...) + editable.Assignees.Options = users + editable.Labels.Options = labels + editable.Projects.Options = projects + editable.Milestone.Options = milestones return nil } @@ -297,9 +317,9 @@ func titleSurvey(title string) (string, error) { func bodySurvey(body, editorCommand string) (string, error) { var result string q := &surveyext.GhEditor{ - BlankAllowed: true, EditorCommand: editorCommand, - Editor: &survey.Editor{Message: "Body", + Editor: &survey.Editor{ + Message: "Body", FileName: "*.md", Default: body, HideDefault: true, @@ -310,79 +330,21 @@ func bodySurvey(body, editorCommand string) (string, error) { return result, err } -func reviewersSurvey(reviewers api.ReviewRequests, opts []string) ([]string, error) { - if len(opts) == 0 { +func multiSelectSurvey(message string, defaults, options []string) ([]string, error) { + if len(options) == 0 { return nil, nil } - logins := []string{} - for _, a := range reviewers.Nodes { - logins = append(logins, a.RequestedReviewer.Login) - } var results []string q := &survey.MultiSelect{ - Message: "Reviewers", - Options: opts, - Default: logins, + Message: message, + Options: options, + Default: defaults, } err := survey.AskOne(q, &results) return results, err } -func assigneesSurvey(assignees api.Assignees, opts []string) ([]string, error) { - if len(opts) == 0 { - return nil, nil - } - logins := []string{} - for _, a := range assignees.Nodes { - logins = append(logins, a.Login) - } - var results []string - q := &survey.MultiSelect{ - Message: "Assignees", - Options: opts, - Default: logins, - } - err := survey.AskOne(q, &results) - return results, err -} - -func labelsSurvey(labels api.Labels, opts []string) ([]string, error) { - if len(opts) == 0 { - return nil, nil - } - names := []string{} - for _, l := range labels.Nodes { - names = append(names, l.Name) - } - var results []string - q := &survey.MultiSelect{ - Message: "Labels", - Options: opts, - Default: names, - } - err := survey.AskOne(q, &results) - return results, err -} - -func projectsSurvey(projectCards api.ProjectCards, opts []string) ([]string, error) { - if len(opts) == 0 { - return nil, nil - } - names := []string{} - for _, c := range projectCards.Nodes { - names = append(names, c.Project.Name) - } - var results []string - q := &survey.MultiSelect{ - Message: "Projects", - Options: opts, - Default: names, - } - err := survey.AskOne(q, &results) - return results, err -} - -func milestoneSurvey(milestone api.Milestone, opts []string) (string, error) { +func milestoneSurvey(title string, opts []string) (string, error) { if len(opts) == 0 { return "", nil } @@ -390,7 +352,7 @@ func milestoneSurvey(milestone api.Milestone, opts []string) (string, error) { q := &survey.Select{ Message: "Milestone", Options: opts, - Default: milestone.Title, + Default: title, } err := survey.AskOne(q, &result) return result, err @@ -405,24 +367,3 @@ func confirmSurvey() (bool, error) { err := survey.AskOne(q, &result) return result, err } - -func toParams(s []string, mapper func([]string) ([]string, error)) (*[]githubv4.ID, error) { - ids, err := mapper(s) - if err != nil { - return nil, err - } - gIds := make([]githubv4.ID, len(ids)) - for i, v := range ids { - gIds[i] = v - } - return &gIds, nil -} - -func toParam(s string, mapper func(string) (string, error)) (*githubv4.ID, error) { - id, err := mapper(s) - if err != nil { - return nil, err - } - gId := githubv4.ID(id) - return &gId, nil -} diff --git a/pkg/set/string_set.go b/pkg/set/string_set.go new file mode 100644 index 000000000..c7e7726a4 --- /dev/null +++ b/pkg/set/string_set.go @@ -0,0 +1,46 @@ +package set + +var exists = struct{}{} + +type stringSet struct { + m map[string]struct{} +} + +func NewStringSet() *stringSet { + s := &stringSet{} + s.m = make(map[string]struct{}) + return s +} + +func (s *stringSet) Add(value string) { + s.m[value] = exists +} + +func (s *stringSet) AddValues(values []string) { + for _, v := range values { + s.Add(v) + } +} + +func (s *stringSet) Remove(value string) { + delete(s.m, value) +} + +func (s *stringSet) RemoveValues(values []string) { + for _, v := range values { + s.Remove(v) + } +} + +func (s *stringSet) Contains(value string) bool { + _, c := s.m[value] + return c +} + +func (s *stringSet) ToSlice() []string { + r := make([]string, 0, len(s.m)) + for k := range s.m { + r = append(r, k) + } + return r +} From 57140ad35e8eb600fcc5b36ac86835a4c126364e Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 16 Feb 2021 12:25:09 -0600 Subject: [PATCH 22/22] add header in correct place --- api/client.go | 1 - pkg/cmd/factory/http.go | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/client.go b/api/client.go index 17534b8b2..09195181b 100644 --- a/api/client.go +++ b/api/client.go @@ -239,7 +239,6 @@ func (c Client) GraphQL(hostname string, query string, variables map[string]inte } req.Header.Set("Content-Type", "application/json; charset=utf-8") - req.Header.Set("Accept", "application/vnd.github.merge-info-preview+json") resp, err := c.http.Do(req) if err != nil { diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index 8cd1bf12a..2e7619540 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -87,6 +87,8 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string api.AddHeaderFunc("Accept", func(req *http.Request) (string, error) { // antiope-preview: Checks accept := "application/vnd.github.antiope-preview+json" + // introduced for #2952: pr branch up to date status + accept += ", application/vnd.github.merge-info-preview+json" if ghinstance.IsEnterprise(req.URL.Hostname()) { // shadow-cat-preview: Draft pull requests accept += ", application/vnd.github.shadow-cat-preview"