[gh issue/pr comment] Add support for last comment delete for issues and PRs by sinansonmez · Pull Request #10596 · cli/cli

sinansonmez · Mar 13, 2025

Add --delete-last option to issue/pr comment.
This option deletes the last comment that you are the author of

sinansonmez · Mar 16, 2025


+func CommentDelete(client *Client, repoHost string, params CommentDeleteInput) (error) {
+	var mutation struct {
+		DeleteIssueComment struct {


1 clarification. The mutation mention issue in the name. However, it works both for issues and PRs. Is it OK to use for both? Or for PRs should I use something else?

babakks

Great PR! Thanks @sinansonmez! It looks very good, just needs some changes.

I also concerned about how to display the comment for confirmation, especially for long/multiline comments, but that's another comment that needs some discussion.

babakks · Apr 16, 2025

While reviewing this, I was wondering "what should we do when confirming the operation?". Probably one of these is the answer:

Ask "Delete comment?".
- This doesn't help much because the user might want to see the comment before proceeding.
Ask "Delete comment: "the entire comment text"?
- Long/multiline comments mess up with this one.
Ask "Delete comment: "truncated comment text..."?
- Multiline comments can still mess with this, unless we quote them (i.e., printf with "%q").
Print the comment text and then a short prompt like "Delete comment?"?
- How about markdown rendering? do we need it?

@williammartin WDYT?

babakks

Thanks for the changes, @sinansonmez!

Could you also please run gofmt to make sure it's formatted as it should be?

Have you already tried the --delete-last and --yes options together on a real issue/PR? Because, as I'm trying, I still get the prompt:

$ go run ./cmd/gh pr comment -R babakks/dummy-gh-pr 1 --delete-last --yes
! Deleted comments cannot be recovered.
? Delete the comment: This is a test comment.? Yes

(Feel free to try it on my test repo above.)

Also, running it with a non-TTY stdout results in an error:

$ go run ./cmd/gh pr comment -R babakks/dummy-gh-pr 1 --delete-last --yes | cat
flags required when not running interactively
...

As I mentioned before, the CommentablePreRun function is responsible for flag/option validation and it needs to be updated with the new changes. This also means we need to revisit the tests, but let's do that as the final step.

sinansonmez · Apr 21, 2025

@babakks thanks for the 2nd round of review.

When you reviewed it, my changes were not final. I hope it is now ready for your review. I also tested it by using one of my test repo, it seems like it is working as expected now.

As suggested

I improved CommentablePreRun to check non-TTY and DeleteLastConfirmed
I also improved TestNewCmdComment by adding isTTY to have a finer control over the environment (i.e. TTY or no-TTY) that we are running tests

Let me know if there is any place which could be improved

babakks

Thanks for the changes! 🙏 And sorry for the premature review. My bad.

Now we need to polish the tests and make sure the behaviour is stable enough. So close to land this. 🎉

I went over the pr comment tests and put a bunch of comments. The issue comment tests need the same changes, so I didn't duplicate my comments.

Note that I've also asked for truncating long comments, because I think it's important to have UX and accessibility in our sight. Anyway, let me know if I can help with impl or writing tests for it (or any other part of the changes). I'd be happy to help.

sinansonmez · Apr 29, 2025

@babakks Thanks for such a deep review. hopefully, I addressed all of your points. Please let me know if I missed something or addressed something incorrectly

babakks

Thanks for the changes, @sinansonmez! and also for picking up this issue. 🙏

I have a couple of comments which I'll just now apply myself and go for merging this PR.

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks · May 1, 2025

I did a bunch of small/cosmetic changes and I'm now waiting for the CI to finish its job.

sinansonmez · May 1, 2025

@babakks Thanks for doing the last mile changes directly and your support during review.

btw I am willing to take more issues. I checked help-wanted issues but there isn’t any available one currently. Feel free to assign if something comes up

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks · May 1, 2025

@sinansonmez No worries. It's my pleasure. ❤️

We're currently reviewing our internal process about help-wanted issues and decided to temporarily put a hold on them. Hopefully, you'll soon see new help-wanted issues popping up.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.72.0` -> `v2.73.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.73.0`](https://github.com/cli/cli/releases/tag/v2.73.0): GitHub CLI 2.73.0 [Compare Source](cli/cli@v2.72.0...v2.73.0) ####

Copilot Coding Agent Support You can now assign issues to GitHub Copilot directly from `gh`, just as you would assign them to a teammate. Use `gh issue edit <number> --add-assignee @copilot` to assign the GitHub Copilot coding agent, and Copilot will work in the background to understand the issue, propose a solution, and open a pull request when it's ready for your review. If you run `gh issue edit` interactively, `Copilot (AI)` will be displayed as a potential assignee. This feature is available for GitHub Copilot Pro+ and Copilot Enterprise subscribers. For more details, refer to [the full changelog post for Copilot coding agent](https://github.blog/changelog/2025-05-19-github-copilot-coding-agent-in-public-preview/). #### What's Changed ##### ✨ Features - Copilot is assignable to issues and pull requests with `issue edit` and `pr edit` by [@BagToad](https://github.com/BagToad) in cli/cli#10992 - `gh issue edit`: actors are assignable to issues by [@BagToad](https://github.com/BagToad) in cli/cli#10960 - `gh pr edit`: Assign actors to pull requests by [@BagToad](https://github.com/BagToad) in cli/cli#10984 - `issue edit`, `pr edit`: handle display names in interactive assignee editing by [@BagToad](https://github.com/BagToad) in cli/cli#10990 - `issue edit`, `pr edit`: Support special non-interactive (flags) assignee name `@copilot` by [@BagToad](https://github.com/BagToad) in cli/cli#10991 - \[gh issue/pr comment] Add support for last comment delete for issues and MRs by [@sinansonmez](https://github.com/sinansonmez) in cli/cli#10596 - \[gh issue view] Expose `closedByPullRequestsReferences` JSON field by [@iamazeem](https://github.com/iamazeem) in cli/cli#10941 - Accessible prompter always displays selection defaults in a format readable by a screen reader by [@BagToad](https://github.com/BagToad) in cli/cli#10937 ##### 🐛 Fixes - Fix `StatusJSONResponse` usage by [@babakks](https://github.com/babakks) in cli/cli#10810 - Fix panic on `gh pr view 0` by [@nopcoder](https://github.com/nopcoder) in cli/cli#10729 - Fix flakey test for accessible prompter by [@BagToad](https://github.com/BagToad) in cli/cli#10918 - Fix accessible prompter flaky tests by [@babakks](https://github.com/babakks) in cli/cli#10977 - Handle missing archive URLs on release download by [@williammartin](https://github.com/williammartin) in cli/cli#10947 - Fix bug when removing all MR reviewers by [@babakks](https://github.com/babakks) in cli/cli#10975 ##### 📚 Docs & Chores - Feature detect v1 projects on pr view by [@williammartin](https://github.com/williammartin) in cli/cli#10821 - Feature detect v1 projects on non-interactive pr create by [@williammartin](https://github.com/williammartin) in cli/cli#10909 - Feature detect v1 projects on web mode pr create by [@williammartin](https://github.com/williammartin) in cli/cli#10911 - Feature detect v1 projects on interactive `pr create` by [@williammartin](https://github.com/williammartin) in cli/cli#10915 - Feature detect v1 projects on pr edit by [@williammartin](https://github.com/williammartin) in cli/cli#10942 - Move predicate type filtering in `gh attestation verify` by [@malancas](https://github.com/malancas) in cli/cli#10670 - Improve assertion for disabled echo mode by [@babakks](https://github.com/babakks) in cli/cli#10927 #####

Dependencies - chore(deps): bump actions/attest-build-provenance from 2.2.2 to 2.3.0 by [@dependabot](https://github.com/dependabot) in cli/cli#10886 - chore(deps): bump github.com/cpuguy83/go-md2man/v2 from 2.0.6 to 2.0.7 by [@dependabot](https://github.com/dependabot) in cli/cli#10869 #### What's Changed #### New Contributors - [@sinansonmez](https://github.com/sinansonmez) made their first contribution in cli/cli#10596 - [@nopcoder](https://github.com/nopcoder) made their first contribution in cli/cli#10729 **Full Changelog**: cli/cli@v2.72.0...v2.73.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

sinansonmez added 2 commits March 13, 2025 22:53

deletion for issues with confirmation flag

bd6c084

add handling for interaction case

1a38559

sinansonmez temporarily deployed to cli-automation March 13, 2025 23:25 — with GitHub Actions Inactive

sinansonmez changed the title ~~[gh issue/pr comment] Feature/10366/issue comment delete last~~ [gh issue/pr comment] Add support for last comment delete for issues and PRs Mar 13, 2025

sinansonmez added 4 commits March 14, 2025 17:37

finish implementation for issues

1e24644

finish the implementation for issues

1d8e9ef

finalize the implementation for PR

37b2c58

fix missing --yes flag for PR

15ae972

sinansonmez marked this pull request as ready for review March 14, 2025 22:26

sinansonmez requested a review from a team as a code owner March 14, 2025 22:26

sinansonmez requested a review from williammartin March 14, 2025 22:26

sinansonmez temporarily deployed to cli-automation March 14, 2025 22:26 — with GitHub Actions Inactive

cliAutomation added the external pull request originating outside of the CLI core team label Mar 14, 2025

sinansonmez commented Mar 16, 2025

View reviewed changes

babakks self-requested a review April 16, 2025 12:13

babakks reviewed Apr 16, 2025

View reviewed changes

sinansonmez and others added 2 commits April 16, 2025 21:20

Merge branch 'trunk' into feature/10366/issue-comment-delete-last

184d9ec

address PR comments related to feedbacks

51e50e5

babakks reviewed Apr 17, 2025

View reviewed changes

Comment thread pkg/cmd/pr/comment/comment.go Outdated

Comment thread pkg/cmd/pr/comment/comment.go Outdated

Comment thread pkg/cmd/pr/shared/commentable.go Outdated

sinansonmez and others added 2 commits April 21, 2025 15:20

improve CommentablePreRun for pre checks

ee921ee

Merge branch 'trunk' into feature/10366/issue-comment-delete-last

241af93

babakks reviewed Apr 29, 2025

View reviewed changes

sinansonmez added 2 commits April 29, 2025 19:11

improve confirmation prompt and truncate long comment body

7225b88

address PR comments on tests

fcd5201

sinansonmez requested a review from babakks April 29, 2025 21:19

babakks reviewed May 1, 2025

View reviewed changes

Comment thread pkg/cmd/issue/comment/comment_test.go Outdated

Comment thread pkg/cmd/pr/comment/comment_test.go Outdated

Comment thread pkg/cmd/pr/shared/commentable.go Outdated

Truncate comment for confirmation prompt

ba857b3

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks added 4 commits May 1, 2025 12:04

Improve test case descriptions

0361970

Signed-off-by: Babak K. Shandiz <babakks@github.com>

Fix mock comment body

3b0fae1

Signed-off-by: Babak K. Shandiz <babakks@github.com>

Remove irrelevant prompt stub

f626950

Signed-off-by: Babak K. Shandiz <babakks@github.com>

Use opts.Interactive as TTY indicator

a55f8a2

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks added 2 commits May 1, 2025 12:51

Fix expected Interactive value

ffabed2

Signed-off-by: Babak K. Shandiz <babakks@github.com>

Polish TestNewCmdComment

39ccaf1

Signed-off-by: Babak K. Shandiz <babakks@github.com>

babakks approved these changes May 1, 2025

View reviewed changes

babakks merged commit 0a1e7a1 into cli:trunk May 1, 2025

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

sinansonmez commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sinansonmez Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babakks commented Apr 16, 2025

Uh oh!

babakks left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sinansonmez commented Apr 21, 2025

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sinansonmez commented Apr 29, 2025

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babakks commented May 1, 2025

Uh oh!

sinansonmez commented May 1, 2025

Uh oh!

babakks commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sinansonmez commented Mar 13, 2025 •

edited

Loading

babakks left a comment •

edited

Loading