From 274a09bbc97657163b2eabf64a5c979987fd64c3 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 23 Apr 2025 10:11:51 -0400 Subject: [PATCH 01/11] Initial `gh accessibility` command draft This commit captures the initial command along with functionality and description. There is an internal discussion about the appropriate place for some of this content. --- pkg/cmd/accessibility/accessibility.go | 150 +++++++++++++++++++++++++ pkg/cmd/root/help.go | 11 +- pkg/cmd/root/root.go | 2 + 3 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 pkg/cmd/accessibility/accessibility.go diff --git a/pkg/cmd/accessibility/accessibility.go b/pkg/cmd/accessibility/accessibility.go new file mode 100644 index 000000000..4992d488b --- /dev/null +++ b/pkg/cmd/accessibility/accessibility.go @@ -0,0 +1,150 @@ +package accessibility + +import ( + "fmt" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +const ( + communityURL = "https://github.com/orgs/community/discussions/categories/accessibility" +) + +type AccessibilityOptions struct { + IO *iostreams.IOStreams + Browser browser.Browser + Web bool +} + +func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { + opts := AccessibilityOptions{ + IO: f.IOStreams, + Browser: f.Browser, + } + + cmd := &cobra.Command{ + Use: "accessibility", + Aliases: []string{"a11y"}, + Short: "Learn about GitHub CLI accessibility experience", + Long: longDescription(opts.IO), + Hidden: true, + RunE: func(cmd *cobra.Command, args []string) error { + if opts.Web { + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(communityURL)) + } + return opts.Browser.Browse(communityURL) + } + + return cmd.Help() + }, + Example: heredoc.Doc(` + # Open the GitHub Community Accessibility discussions in your browser + $ gh accessibility --web + + # Display color using customizable, 4-bit accessible colors + $ gh config set accessible_colors enabled + + # Display issue and pull request labels using RGB hex color codes in terminals that support 24-bit truecolor + $ gh config set color_labels enabled + + # Use input prompts without redrawing the screen + $ gh config set accessible_prompter enabled + + # Disable motion-based spinners for progress indicators in favor of text + $ gh config set spinner disabled + `), + } + + cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open the GitHub Community Accessibility discussions in the browser") + cmdutil.DisableAuthCheck(cmd) + + return cmd +} + +func longDescription(io *iostreams.IOStreams) string { + cs := io.ColorScheme() + title := cs.Bold("LEARN ABOUT GITHUB CLI ACCESSIBILITY EFFORTS") + color := cs.Bold("CUSTOMIZABLE AND CONTRASTING COLORS") + prompter := cs.Bold("NON-INTERACTIVE USER INPUT PROMPTING") + spinner := cs.Bold("TEXT-BASED SPINNERS") + + return heredoc.Docf(` + %[2]s + + As the home for all developers, we want every developer to feel welcome in our + community and be empowered to contribute to the future of global software + development with everything GitHub has to offer including the GitHub CLI. + + We invite you to join us in improving GitHub CLI accessibility by sharing your + feedback and ideas in the GitHub Community Accessibility discussions: + %[3]s + + + %[4]s + + Color is a common approach to enhance user experiences, however users can find + themselves with a worse experience due to insufficient contrast or + customizability. + + To create an accessible experience, CLIs should use color palettes based on + terminal background appearance and limit colors to 4-bit ANSI color palettes, + which users can customize within terminal preferences. + + With this new experience, the GitHub CLI provides multiple options to address + color usage: + + 1. The GitHub CLI will use 4-bit color palette for increased color contrast based on + dark and light backgrounds including rendering markdown based on GitHub Primer. + + To enable this experience, use one of the following methods: + - Run %[1]sgh config set accessible_colors enabled%[1]s + - Set %[1]sGH_ACCESSIBLE_COLORS=enabled%[1]s environment variable + + 2. The GitHub CLI will display issue and pull request labels' custom RGB colors + in terminals with truecolor support. + + To enable this experience, use one of the following methods: + - Run %[1]sgh config set color_labels enabled%[1]s + - Set %[1]sGH_COLOR_LABELS=enabled%[1]s environment variable + + + %[5]s + + Interactive text user interfaces are an advanced approach to enhance user + experiences, which manipulate the terminal cursor to redraw parts of the screen. + However, this can be difficult for speech synthesizers or braille displays to + accurately detect and read. + + To create an accessible experience, CLIs should give users the ability to disable + this interactivity while providing a similar experience. + + With this new experience, the GitHub CLI will use non-interactive prompts for + user input. + + To enable this experience, use one of the following methods: + - Run %[1]sgh config set accessible_prompter enabled%[1]s + - Set %[1]sGH_ACCESSIBLE_PROMPTER=enabled%[1]s environment variable + + + %[6]s + + Motion-based spinners are a common approach to communicate activity, which + manipulate the terminal cursor to create a spinning effect. However, this can be + difficult for users with motion sensitivity as well as speech synthesizers. + + To create an accessible experience, CLIs should give users the ability to disable + this interactivity while providing a similar experience. + + With this new experience, the GitHub CLI will use text-based progress indicators. + + To enable this experience, use one of the following methods: + - Run %[1]sgh config set spinner disabled%[1]s + - Set %[1]sGH_SPINNER_DISABLED=yes%[1]s environment variable + `, "`", title, communityURL, color, prompter, spinner) +} diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index 7f8fb1c2e..ec6499f21 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -109,8 +109,6 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) { return } - namePadding := 12 - type helpEntry struct { Title string Body string @@ -135,6 +133,12 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) { helpEntries = append(helpEntries, helpEntry{"ALIASES", strings.Join(BuildAliasList(command, command.Aliases), ", ") + "\n"}) } + // Statically calculated padding for non-extension commands, + // longest is `gh accessibility` with 13 characters + 1 space. + // + // Should consider novel way to calculate this in the future [AF] + namePadding := 14 + for _, g := range GroupedCommands(command) { var names []string for _, c := range g.Commands { @@ -148,6 +152,9 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) { if isRootCmd(command) { var helpTopics []string + if c := findCommand(command, "accessibility"); c != nil { + helpTopics = append(helpTopics, rpad(c.Name()+":", namePadding)+c.Short) + } if c := findCommand(command, "actions"); c != nil { helpTopics = append(helpTopics, rpad(c.Name()+":", namePadding)+c.Short) } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index c0dad93ec..8cf30db1b 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/MakeNowJust/heredoc" + accessibilityCmd "github.com/cli/cli/v2/pkg/cmd/accessibility" actionsCmd "github.com/cli/cli/v2/pkg/cmd/actions" aliasCmd "github.com/cli/cli/v2/pkg/cmd/alias" "github.com/cli/cli/v2/pkg/cmd/alias/shared" @@ -122,6 +123,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) (*cobra.Command, // Child commands cmd.AddCommand(versionCmd.NewCmdVersion(f, version, buildDate)) + cmd.AddCommand(accessibilityCmd.NewCmdAccessibility(f)) cmd.AddCommand(actionsCmd.NewCmdActions(f)) cmd.AddCommand(aliasCmd.NewCmdAlias(f)) cmd.AddCommand(authCmd.NewCmdAuth(f)) From abd98bd727521286e0d0179a1c4818d38eb74ab7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 25 Apr 2025 15:00:36 +0000 Subject: [PATCH 02/11] chore(deps): bump github.com/cpuguy83/go-md2man/v2 from 2.0.6 to 2.0.7 Bumps [github.com/cpuguy83/go-md2man/v2](https://github.com/cpuguy83/go-md2man) from 2.0.6 to 2.0.7. - [Release notes](https://github.com/cpuguy83/go-md2man/releases) - [Commits](https://github.com/cpuguy83/go-md2man/compare/v2.0.6...v2.0.7) --- updated-dependencies: - dependency-name: github.com/cpuguy83/go-md2man/v2 dependency-version: 2.0.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 31b07f2cf..1ea50709d 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 github.com/cli/oauth v1.1.1 github.com/cli/safeexec v1.0.1 - github.com/cpuguy83/go-md2man/v2 v2.0.6 + github.com/cpuguy83/go-md2man/v2 v2.0.7 github.com/creack/pty v1.1.24 github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 github.com/distribution/reference v0.6.0 diff --git a/go.sum b/go.sum index b312bcf6c..0203b8bfc 100644 --- a/go.sum +++ b/go.sum @@ -142,8 +142,9 @@ github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUo github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4= github.com/containerd/stargz-snapshotter/estargz v0.16.3 h1:7evrXtoh1mSbGj/pfRccTampEyKpjpOnS3CyiV1Ebr8= github.com/containerd/stargz-snapshotter/estargz v0.16.3/go.mod h1:uyr4BfYfOj3G9WBVE8cOlQmXAbPN9VEQpBBeJIuOipU= -github.com/cpuguy83/go-md2man/v2 v2.0.6 h1:XJtiaUW6dEEqVuZiMTn1ldk455QWwEIsMIJlo5vtkx0= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo= +github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= From 9bb89de87c7fdeca18ccfc9b338e110c1ba676a9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 28 Apr 2025 15:44:19 +0000 Subject: [PATCH 03/11] chore(deps): bump actions/attest-build-provenance from 2.2.2 to 2.3.0 Bumps [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance) from 2.2.2 to 2.3.0. - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](https://github.com/actions/attest-build-provenance/compare/bd77c077858b8d561b7a36cbe48ef4cc642ca39d...db473fddc028af60658334401dc6fa3ffd8669fd) --- updated-dependencies: - dependency-name: actions/attest-build-provenance dependency-version: 2.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/deployment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index a7b03f40d..850cc19b7 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -309,7 +309,7 @@ jobs: rpmsign --addsign dist/*.rpm - name: Attest release artifacts if: inputs.environment == 'production' - uses: actions/attest-build-provenance@bd77c077858b8d561b7a36cbe48ef4cc642ca39d # v2.2.2 + uses: actions/attest-build-provenance@db473fddc028af60658334401dc6fa3ffd8669fd # v2.3.0 with: subject-path: "dist/gh_*" - name: Run createrepo From d7e2468286db92630a0393386d717aec3c46f0fb Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Mon, 28 Apr 2025 15:01:15 -0400 Subject: [PATCH 04/11] Update a11y text based on draft feedback --- pkg/cmd/accessibility/accessibility.go | 89 +++++++++++++------------- pkg/cmd/root/help.go | 1 + 2 files changed, 44 insertions(+), 46 deletions(-) diff --git a/pkg/cmd/accessibility/accessibility.go b/pkg/cmd/accessibility/accessibility.go index 4992d488b..1d4a8009f 100644 --- a/pkg/cmd/accessibility/accessibility.go +++ b/pkg/cmd/accessibility/accessibility.go @@ -12,7 +12,7 @@ import ( ) const ( - communityURL = "https://github.com/orgs/community/discussions/categories/accessibility" + feedbackURL = "https://accessibility.github.com/feedback" ) type AccessibilityOptions struct { @@ -30,27 +30,27 @@ func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "accessibility", Aliases: []string{"a11y"}, - Short: "Learn about GitHub CLI accessibility experience", + Short: "Learn about GitHub CLI accessibility experiences", Long: longDescription(opts.IO), Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { if opts.Web { if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(communityURL)) + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(feedbackURL)) } - return opts.Browser.Browse(communityURL) + return opts.Browser.Browse(feedbackURL) } return cmd.Help() }, Example: heredoc.Doc(` - # Open the GitHub Community Accessibility discussions in your browser + # Open the GitHub Accessibility site in your browser $ gh accessibility --web # Display color using customizable, 4-bit accessible colors $ gh config set accessible_colors enabled - # Display issue and pull request labels using RGB hex color codes in terminals that support 24-bit truecolor + # Display issue and pull request labels using RGB hex color codes in terminals that support 24-bit true color $ gh config set color_labels enabled # Use input prompts without redrawing the screen @@ -61,7 +61,7 @@ func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { `), } - cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open the GitHub Community Accessibility discussions in the browser") + cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open the GitHub Accessibility site in your browser") cmdutil.DisableAuthCheck(cmd) return cmd @@ -69,10 +69,11 @@ func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { func longDescription(io *iostreams.IOStreams) string { cs := io.ColorScheme() - title := cs.Bold("LEARN ABOUT GITHUB CLI ACCESSIBILITY EFFORTS") - color := cs.Bold("CUSTOMIZABLE AND CONTRASTING COLORS") - prompter := cs.Bold("NON-INTERACTIVE USER INPUT PROMPTING") - spinner := cs.Bold("TEXT-BASED SPINNERS") + title := cs.Bold("Learn about GitHub CLI accessibility experiences") + color := cs.Bold("Customizable and contrasting colors") + prompter := cs.Bold("Non-interactive user input prompting") + spinner := cs.Bold("Text-based spinners") + feedback := cs.Bold("Join the conversation") return heredoc.Docf(` %[2]s @@ -81,70 +82,66 @@ func longDescription(io *iostreams.IOStreams) string { community and be empowered to contribute to the future of global software development with everything GitHub has to offer including the GitHub CLI. - We invite you to join us in improving GitHub CLI accessibility by sharing your - feedback and ideas in the GitHub Community Accessibility discussions: %[3]s + Text interfaces often use color for various purposes, but insufficient contrast + or customizability can leave some users unable to benefit. - %[4]s - - Color is a common approach to enhance user experiences, however users can find - themselves with a worse experience due to insufficient contrast or - customizability. - - To create an accessible experience, CLIs should use color palettes based on - terminal background appearance and limit colors to 4-bit ANSI color palettes, - which users can customize within terminal preferences. + To create a more accessible experience, the GitHub CLI will use color palettes + based on terminal background appearance and limit colors to 4-bit ANSI color + palettes, which users can customize within terminal preferences. With this new experience, the GitHub CLI provides multiple options to address color usage: - 1. The GitHub CLI will use 4-bit color palette for increased color contrast based on - dark and light backgrounds including rendering markdown based on GitHub Primer. + 1. The GitHub CLI will use 4-bit color palette for increased color contrast based + on dark and light backgrounds including rendering Markdown based on the + GitHub Primer design system. To enable this experience, use one of the following methods: - Run %[1]sgh config set accessible_colors enabled%[1]s - Set %[1]sGH_ACCESSIBLE_COLORS=enabled%[1]s environment variable 2. The GitHub CLI will display issue and pull request labels' custom RGB colors - in terminals with truecolor support. + in terminals with true color support. To enable this experience, use one of the following methods: - Run %[1]sgh config set color_labels enabled%[1]s - Set %[1]sGH_COLOR_LABELS=enabled%[1]s environment variable + %[4]s - %[5]s + Interactive text user interfaces manipulate the terminal cursor to redraw parts + of the screen, which can be difficult for speech synthesizers or braille displays + to accurately detect and read. - Interactive text user interfaces are an advanced approach to enhance user - experiences, which manipulate the terminal cursor to redraw parts of the screen. - However, this can be difficult for speech synthesizers or braille displays to - accurately detect and read. - - To create an accessible experience, CLIs should give users the ability to disable - this interactivity while providing a similar experience. - - With this new experience, the GitHub CLI will use non-interactive prompts for - user input. + To create a more accessible experience, the GitHub CLI gives users the ability to + disable this interactivity while providing a similar experience using + non-interactive prompts for user input. To enable this experience, use one of the following methods: - Run %[1]sgh config set accessible_prompter enabled%[1]s - Set %[1]sGH_ACCESSIBLE_PROMPTER=enabled%[1]s environment variable + %[5]s - %[6]s + Motion-based spinners communicate in-progress activity by manipulating the + terminal cursor to create a spinning effect, which can be difficult for users + with motion sensitivity or miscommunicate information to speech synthesizers. - Motion-based spinners are a common approach to communicate activity, which - manipulate the terminal cursor to create a spinning effect. However, this can be - difficult for users with motion sensitivity as well as speech synthesizers. - - To create an accessible experience, CLIs should give users the ability to disable - this interactivity while providing a similar experience. - - With this new experience, the GitHub CLI will use text-based progress indicators. + To create a more accessible experience, the GitHub CLI gives users the ability to + disable this interactivity while providing a similar experience using text-based + progress indicators. To enable this experience, use one of the following methods: - Run %[1]sgh config set spinner disabled%[1]s - Set %[1]sGH_SPINNER_DISABLED=yes%[1]s environment variable - `, "`", title, communityURL, color, prompter, spinner) + + %[6]s + + We invite you to join us in improving GitHub CLI accessibility by sharing your + feedback and ideas through GitHub Accessibility feedback channels: + + %[7]s + `, "`", title, color, prompter, spinner, feedback, feedbackURL) } diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index ec6499f21..2676cdd15 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -190,6 +190,7 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) { Use %[1]sgh --help%[1]s for more information about a command. Read the manual at https://cli.github.com/manual Learn about exit codes using %[1]sgh help exit-codes%[1]s + Learn about accessibility experiences using %[1]sgh help accessibility%[1]s `, "`")}) out := f.IOStreams.Out From d8512a90666afc2e247506777b3319b2ddb820e8 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 29 Apr 2025 16:35:04 -0600 Subject: [PATCH 05/11] fix(prompter): respect default MultiSelect a11y prompter --- internal/prompter/accessible_prompter_test.go | 21 +++++++++++++++++++ internal/prompter/prompter.go | 9 ++++++++ 2 files changed, 30 insertions(+) diff --git a/internal/prompter/accessible_prompter_test.go b/internal/prompter/accessible_prompter_test.go index 619eb14f1..a7326752d 100644 --- a/internal/prompter/accessible_prompter_test.go +++ b/internal/prompter/accessible_prompter_test.go @@ -76,6 +76,27 @@ func TestAccessiblePrompter(t *testing.T) { assert.Equal(t, []int{0, 1}, multiSelectValue) }) + t.Run("MultiSelect - default values are respected by being pre-selected", func(t *testing.T) { + console := newTestVirtualTerminal(t) + p := newTestAccessiblePrompter(t, console) + + go func() { + // Wait for prompt to appear + _, err := console.ExpectString("Select a number") + require.NoError(t, err) + + // Don't select anything because the default should be selected. + + // This confirms selections + _, err = console.SendLine("0") + require.NoError(t, err) + }() + + multiSelectValue, err := p.MultiSelect("Select a number", []string{"2"}, []string{"1", "2", "3"}) + require.NoError(t, err) + assert.Equal(t, []int{1}, multiSelectValue) + }) + t.Run("Input", func(t *testing.T) { console := newTestVirtualTerminal(t) p := newTestAccessiblePrompter(t, console) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index 2a4328366..322270086 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -2,6 +2,7 @@ package prompter import ( "fmt" + "slices" "strings" "github.com/AlecAivazis/survey/v2" @@ -100,6 +101,14 @@ func (p *accessiblePrompter) MultiSelect(prompt string, defaults []string, optio var result []int formOptions := make([]huh.Option[int], len(options)) for i, o := range options { + // If this option is in the defaults slice, + // let's add it's index to the result slice and huh + // will treat it as a default selection. + // TODO: does an invalid default value constitute a panic? + if slices.Contains(defaults, o) { + result = append(result, i) + } + formOptions[i] = huh.NewOption(o, i) } From 00c930d50957c1bededbf6a40b243be4e5a71bab Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 30 Apr 2025 08:04:16 -0600 Subject: [PATCH 06/11] doc(prompter): small typo --- internal/prompter/prompter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/prompter/prompter.go b/internal/prompter/prompter.go index 2c5c221b0..1e4f5592a 100644 --- a/internal/prompter/prompter.go +++ b/internal/prompter/prompter.go @@ -102,7 +102,7 @@ func (p *accessiblePrompter) MultiSelect(prompt string, defaults []string, optio formOptions := make([]huh.Option[int], len(options)) for i, o := range options { // If this option is in the defaults slice, - // let's add it's index to the result slice and huh + // let's add its index to the result slice and huh // will treat it as a default selection. // TODO: does an invalid default value constitute a panic? if slices.Contains(defaults, o) { From 096106a3d703ce9c8177b6608b58f6338f5478f0 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 30 Apr 2025 14:20:16 -0400 Subject: [PATCH 07/11] Apply suggestions from code review Co-authored-by: Melissa Xie --- pkg/cmd/accessibility/accessibility.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/accessibility/accessibility.go b/pkg/cmd/accessibility/accessibility.go index 1d4a8009f..f05bc7bcc 100644 --- a/pkg/cmd/accessibility/accessibility.go +++ b/pkg/cmd/accessibility/accessibility.go @@ -30,7 +30,7 @@ func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "accessibility", Aliases: []string{"a11y"}, - Short: "Learn about GitHub CLI accessibility experiences", + Short: "Learn about GitHub CLI's accessibility experiences", Long: longDescription(opts.IO), Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { @@ -87,7 +87,7 @@ func longDescription(io *iostreams.IOStreams) string { Text interfaces often use color for various purposes, but insufficient contrast or customizability can leave some users unable to benefit. - To create a more accessible experience, the GitHub CLI will use color palettes + For a more accessible experience, the GitHub CLI can use color palettes based on terminal background appearance and limit colors to 4-bit ANSI color palettes, which users can customize within terminal preferences. @@ -115,8 +115,7 @@ func longDescription(io *iostreams.IOStreams) string { of the screen, which can be difficult for speech synthesizers or braille displays to accurately detect and read. - To create a more accessible experience, the GitHub CLI gives users the ability to - disable this interactivity while providing a similar experience using + For a more accessible experience, the GitHub CLI can provide a similar experience using non-interactive prompts for user input. To enable this experience, use one of the following methods: @@ -126,12 +125,11 @@ func longDescription(io *iostreams.IOStreams) string { %[5]s Motion-based spinners communicate in-progress activity by manipulating the - terminal cursor to create a spinning effect, which can be difficult for users + terminal cursor to create a spinning effect, which may cause discomfort to users with motion sensitivity or miscommunicate information to speech synthesizers. - To create a more accessible experience, the GitHub CLI gives users the ability to - disable this interactivity while providing a similar experience using text-based - progress indicators. + For a more accessible experience, this interactivity can be disabled in favor + of text-based progress indicators. To enable this experience, use one of the following methods: - Run %[1]sgh config set spinner disabled%[1]s From 2fd1a45a81ae6466dea5598ce9825f984a6fb770 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 30 Apr 2025 14:21:02 -0400 Subject: [PATCH 08/11] Update pkg/cmd/accessibility/accessibility.go --- pkg/cmd/accessibility/accessibility.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/accessibility/accessibility.go b/pkg/cmd/accessibility/accessibility.go index f05bc7bcc..d37631d25 100644 --- a/pkg/cmd/accessibility/accessibility.go +++ b/pkg/cmd/accessibility/accessibility.go @@ -69,7 +69,7 @@ func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { func longDescription(io *iostreams.IOStreams) string { cs := io.ColorScheme() - title := cs.Bold("Learn about GitHub CLI accessibility experiences") + title := cs.Bold("Learn about GitHub CLI's accessibility experiences") color := cs.Bold("Customizable and contrasting colors") prompter := cs.Bold("Non-interactive user input prompting") spinner := cs.Bold("Text-based spinners") From c20138d8442457c5c6f326fdbd13d73e47b06a32 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 30 Apr 2025 14:35:35 -0400 Subject: [PATCH 09/11] Update pkg/cmd/accessibility/accessibility.go --- pkg/cmd/accessibility/accessibility.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/cmd/accessibility/accessibility.go b/pkg/cmd/accessibility/accessibility.go index d37631d25..19fb813a4 100644 --- a/pkg/cmd/accessibility/accessibility.go +++ b/pkg/cmd/accessibility/accessibility.go @@ -50,9 +50,6 @@ func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { # Display color using customizable, 4-bit accessible colors $ gh config set accessible_colors enabled - # Display issue and pull request labels using RGB hex color codes in terminals that support 24-bit true color - $ gh config set color_labels enabled - # Use input prompts without redrawing the screen $ gh config set accessible_prompter enabled From 830335d9209d399ae1415ec927dd4d00ea1e0ab2 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 30 Apr 2025 15:05:07 -0400 Subject: [PATCH 10/11] PR feedback --- pkg/cmd/accessibility/accessibility.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/accessibility/accessibility.go b/pkg/cmd/accessibility/accessibility.go index 19fb813a4..c5de6c1a4 100644 --- a/pkg/cmd/accessibility/accessibility.go +++ b/pkg/cmd/accessibility/accessibility.go @@ -12,7 +12,7 @@ import ( ) const ( - feedbackURL = "https://accessibility.github.com/feedback" + webURL = "https://accessibility.github.com/conformance/cli/" ) type AccessibilityOptions struct { @@ -36,9 +36,9 @@ func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { if opts.Web { if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(feedbackURL)) + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL)) } - return opts.Browser.Browse(feedbackURL) + return opts.Browser.Browse(webURL) } return cmd.Help() @@ -125,7 +125,7 @@ func longDescription(io *iostreams.IOStreams) string { terminal cursor to create a spinning effect, which may cause discomfort to users with motion sensitivity or miscommunicate information to speech synthesizers. - For a more accessible experience, this interactivity can be disabled in favor + For a more accessible experience, this interactivity can be disabled in favor of text-based progress indicators. To enable this experience, use one of the following methods: @@ -138,5 +138,5 @@ func longDescription(io *iostreams.IOStreams) string { feedback and ideas through GitHub Accessibility feedback channels: %[7]s - `, "`", title, color, prompter, spinner, feedback, feedbackURL) + `, "`", title, color, prompter, spinner, feedback, webURL) } From 0a1e7a1fdc68e2824605f0c4c6dd4dbcad448888 Mon Sep 17 00:00:00 2001 From: "Sinan Sonmez (Chaush)" <37421564+sinansonmez@users.noreply.github.com> Date: Thu, 1 May 2025 15:12:55 +0200 Subject: [PATCH 11/11] Add `--delete-last` option to `pr comment` and `issue comment` (#10596) * deletion for issues with confirmation flag * add handling for interaction case * finish implementation for issues * finish the implementation for issues * finalize the implementation for PR * fix missing --yes flag for PR * address PR comments related to feedbacks * improve CommentablePreRun for pre checks * improve confirmation prompt and truncate long comment body * address PR comments on tests * Truncate comment for confirmation prompt Signed-off-by: Babak K. Shandiz * Improve test case descriptions Signed-off-by: Babak K. Shandiz * Fix mock comment body Signed-off-by: Babak K. Shandiz * Remove irrelevant prompt stub Signed-off-by: Babak K. Shandiz * Use `opts.Interactive` as TTY indicator Signed-off-by: Babak K. Shandiz * Fix expected `Interactive` value Signed-off-by: Babak K. Shandiz * Polish `TestNewCmdComment` Signed-off-by: Babak K. Shandiz --------- Signed-off-by: Babak K. Shandiz Co-authored-by: Babak K. Shandiz --- api/queries_comments.go | 25 +++ pkg/cmd/issue/comment/comment.go | 7 +- pkg/cmd/issue/comment/comment_test.go | 218 ++++++++++++++++++++++++- pkg/cmd/pr/comment/comment.go | 7 +- pkg/cmd/pr/comment/comment_test.go | 220 +++++++++++++++++++++++++- pkg/cmd/pr/shared/commentable.go | 75 +++++++++ 6 files changed, 542 insertions(+), 10 deletions(-) diff --git a/api/queries_comments.go b/api/queries_comments.go index 5cc84a3e4..8af17fd2a 100644 --- a/api/queries_comments.go +++ b/api/queries_comments.go @@ -44,6 +44,10 @@ type CommentCreateInput struct { SubjectId string } +type CommentDeleteInput struct { + CommentId string +} + type CommentUpdateInput struct { Body string CommentId string @@ -99,6 +103,27 @@ func CommentUpdate(client *Client, repoHost string, params CommentUpdateInput) ( return mutation.UpdateIssueComment.IssueComment.URL, nil } +func CommentDelete(client *Client, repoHost string, params CommentDeleteInput) error { + var mutation struct { + DeleteIssueComment struct { + ClientMutationID string + } `graphql:"deleteIssueComment(input: $input)"` + } + + variables := map[string]interface{}{ + "input": githubv4.DeleteIssueCommentInput{ + ID: githubv4.ID(params.CommentId), + }, + } + + err := client.Mutate(repoHost, "CommentDelete", &mutation, variables) + if err != nil { + return err + } + + return nil +} + func (c Comment) Identifier() string { return c.ID } diff --git a/pkg/cmd/issue/comment/comment.go b/pkg/cmd/issue/comment/comment.go index 706ff791e..9b7791656 100644 --- a/pkg/cmd/issue/comment/comment.go +++ b/pkg/cmd/issue/comment/comment.go @@ -18,6 +18,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey(f.Prompter), ConfirmCreateIfNoneSurvey: prShared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter), + ConfirmDeleteLastComment: prShared.CommentableConfirmDeleteLastComment(f.Prompter), OpenInBrowser: f.Browser.Browse, } @@ -63,7 +64,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e } fields := []string{"id", "url"} - if opts.EditLast { + if opts.EditLast || opts.DeleteLast { fields = append(fields, "comments") } @@ -96,7 +97,9 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") cmd.Flags().BoolP("editor", "e", false, "Skip prompts and open the text editor to write the body in") cmd.Flags().BoolP("web", "w", false, "Open the web browser to write the comment") - cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the same author") + cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the current user") + cmd.Flags().BoolVar(&opts.DeleteLast, "delete-last", false, "Delete the last comment of the current user") + cmd.Flags().BoolVar(&opts.DeleteLastConfirmed, "yes", false, "Skip the delete confirmation prompt when --delete-last is provided") cmd.Flags().BoolVar(&opts.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last") return cmd diff --git a/pkg/cmd/issue/comment/comment_test.go b/pkg/cmd/issue/comment/comment_test.go index 794dafda4..adee53f7e 100644 --- a/pkg/cmd/issue/comment/comment_test.go +++ b/pkg/cmd/issue/comment/comment_test.go @@ -2,6 +2,7 @@ package comment import ( "bytes" + "errors" "fmt" "net/http" "os" @@ -31,11 +32,13 @@ func TestNewCmdComment(t *testing.T) { stdin string output shared.CommentableOptions wantsErr bool + isTTY bool }{ { name: "no arguments", input: "", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { @@ -46,6 +49,7 @@ func TestNewCmdComment(t *testing.T) { InputType: 0, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -56,6 +60,7 @@ func TestNewCmdComment(t *testing.T) { InputType: 0, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -66,6 +71,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeInline, Body: "test", }, + isTTY: true, wantsErr: false, }, { @@ -77,6 +83,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeInline, Body: "this is on standard input", }, + isTTY: true, wantsErr: false, }, { @@ -87,6 +94,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeInline, Body: "a body from file", }, + isTTY: true, wantsErr: false, }, { @@ -97,6 +105,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeEditor, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -107,6 +116,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeWeb, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -118,6 +128,7 @@ func TestNewCmdComment(t *testing.T) { Body: "", EditLast: true, }, + isTTY: true, wantsErr: false, }, { @@ -130,42 +141,110 @@ func TestNewCmdComment(t *testing.T) { EditLast: true, CreateIfNone: true, }, + isTTY: true, wantsErr: false, }, + { + name: "delete last flag non-interactive", + input: "1 --delete-last", + isTTY: false, + wantsErr: true, + }, + { + name: "delete last flag and pre-confirmation non-interactive", + input: "1 --delete-last --yes", + output: shared.CommentableOptions{ + DeleteLast: true, + DeleteLastConfirmed: true, + }, + isTTY: false, + wantsErr: false, + }, + { + name: "delete last flag interactive", + input: "1 --delete-last", + output: shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + }, + isTTY: true, + wantsErr: false, + }, + { + name: "delete last flag and pre-confirmation interactive", + input: "1 --delete-last --yes", + output: shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + DeleteLastConfirmed: true, + }, + isTTY: true, + wantsErr: false, + }, + { + name: "delete last flag and pre-confirmation with web flag", + input: "1 --delete-last --yes --web", + isTTY: true, + wantsErr: true, + }, + { + name: "delete last flag and pre-confirmation with editor flag", + input: "1 --delete-last --yes --editor", + isTTY: true, + wantsErr: true, + }, + { + name: "delete last flag and pre-confirmation with body flag", + input: "1 --delete-last --yes --body", + isTTY: true, + wantsErr: true, + }, + { + name: "delete pre-confirmation without delete last flag", + input: "1 --yes", + isTTY: true, + wantsErr: true, + }, { name: "body and body-file flags", input: "1 --body 'test' --body-file 'test-file.txt'", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "editor and web flags", input: "1 --editor --web", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "editor and body flags", input: "1 --editor --body test", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "web and body flags", input: "1 --web --body test", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "editor, web, and body flags", input: "1 --editor --web --body test", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "create-if-none flag without edit-last", input: "1 --create-if-none", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, } @@ -173,9 +252,10 @@ func TestNewCmdComment(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ios, stdin, _, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStdinTTY(true) - ios.SetStderrTTY(true) + isTTY := tt.isTTY + ios.SetStdoutTTY(isTTY) + ios.SetStdinTTY(isTTY) + ios.SetStderrTTY(isTTY) if tt.stdin != "" { _, _ = stdin.WriteString(tt.stdin) @@ -211,6 +291,8 @@ func TestNewCmdComment(t *testing.T) { assert.Equal(t, tt.output.Interactive, gotOpts.Interactive) assert.Equal(t, tt.output.InputType, gotOpts.InputType) assert.Equal(t, tt.output.Body, gotOpts.Body) + assert.Equal(t, tt.output.DeleteLast, gotOpts.DeleteLast) + assert.Equal(t, tt.output.DeleteLastConfirmed, gotOpts.DeleteLastConfirmed) }) } } @@ -220,6 +302,7 @@ func Test_commentRun(t *testing.T) { name string input *shared.CommentableOptions emptyComments bool + comments api.Comments httpStubs func(*testing.T, *httpmock.Registry) stdout string stderr string @@ -255,6 +338,7 @@ func Test_commentRun(t *testing.T) { }, emptyComments: true, wantsErr: true, + stdout: "no comments found for current user", }, { name: "updating last comment with interactive editor succeeds if there are comments", @@ -331,6 +415,7 @@ func Test_commentRun(t *testing.T) { }, emptyComments: true, wantsErr: true, + stdout: "no comments found for current user", }, { name: "creating new comment with non-interactive editor succeeds", @@ -358,6 +443,7 @@ func Test_commentRun(t *testing.T) { }, emptyComments: true, wantsErr: true, + stdout: "no comments found for current user", }, { name: "updating last comment with non-interactive editor succeeds if there are comments", @@ -433,6 +519,117 @@ func Test_commentRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n", }, + { + name: "deleting last comment non-interactively without any comment", + input: &shared.CommentableOptions{ + Interactive: false, + DeleteLast: true, + }, + emptyComments: true, + wantsErr: true, + stdout: "no comments found for current user", + }, + { + name: "deleting last comment interactively without any comment", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + }, + emptyComments: true, + wantsErr: true, + stdout: "no comments found for current user", + }, + { + name: "deleting last comment non-interactively and pre-confirmed", + input: &shared.CommentableOptions{ + Interactive: false, + DeleteLast: true, + DeleteLastConfirmed: true, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + stderr: "Comment deleted\n", + }, + { + name: "deleting last comment interactively and pre-confirmed", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + DeleteLastConfirmed: true, + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"}, + }}, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + stderr: "Comment deleted\n", + }, + { + name: "deleting last comment interactively and confirmed", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + + ConfirmDeleteLastComment: func(body string) (bool, error) { + if body != "comment body" { + return false, errors.New("unexpected comment body") + } + return true, nil + }, + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"}, + }}, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + stdout: "! Deleted comments cannot be recovered.\n", + stderr: "Comment deleted\n", + }, + { + name: "deleting last comment interactively and confirmation declined", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + + ConfirmDeleteLastComment: func(body string) (bool, error) { + if body != "comment body" { + return false, errors.New("unexpected comment body") + } + return true, nil + }, + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"}, + }}, + wantsErr: true, + stdout: "deletion not confirmed", + }, + { + name: "deleting last comment interactively and confirmed with long comment body", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + + ConfirmDeleteLastComment: func(body string) (bool, error) { + if body != "Lorem ipsum dolor sit amet, consectet lo..." { + return false, errors.New("unexpected comment body") + } + return true, nil + }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "Lorem ipsum dolor sit amet, consectet lorem ipsum again"}, + }}, + wantsErr: false, + stdout: "! Deleted comments cannot be recovered.\n", + stderr: "Comment deleted\n", + }, } for _, tt := range tests { ios, _, stdout, stderr := iostreams.Test() @@ -458,6 +655,8 @@ func Test_commentRun(t *testing.T) { if tt.emptyComments { comments.Nodes = []api.Comment{} + } else if len(tt.comments.Nodes) > 0 { + comments = tt.comments } tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) { @@ -472,6 +671,7 @@ func Test_commentRun(t *testing.T) { err := shared.CommentableRun(tt.input) if tt.wantsErr { assert.Error(t, err) + assert.Equal(t, tt.stderr, stderr.String()) return } assert.NoError(t, err) @@ -508,3 +708,15 @@ func mockCommentUpdate(t *testing.T, reg *httpmock.Registry) { }), ) } + +func mockCommentDelete(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation CommentDelete\b`), + httpmock.GraphQLMutation(` + { "data": { "deleteIssueComment": {} } }`, + func(inputs map[string]interface{}) { + assert.Equal(t, "id1", inputs["id"]) + }, + ), + ) +} diff --git a/pkg/cmd/pr/comment/comment.go b/pkg/cmd/pr/comment/comment.go index a2ab4bf9e..2eed7d353 100644 --- a/pkg/cmd/pr/comment/comment.go +++ b/pkg/cmd/pr/comment/comment.go @@ -16,6 +16,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err InteractiveEditSurvey: shared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey(f.Prompter), ConfirmCreateIfNoneSurvey: shared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter), + ConfirmDeleteLastComment: shared.CommentableConfirmDeleteLastComment(f.Prompter), OpenInBrowser: f.Browser.Browse, } @@ -43,7 +44,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err selector = args[0] } fields := []string{"id", "url"} - if opts.EditLast { + if opts.EditLast || opts.DeleteLast { fields = append(fields, "comments") } finder := shared.NewFinder(f) @@ -75,7 +76,9 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") cmd.Flags().BoolP("editor", "e", false, "Skip prompts and open the text editor to write the body in") cmd.Flags().BoolP("web", "w", false, "Open the web browser to write the comment") - cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the same author") + cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the current user") + cmd.Flags().BoolVar(&opts.DeleteLast, "delete-last", false, "Delete the last comment of the current user") + cmd.Flags().BoolVar(&opts.DeleteLastConfirmed, "yes", false, "Skip the delete confirmation prompt when --delete-last is provided") cmd.Flags().BoolVar(&opts.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last") return cmd diff --git a/pkg/cmd/pr/comment/comment_test.go b/pkg/cmd/pr/comment/comment_test.go index 0941f2533..b9d8e153d 100644 --- a/pkg/cmd/pr/comment/comment_test.go +++ b/pkg/cmd/pr/comment/comment_test.go @@ -2,6 +2,7 @@ package comment import ( "bytes" + "errors" "fmt" "net/http" "os" @@ -31,6 +32,7 @@ func TestNewCmdComment(t *testing.T) { stdin string output shared.CommentableOptions wantsErr bool + isTTY bool }{ { name: "no arguments", @@ -40,12 +42,14 @@ func TestNewCmdComment(t *testing.T) { InputType: 0, Body: "", }, + isTTY: true, wantsErr: false, }, { name: "two arguments", input: "1 2", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { @@ -56,6 +60,7 @@ func TestNewCmdComment(t *testing.T) { InputType: 0, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -66,6 +71,7 @@ func TestNewCmdComment(t *testing.T) { InputType: 0, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -76,6 +82,7 @@ func TestNewCmdComment(t *testing.T) { InputType: 0, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -86,6 +93,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeInline, Body: "test", }, + isTTY: true, wantsErr: false, }, { @@ -97,6 +105,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeInline, Body: "this is on standard input", }, + isTTY: true, wantsErr: false, }, { @@ -107,6 +116,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeInline, Body: "a body from file", }, + isTTY: true, wantsErr: false, }, { @@ -117,6 +127,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeEditor, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -127,6 +138,7 @@ func TestNewCmdComment(t *testing.T) { InputType: shared.InputTypeWeb, Body: "", }, + isTTY: true, wantsErr: false, }, { @@ -138,6 +150,7 @@ func TestNewCmdComment(t *testing.T) { Body: "", EditLast: true, }, + isTTY: true, wantsErr: false, }, { @@ -150,42 +163,110 @@ func TestNewCmdComment(t *testing.T) { EditLast: true, CreateIfNone: true, }, + isTTY: true, wantsErr: false, }, + { + name: "delete last flag non-interactive", + input: "1 --delete-last", + isTTY: false, + wantsErr: true, + }, + { + name: "delete last flag and pre-confirmation non-interactive", + input: "1 --delete-last --yes", + output: shared.CommentableOptions{ + DeleteLast: true, + DeleteLastConfirmed: true, + }, + isTTY: false, + wantsErr: false, + }, + { + name: "delete last flag interactive", + input: "1 --delete-last", + output: shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + }, + isTTY: true, + wantsErr: false, + }, + { + name: "delete last flag and pre-confirmation interactive", + input: "1 --delete-last --yes", + output: shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + DeleteLastConfirmed: true, + }, + isTTY: true, + wantsErr: false, + }, + { + name: "delete last flag and pre-confirmation with web flag", + input: "1 --delete-last --yes --web", + isTTY: true, + wantsErr: true, + }, + { + name: "delete last flag and pre-confirmation with editor flag", + input: "1 --delete-last --yes --editor", + isTTY: true, + wantsErr: true, + }, + { + name: "delete last flag and pre-confirmation with body flag", + input: "1 --delete-last --yes --body", + isTTY: true, + wantsErr: true, + }, + { + name: "delete pre-confirmation without delete last flag", + input: "1 --yes", + isTTY: true, + wantsErr: true, + }, { name: "body and body-file flags", input: "1 --body 'test' --body-file 'test-file.txt'", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "editor and web flags", input: "1 --editor --web", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "editor and body flags", input: "1 --editor --body test", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "web and body flags", input: "1 --web --body test", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "editor, web, and body flags", input: "1 --editor --web --body test", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, { name: "create-if-none flag without edit-last", input: "1 --create-if-none", output: shared.CommentableOptions{}, + isTTY: true, wantsErr: true, }, } @@ -193,9 +274,10 @@ func TestNewCmdComment(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ios, stdin, _, _ := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStdinTTY(true) - ios.SetStderrTTY(true) + isTTY := tt.isTTY + ios.SetStdoutTTY(isTTY) + ios.SetStdinTTY(isTTY) + ios.SetStderrTTY(isTTY) if tt.stdin != "" { _, _ = stdin.WriteString(tt.stdin) @@ -231,6 +313,8 @@ func TestNewCmdComment(t *testing.T) { assert.Equal(t, tt.output.Interactive, gotOpts.Interactive) assert.Equal(t, tt.output.InputType, gotOpts.InputType) assert.Equal(t, tt.output.Body, gotOpts.Body) + assert.Equal(t, tt.output.DeleteLast, gotOpts.DeleteLast) + assert.Equal(t, tt.output.DeleteLastConfirmed, gotOpts.DeleteLastConfirmed) }) } } @@ -240,6 +324,7 @@ func Test_commentRun(t *testing.T) { name string input *shared.CommentableOptions emptyComments bool + comments api.Comments httpStubs func(*testing.T, *httpmock.Registry) stdout string stderr string @@ -274,6 +359,7 @@ func Test_commentRun(t *testing.T) { }, emptyComments: true, wantsErr: true, + stdout: "no comments found for current user", }, { name: "updating last comment with interactive editor succeeds if there are comments", @@ -350,6 +436,7 @@ func Test_commentRun(t *testing.T) { }, emptyComments: true, wantsErr: true, + stdout: "no comments found for current user", }, { name: "creating new comment with non-interactive editor succeeds", @@ -377,6 +464,7 @@ func Test_commentRun(t *testing.T) { }, emptyComments: true, wantsErr: true, + stdout: "no comments found for current user", }, { name: "updating last comment with non-interactive editor succeeds if there are comments", @@ -451,6 +539,117 @@ func Test_commentRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-456\n", }, + { + name: "deleting last comment non-interactively without any comment", + input: &shared.CommentableOptions{ + Interactive: false, + DeleteLast: true, + }, + emptyComments: true, + wantsErr: true, + stdout: "no comments found for current user", + }, + { + name: "deleting last comment interactively without any comment", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + }, + emptyComments: true, + wantsErr: true, + stdout: "no comments found for current user", + }, + { + name: "deleting last comment non-interactively and pre-confirmed", + input: &shared.CommentableOptions{ + Interactive: false, + DeleteLast: true, + DeleteLastConfirmed: true, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + stderr: "Comment deleted\n", + }, + { + name: "deleting last comment interactively and pre-confirmed", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + DeleteLastConfirmed: true, + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"}, + }}, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + stderr: "Comment deleted\n", + }, + { + name: "deleting last comment interactively and confirmed", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + + ConfirmDeleteLastComment: func(body string) (bool, error) { + if body != "comment body" { + return false, errors.New("unexpected comment body") + } + return true, nil + }, + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"}, + }}, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + stdout: "! Deleted comments cannot be recovered.\n", + stderr: "Comment deleted\n", + }, + { + name: "deleting last comment interactively and confirmation declined", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + + ConfirmDeleteLastComment: func(body string) (bool, error) { + if body != "comment body" { + return false, errors.New("unexpected comment body") + } + return true, nil + }, + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"}, + }}, + wantsErr: true, + stdout: "deletion not confirmed", + }, + { + name: "deleting last comment interactively and confirmed with long comment body", + input: &shared.CommentableOptions{ + Interactive: true, + DeleteLast: true, + + ConfirmDeleteLastComment: func(body string) (bool, error) { + if body != "Lorem ipsum dolor sit amet, consectet lo..." { + return false, errors.New("unexpected comment body") + } + return true, nil + }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentDelete(t, reg) + }, + comments: api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "Lorem ipsum dolor sit amet, consectet lorem ipsum again"}, + }}, + wantsErr: false, + stdout: "! Deleted comments cannot be recovered.\n", + stderr: "Comment deleted\n", + }, } for _, tt := range tests { ios, _, stdout, stderr := iostreams.Test() @@ -475,6 +674,8 @@ func Test_commentRun(t *testing.T) { }} if tt.emptyComments { comments.Nodes = []api.Comment{} + } else if len(tt.comments.Nodes) > 0 { + comments = tt.comments } tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) { @@ -489,6 +690,7 @@ func Test_commentRun(t *testing.T) { err := shared.CommentableRun(tt.input) if tt.wantsErr { assert.Error(t, err) + assert.Equal(t, tt.stderr, stderr.String()) return } assert.NoError(t, err) @@ -524,3 +726,15 @@ func mockCommentUpdate(t *testing.T, reg *httpmock.Registry) { }), ) } + +func mockCommentDelete(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation CommentDelete\b`), + httpmock.GraphQLMutation(` + { "data": { "deleteIssueComment": {} } }`, + func(inputs map[string]interface{}) { + assert.Equal(t, "id1", inputs["id"]) + }, + ), + ) +} diff --git a/pkg/cmd/pr/shared/commentable.go b/pkg/cmd/pr/shared/commentable.go index f909c7559..015d84a4b 100644 --- a/pkg/cmd/pr/shared/commentable.go +++ b/pkg/cmd/pr/shared/commentable.go @@ -18,6 +18,7 @@ import ( ) var errNoUserComments = errors.New("no comments found for current user") +var errDeleteNotConfirmed = errors.New("deletion not confirmed") type InputType int @@ -41,11 +42,14 @@ type CommentableOptions struct { InteractiveEditSurvey func(string) (string, error) ConfirmSubmitSurvey func() (bool, error) ConfirmCreateIfNoneSurvey func() (bool, error) + ConfirmDeleteLastComment func(string) (bool, error) OpenInBrowser func(string) error Interactive bool InputType InputType Body string EditLast bool + DeleteLast bool + DeleteLastConfirmed bool CreateIfNone bool Quiet bool Host string @@ -74,6 +78,21 @@ func CommentablePreRun(cmd *cobra.Command, opts *CommentableOptions) error { return cmdutil.FlagErrorf("`--create-if-none` can only be used with `--edit-last`") } + if opts.DeleteLastConfirmed && !opts.DeleteLast { + return cmdutil.FlagErrorf("`--yes` should only be used with `--delete-last`") + } + + if opts.DeleteLast { + if inputFlags > 0 { + return cmdutil.FlagErrorf("should not provide comment body when using `--delete-last`") + } + if opts.IO.CanPrompt() || opts.DeleteLastConfirmed { + opts.Interactive = opts.IO.CanPrompt() + return nil + } + return cmdutil.FlagErrorf("should provide `--yes` to confirm deletion in non-interactive mode") + } + if inputFlags == 0 { if !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("flags required when not running interactively") @@ -92,6 +111,9 @@ func CommentableRun(opts *CommentableOptions) error { return err } opts.Host = repo.RepoHost() + if opts.DeleteLast { + return deleteComment(commentable, opts) + } // Create new comment, bail before complexities of updating the last comment if !opts.EditLast { @@ -236,6 +258,53 @@ func updateComment(commentable Commentable, opts *CommentableOptions) error { return nil } +func deleteComment(commentable Commentable, opts *CommentableOptions) error { + comments := commentable.CurrentUserComments() + if len(comments) == 0 { + return errNoUserComments + } + + lastComment := comments[len(comments)-1] + + cs := opts.IO.ColorScheme() + + if opts.Interactive && !opts.DeleteLastConfirmed { + // This is not an ideal way of truncating a random string that may + // contain emojis or other kind of wide chars. + truncated := lastComment.Body + if len(lastComment.Body) > 40 { + truncated = lastComment.Body[:40] + "..." + } + + fmt.Fprintf(opts.IO.Out, "%s Deleted comments cannot be recovered.\n", cs.WarningIcon()) + ok, err := opts.ConfirmDeleteLastComment(truncated) + if err != nil { + return err + } + if !ok { + return errDeleteNotConfirmed + } + } + + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + apiClient := api.NewClientFromHTTP(httpClient) + params := api.CommentDeleteInput{CommentId: lastComment.Identifier()} + deletionErr := api.CommentDelete(apiClient, opts.Host, params) + if deletionErr != nil { + return deletionErr + } + + if !opts.Quiet { + fmt.Fprintln(opts.IO.ErrOut, "Comment deleted") + } + + return nil +} + func CommentableConfirmSubmitSurvey(p Prompt) func() (bool, error) { return func() (bool, error) { return p.Confirm("Submit?", true) @@ -271,6 +340,12 @@ func CommentableEditSurvey(cf func() (gh.Config, error), io *iostreams.IOStreams } } +func CommentableConfirmDeleteLastComment(p Prompt) func(string) (bool, error) { + return func(body string) (bool, error) { + return p.Confirm(fmt.Sprintf("Delete the comment: %q?", body), true) + } +} + func waitForEnter(r io.Reader) error { scanner := bufio.NewScanner(r) scanner.Scan()