From 17257df06458838c7c7309128ad0ca4e7f9f0457 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 21 Jul 2024 12:45:28 +0100 Subject: [PATCH 01/14] Verify `--body` and `--body-file` are not assignable at the same time Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index db0b09d2d..b6b7e8f18 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -104,6 +104,11 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, + { + name: "both body and body-file flags", + input: "23 --body foo --body-file bar", + wantsErr: true, + }, { name: "add-assignee flag", input: "23 --add-assignee monalisa,hubot", From 9dabc3b8dd282275a785a2461370fee907af847c Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 21 Jul 2024 12:53:15 +0100 Subject: [PATCH 02/14] Remove unused expected `output` from test case (with `wantsErr: true`) Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit_test.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index b6b7e8f18..a7e68cbc0 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -226,17 +226,8 @@ func TestNewCmdEdit(t *testing.T) { wantsErr: false, }, { - name: "interactive multiple issues", - input: "23 34", - output: EditOptions{ - SelectorArgs: []string{"23", "34"}, - Editable: prShared.Editable{ - Labels: prShared.EditableSlice{ - Add: []string{"bug"}, - Edited: true, - }, - }, - }, + name: "interactive multiple issues", + input: "23 34", wantsErr: true, }, } From 683576d6bf509e1602d38a44554a38abd6994b79 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 21 Jul 2024 12:54:23 +0100 Subject: [PATCH 03/14] Add `--remove-milestone` option Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 1b775f88b..7b9719468 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -32,6 +32,7 @@ type EditOptions struct { Interactive bool prShared.Editable + RemoveMilestone bool } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -62,6 +63,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman $ 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" + $ gh issue edit 23 --remove-milestone $ gh issue edit 23 --body-file body.txt $ gh issue edit 23 34 --add-label "help wanted" `), @@ -95,6 +97,14 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } } + if err := cmdutil.MutuallyExclusive( + "specify only one of `--milestone` or `--remove-milestone`", + flags.Changed("milestone"), + opts.RemoveMilestone, + ); err != nil { + return err + } + if flags.Changed("title") { opts.Editable.Title.Edited = true } @@ -107,8 +117,12 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if flags.Changed("add-project") || flags.Changed("remove-project") { opts.Editable.Projects.Edited = true } - if flags.Changed("milestone") { + if flags.Changed("milestone") || opts.RemoveMilestone { opts.Editable.Milestone.Edited = true + + // Note that when `--remove-milestone` is provided, the value of + // `opts.Editable.Milestone.Value` will automatically be empty, + // which results in milestone association removal. } if !opts.Editable.Dirty() { @@ -141,6 +155,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman 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`") + cmd.Flags().BoolVar(&opts.RemoveMilestone, "remove-milestone", false, "Remove the milestone the issue belongs to") return cmd } From 168a8832f087ba1a3a021c16b96743c42959f0f4 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 21 Jul 2024 12:55:47 +0100 Subject: [PATCH 04/14] Assert correct parsing of `--remove-milestone` option Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index a7e68cbc0..1457c6464 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -211,6 +211,20 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, + { + name: "remove-milestone flag", + input: "23 --remove-milestone", + output: EditOptions{ + SelectorArgs: []string{"23"}, + Editable: prShared.Editable{ + Milestone: prShared.EditableString{ + Value: "", + Edited: true, + }, + }, + }, + wantsErr: false, + }, { name: "add label to multiple issues", input: "23 34 --add-label bug", From 42da8baf7046b801e4c9c78a23effc048a2c81c5 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 21 Jul 2024 12:56:14 +0100 Subject: [PATCH 05/14] Verify `--milestone` and `--remove-milestone` are not assignable at the same time Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 1457c6464..40fe6491c 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -225,6 +225,11 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, + { + name: "both milestone and remove-milestone flags", + input: "23 --milestone foo --remove-milestone", + wantsErr: true, + }, { name: "add label to multiple issues", input: "23 34 --add-label bug", From 6e34e81b764c8587d8c5bbf7e9e2a63449e3c22a Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 28 Jul 2024 15:22:54 +0100 Subject: [PATCH 06/14] Point to `Editable.MilestoneId` method Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 7b9719468..263a48f20 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -122,7 +122,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman // Note that when `--remove-milestone` is provided, the value of // `opts.Editable.Milestone.Value` will automatically be empty, - // which results in milestone association removal. + // which results in milestone association removal. For reference, + // see the `Editable.MilestoneId` method. } if !opts.Editable.Dirty() { From 4c8ec84e2654ad6d622107e3a8d675a2d05c1388 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 28 Jul 2024 15:23:19 +0100 Subject: [PATCH 07/14] Improve `--remove-milestone` option description Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 263a48f20..de334148a 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -156,7 +156,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman 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`") - cmd.Flags().BoolVar(&opts.RemoveMilestone, "remove-milestone", false, "Remove the milestone the issue belongs to") + cmd.Flags().BoolVar(&opts.RemoveMilestone, "remove-milestone", false, "Remove the milestone association from the issue") return cmd } From 1520292726db9e5e7c0b6e35918792f34e47304d Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 28 Jul 2024 15:40:38 +0100 Subject: [PATCH 08/14] Add `--remove-milestone` option Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 3f80aa80a..2f288857b 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -30,6 +30,7 @@ type EditOptions struct { Interactive bool shared.Editable + RemoveMilestone bool } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -63,6 +64,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman $ 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" + $ gh pr edit 23 --remove-milestone `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -95,6 +97,14 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } } + if err := cmdutil.MutuallyExclusive( + "specify only one of `--milestone` or `--remove-milestone`", + flags.Changed("milestone"), + opts.RemoveMilestone, + ); err != nil { + return err + } + if flags.Changed("title") { opts.Editable.Title.Edited = true } @@ -116,8 +126,13 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if flags.Changed("add-project") || flags.Changed("remove-project") { opts.Editable.Projects.Edited = true } - if flags.Changed("milestone") { + if flags.Changed("milestone") || opts.RemoveMilestone { opts.Editable.Milestone.Edited = true + + // Note that when `--remove-milestone` is provided, the value of + // `opts.Editable.Milestone.Value` will automatically be empty, + // which results in milestone association removal. For reference, + // see the `Editable.MilestoneId` method. } if !opts.Editable.Dirty() { @@ -149,6 +164,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman 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`") + cmd.Flags().BoolVar(&opts.RemoveMilestone, "remove-milestone", false, "Remove the milestone association from the pull request") _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "base") From 6dc8cc41d0ed2892d14089781540fb9b4ef788d7 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 28 Jul 2024 15:41:54 +0100 Subject: [PATCH 09/14] Verify `--body` and `--body-file` are not assignable at the same time --- pkg/cmd/pr/edit/edit_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 0986797ea..6884d3d6e 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -112,6 +112,11 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, + { + name: "both body and body-file flags", + input: "23 --body foo --body-file bar", + wantsErr: true, + }, { name: "base flag", input: "23 --base base-branch-name", From 605b4b19e11c5c5deb50d5fc5683312620d4ec24 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 28 Jul 2024 15:43:38 +0100 Subject: [PATCH 10/14] Assert correct parsing of `--remove-milestone` option Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 6884d3d6e..35840559f 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -261,6 +261,20 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, + { + name: "remove-milestone flag", + input: "23 --remove-milestone", + output: EditOptions{ + SelectorArg: "23", + Editable: shared.Editable{ + Milestone: shared.EditableString{ + Value: "", + Edited: true, + }, + }, + }, + wantsErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From c9fb4ed0996e8b56ac755fe1d0d738f893e46906 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sun, 28 Jul 2024 15:44:05 +0100 Subject: [PATCH 11/14] Verify `--milestone` and `--remove-milestone` are not assignable at the same time Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 35840559f..3c4882961 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -275,6 +275,11 @@ func TestNewCmdEdit(t *testing.T) { }, wantsErr: false, }, + { + name: "both milestone and remove-milestone flags", + input: "23 --milestone foo --remove-milestone", + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 842562d3dbc6c7a0ed62d1e16a95a9dce8f1904e Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 29 Jul 2024 21:36:48 +0100 Subject: [PATCH 12/14] Use closure-scoped variable to catch `--remove-milestone` option Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 2f288857b..21aa80c07 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -30,7 +30,6 @@ type EditOptions struct { Interactive bool shared.Editable - RemoveMilestone bool } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -44,6 +43,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } var bodyFile string + var removeMilestone bool cmd := &cobra.Command{ Use: "edit [ | | ]", @@ -100,7 +100,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if err := cmdutil.MutuallyExclusive( "specify only one of `--milestone` or `--remove-milestone`", flags.Changed("milestone"), - opts.RemoveMilestone, + opts.removeMilestone, ); err != nil { return err } @@ -126,7 +126,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if flags.Changed("add-project") || flags.Changed("remove-project") { opts.Editable.Projects.Edited = true } - if flags.Changed("milestone") || opts.RemoveMilestone { + if flags.Changed("milestone") || opts.removeMilestone { opts.Editable.Milestone.Edited = true // Note that when `--remove-milestone` is provided, the value of @@ -164,7 +164,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman 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`") - cmd.Flags().BoolVar(&opts.RemoveMilestone, "remove-milestone", false, "Remove the milestone association from the pull request") + cmd.Flags().BoolVar(&removeMilestone, "remove-milestone", false, "Remove the milestone association from the pull request") _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "base") From a73ae65ca81d9c3647725ec7e3391f196dfa49b1 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 29 Jul 2024 21:37:55 +0100 Subject: [PATCH 13/14] Use closure-scoped variable to catch `--remove-milestone` option Signed-off-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index de334148a..11a69383d 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -32,7 +32,6 @@ type EditOptions struct { Interactive bool prShared.Editable - RemoveMilestone bool } func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { @@ -47,6 +46,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } var bodyFile string + var removeMilestone bool cmd := &cobra.Command{ Use: "edit { | }", @@ -100,7 +100,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if err := cmdutil.MutuallyExclusive( "specify only one of `--milestone` or `--remove-milestone`", flags.Changed("milestone"), - opts.RemoveMilestone, + removeMilestone, ); err != nil { return err } @@ -117,7 +117,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if flags.Changed("add-project") || flags.Changed("remove-project") { opts.Editable.Projects.Edited = true } - if flags.Changed("milestone") || opts.RemoveMilestone { + if flags.Changed("milestone") || removeMilestone { opts.Editable.Milestone.Edited = true // Note that when `--remove-milestone` is provided, the value of @@ -156,7 +156,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman 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`") - cmd.Flags().BoolVar(&opts.RemoveMilestone, "remove-milestone", false, "Remove the milestone association from the issue") + cmd.Flags().BoolVar(&removeMilestone, "remove-milestone", false, "Remove the milestone association from the issue") return cmd } From 58e61967774d93e02d5a03946a4845f0ce1814be Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 29 Jul 2024 21:41:43 +0100 Subject: [PATCH 14/14] Fix missing variable Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 21aa80c07..9dc190011 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -100,7 +100,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if err := cmdutil.MutuallyExclusive( "specify only one of `--milestone` or `--remove-milestone`", flags.Changed("milestone"), - opts.removeMilestone, + removeMilestone, ); err != nil { return err } @@ -126,7 +126,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman if flags.Changed("add-project") || flags.Changed("remove-project") { opts.Editable.Projects.Edited = true } - if flags.Changed("milestone") || opts.removeMilestone { + if flags.Changed("milestone") || removeMilestone { opts.Editable.Milestone.Edited = true // Note that when `--remove-milestone` is provided, the value of