fix: respect edit-last and create-if-none in non-interactive mode - #10582
#10582fix: respect edit-last and create-if-none in non-interactive mode#10582GriceTurrble wants to merge 1 commit into
Conversation
…, instead of creating new comments automatically
|
@GriceTurrble : thank you for working to improve the GitHub CLI! ❤️ Generally, we wait until an issue has been fully triaged with acceptance criteria and marked as I might also suggest that the current conditional logic structure of Current logic cli/pkg/cmd/pr/shared/commentable.go Lines 89 to 118 in 7299a52 Previous logic cli/pkg/cmd/pr/shared/commentable.go Lines 81 to 91 in cdb44f8 For now, I'm closing this pull request so we can understand exactly what should change here. 🙇 |
|
I'm not saying this is exactly what I would do but want to provide context of what might be easier to understand and manage: func CommentableRun(opts *CommentableOptions) error {
commentable, repo, err := opts.RetrieveCommentable()
if err != nil {
return err
}
opts.Host = repo.RepoHost()
// Create new comment, bail before complexities of updating the last comment
if !opts.EditLast {
return createComment(commentable, opts)
}
// Update the last comment, handling success or unexpected errors accordingly
err := updateComment(commentable, opts)
if err == nil {
return nil
}
if !errors.Is(err, errNoUserComments) {
return err
}
// Determine whether to create new comment, prompt user if interactive and missing option
if !opts.CreateIfNone && opts.Interactive {
opts.CreateIfNone, err = opts.ConfirmCreateIfNoneSurvey()
if err != nil {
return err
}
}
// Create new comment because updating the last comment failed due to no user comments
if opts.CreateIfNone {
if opts.Interactive {
fmt.Fprintln(opts.IO.ErrOut, "No comments found. Creating a new comment.")
}
return createComment(commentable, opts)
}
return errNoUserComments
} |
Fixes #10580
Previous changes made in #10427 created a behavioral change when using i.e.
gh pr comment --edit-lastin non-interactive mode, which was not properly accounted for. Ideally, using--edit-lastshould error out if the newer--create-if-noneflag is not used, instead of creating a new comment anyway.Assistance on updating tests would be appreciated, this is my first time editing Golang code. :)Edit: tested locally, should be working now. And the changes are much smaller than originally pushed. 🙂