diff --git a/.changes/unreleased/Added-20260314-155204.yaml b/.changes/unreleased/Added-20260314-155204.yaml new file mode 100644 index 000000000..54a978f25 --- /dev/null +++ b/.changes/unreleased/Added-20260314-155204.yaml @@ -0,0 +1,3 @@ +kind: Added +body: 'Add commands for drafting, publishing, and managing review comments' +time: 2026-03-14T15:52:04.220487-04:00 diff --git a/doc/includes/cli-reference.md b/doc/includes/cli-reference.md index 7255a6cb9..f8657355d 100644 --- a/doc/includes/cli-reference.md +++ b/doc/includes/cli-reference.md @@ -1465,6 +1465,183 @@ This command requires at least Git 2.45. **Configuration**: [spice.commitPick.restack](/cli/config.md#spicecommitpickrestack) +## Review + +### git-spice review comment {#gs-review-comment} + +``` +gs review comment [] [flags] +``` + +Draft or post a review comment + +Adds a review comment to the change request +for the current branch. +The anchor controls the comment scope: + + file.go:42 anchored to that line + file.go:42-50 anchored to that line range + file.go anchored to the file + +Comments are saved as local drafts by default. +Use --no-draft to post immediately. + +If no message is given with -m, an editor is opened. + +**Arguments** + +* `anchor`: Comment anchor: file.go, file.go:42, or file.go:42-50. + +**Flags** + +* `-m`, `--message=MSG`: Comment body. Opens editor if not provided. +* `--[no-]draft`: Save the comment as a local draft instead of posting it. +* `-b`, `--branch=BRANCH`: Branch to comment on. Defaults to the current branch. + +### git-spice review reply {#gs-review-reply} + +``` +gs review reply [flags] +``` + +Draft or post a reply to a review thread + +Replies to a review thread on the change request +for the current branch. + +Replies are saved as local drafts by default. +Use --no-draft to post immediately. + +If no message is given with -m, an editor is opened. + +**Arguments** + +* `thread-id`: Thread ID to reply to. + +**Flags** + +* `-m`, `--message=MSG`: Reply body. Opens editor if not provided. +* `--[no-]draft`: Save the reply as a local draft instead of posting it. +* `-b`, `--branch=BRANCH`: Branch containing the thread. Defaults to the current branch. + +### git-spice review publish {#gs-review-publish} + +``` +gs review publish [flags] +``` + +Publish draft comments as a review + +Publishes all draft comments for the current branch +as a single review on the change request. + +Use --approve or --request-changes +to set the review event type. +Defaults to a comment-only review. + +Use --body to add an overall review body. + +**Flags** + +* `--body=BODY`: Overall review body. +* `--approve`: Mark the review as approved. +* `--request-changes`: Mark the review as requesting changes. +* `-b`, `--branch=BRANCH`: Branch whose draft comments to publish. Defaults to the current branch. + +### git-spice review list {#gs-review-list} + +``` +gs review list (ls) [flags] +``` + +List review comments + +Lists comments on the change request +associated with the current branch. +Use --branch to target a different branch. + +Draft comments are identified by a branch-local integer. + +Use --draft-only to show only draft comments. +Use --unresolved to show only unresolved comments. + +With --json, prints output to stdout +as a stream of JSON objects. + +**Flags** + +* `-b`, `--branch=BRANCH`: Branch to list comments for. Defaults to the current branch. +* `--draft-only`: Show only draft comments. +* `--unresolved`: Show only unresolved comments. +* `--json`: Write to stdout as a stream of JSON objects. :material-tag-hidden:{ title="Released in version" }Unreleased + +### git-spice review edit {#gs-review-edit} + +``` +gs review edit [flags] +``` + +Edit a draft comment + +Edits a local draft comment. + +Use 'gs review list --draft-only' +to find the branch-local draft ID. + +If no message is given with -m, an editor is opened +with the current comment body pre-filled. + +**Arguments** + +* `id`: Draft comment ID to edit. + +**Flags** + +* `-m`, `--message=MSG`: New comment body. Opens editor if not provided. +* `-b`, `--branch=BRANCH`: Branch containing the draft. Defaults to the current branch. + +### git-spice review resolve {#gs-review-resolve} + +``` +gs review resolve [flags] +``` + +Resolve a review thread + +Resolves a review thread on the change request +for the current branch. + +The thread ID is shown in 'gs review list'. + +**Arguments** + +* `thread-id`: Thread ID to resolve. + +**Flags** + +* `-b`, `--branch=BRANCH`: Branch containing the thread. Defaults to the current branch. + +### git-spice review reopen {#gs-review-reopen} + +``` +gs review reopen [flags] +``` + +Reopen a resolved review thread + +Reopens a resolved review thread on the change request +for the current branch. + +The thread ID is shown in 'gs review list'. + +**Arguments** + +* `thread-id`: Thread ID to reopen. + +**Flags** + +* `-b`, `--branch=BRANCH`: Branch containing the thread. Defaults to the current branch. + ## Rebase ### git-spice rebase continue {#gs-rebase-continue} diff --git a/internal/git/diff_wt.go b/internal/git/diff_wt.go index a3a805de9..f1686410b 100644 --- a/internal/git/diff_wt.go +++ b/internal/git/diff_wt.go @@ -19,3 +19,18 @@ func (w *Worktree) DiffBranch(ctx context.Context, base, head string) error { } return nil } + +// DiffBranchBytes returns the unified diff output +// between base and head using triple-dot syntax. +func (w *Worktree) DiffBranchBytes( + ctx context.Context, + base, head string, +) ([]byte, error) { + out, err := w.gitCmd( + ctx, "diff", base+"..."+head, + ).Output() + if err != nil { + return nil, fmt.Errorf("diff: %w", err) + } + return out, nil +} diff --git a/internal/spice/state/staged_comment.go b/internal/spice/state/staged_comment.go new file mode 100644 index 000000000..11079e783 --- /dev/null +++ b/internal/spice/state/staged_comment.go @@ -0,0 +1,121 @@ +package state + +import ( + "context" + "errors" + "fmt" + "path" + + "go.abhg.dev/gs/internal/spice/state/storage" +) + +// _stagedCommentsDir is the directory holding staged comments +// for branches that have not yet been submitted as reviews. +const _stagedCommentsDir = "staged-comments" + +// StagedComment is a draft inline comment +// waiting to be batch-submitted as part of a review. +type StagedComment struct { + // ID is a local auto-increment identifier + // unique within the branch's staged comments. + ID int `json:"id"` + + // File is the file path relative to the repository root. + File string `json:"file"` + + // Line is the line number in the new version of the file. + Line int `json:"line"` + + // Body is the markdown body of the comment. + Body string `json:"body"` + + // ThreadID is set when replying to an existing thread. + // The format is forge-specific. + ThreadID string `json:"threadID,omitempty"` +} + +// StagedComments is the collection of staged comments +// for a branch. +type StagedComments struct { + // NextID is the next ID to assign + // to a new staged comment. + NextID int `json:"nextID"` + + // Comments are the staged comments. + Comments []StagedComment `json:"comments"` +} + +func (s *Store) stagedCommentsJSON(branch string) string { + return path.Join(_stagedCommentsDir, branch) +} + +// SaveStagedComments saves the staged comments +// for the given branch. +// If staged comments already exist for the branch, +// they will be overwritten. +func (s *Store) SaveStagedComments( + ctx context.Context, + branch string, + comments *StagedComments, +) error { + err := s.db.Set( + ctx, + s.stagedCommentsJSON(branch), + comments, + fmt.Sprintf( + "%v: save staged comments", branch, + ), + ) + if err != nil { + return fmt.Errorf( + "set staged comments: %w", err, + ) + } + return nil +} + +// LoadStagedComments retrieves staged comments +// for the given branch. +// Returns nil if no staged comments exist. +func (s *Store) LoadStagedComments( + ctx context.Context, + branch string, +) (*StagedComments, error) { + var comments StagedComments + err := s.db.Get( + ctx, + s.stagedCommentsJSON(branch), + &comments, + ) + if err != nil { + if errors.Is(err, storage.ErrNotExist) { + return nil, nil + } + return nil, fmt.Errorf( + "get staged comments: %w", err, + ) + } + return &comments, nil +} + +// ClearStagedComments removes staged comments +// for the given branch. +// This is a no-op if no staged comments exist. +func (s *Store) ClearStagedComments( + ctx context.Context, + branch string, +) error { + err := s.db.Delete( + ctx, + s.stagedCommentsJSON(branch), + fmt.Sprintf( + "%v: clear staged comments", branch, + ), + ) + if err != nil { + return fmt.Errorf( + "delete staged comments: %w", err, + ) + } + return nil +} diff --git a/internal/spice/state/staged_comment_test.go b/internal/spice/state/staged_comment_test.go new file mode 100644 index 000000000..0f8da8bc6 --- /dev/null +++ b/internal/spice/state/staged_comment_test.go @@ -0,0 +1,128 @@ +package state_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/silog/silogtest" + "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/spice/state/storage" +) + +func TestStagedComments(t *testing.T) { + ctx := t.Context() + db := storage.NewDB(make(storage.MapBackend)) + + _, err := state.InitStore(ctx, state.InitStoreRequest{ + DB: db, + Trunk: "main", + }) + require.NoError(t, err) + + store, err := state.OpenStore(ctx, db, silogtest.New(t)) + require.NoError(t, err) + + t.Run("LoadEmpty", func(t *testing.T) { + got, err := store.LoadStagedComments(ctx, "feat") + require.NoError(t, err) + assert.Nil(t, got) + }) + + t.Run("SaveAndLoad", func(t *testing.T) { + comments := &state.StagedComments{ + NextID: 3, + Comments: []state.StagedComment{ + { + ID: 1, + File: "main.go", + Line: 42, + Body: "Consider using a const here.", + }, + { + ID: 2, + File: "handler.go", + Line: 15, + Body: "I agree with your suggestion.", + ThreadID: "thread-abc", + }, + }, + } + + err := store.SaveStagedComments(ctx, "feat", comments) + require.NoError(t, err) + + got, err := store.LoadStagedComments(ctx, "feat") + require.NoError(t, err) + require.NotNil(t, got) + + assert.Equal(t, 3, got.NextID) + assert.Len(t, got.Comments, 2) + assert.Equal(t, "main.go", got.Comments[0].File) + assert.Equal(t, 42, got.Comments[0].Line) + assert.Equal(t, "thread-abc", got.Comments[1].ThreadID) + }) + + t.Run("Overwrite", func(t *testing.T) { + comments := &state.StagedComments{ + NextID: 2, + Comments: []state.StagedComment{ + {ID: 1, File: "new.go", Line: 1, Body: "New comment"}, + }, + } + + err := store.SaveStagedComments(ctx, "feat", comments) + require.NoError(t, err) + + got, err := store.LoadStagedComments(ctx, "feat") + require.NoError(t, err) + require.NotNil(t, got) + + assert.Len(t, got.Comments, 1) + assert.Equal(t, "new.go", got.Comments[0].File) + }) + + t.Run("Clear", func(t *testing.T) { + err := store.ClearStagedComments(ctx, "feat") + require.NoError(t, err) + + got, err := store.LoadStagedComments(ctx, "feat") + require.NoError(t, err) + assert.Nil(t, got) + }) + + t.Run("ClearNonExistent", func(t *testing.T) { + // Clearing a branch with no staged comments + // should not error. + err := store.ClearStagedComments(ctx, "nonexistent") + require.NoError(t, err) + }) + + t.Run("MultipleBranches", func(t *testing.T) { + commentsA := &state.StagedComments{ + NextID: 2, + Comments: []state.StagedComment{ + {ID: 1, File: "a.go", Line: 1, Body: "A"}, + }, + } + commentsB := &state.StagedComments{ + NextID: 2, + Comments: []state.StagedComment{ + {ID: 1, File: "b.go", Line: 2, Body: "B"}, + }, + } + + require.NoError(t, + store.SaveStagedComments(ctx, "branch-a", commentsA)) + require.NoError(t, + store.SaveStagedComments(ctx, "branch-b", commentsB)) + + gotA, err := store.LoadStagedComments(ctx, "branch-a") + require.NoError(t, err) + assert.Equal(t, "a.go", gotA.Comments[0].File) + + gotB, err := store.LoadStagedComments(ctx, "branch-b") + require.NoError(t, err) + assert.Equal(t, "b.go", gotB.Comments[0].File) + }) +} diff --git a/main.go b/main.go index e39fcdab7..b3344400b 100644 --- a/main.go +++ b/main.go @@ -319,6 +319,7 @@ type mainCmd struct { Branch branchCmd `cmd:"" aliases:"b" group:"Branch"` Commit commitCmd `cmd:"" aliases:"c" group:"Commit"` + Review reviewCmd `cmd:"" group:"Review" help:"Manage change request reviews"` Rebase rebaseCmd `cmd:"" aliases:"rb" group:"Rebase"` diff --git a/review.go b/review.go new file mode 100644 index 000000000..591091357 --- /dev/null +++ b/review.go @@ -0,0 +1,86 @@ +package main + +import ( + "context" + "errors" + "fmt" + + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/state" +) + +type reviewCmd struct { + Comment reviewCommentCmd `cmd:"" help:"Draft or post a review comment"` + Reply reviewReplyCmd `cmd:"" help:"Draft or post a reply to a review thread"` + Publish reviewPublishCmd `cmd:"" help:"Publish draft comments as a review"` + List reviewListCmd `cmd:"" aliases:"ls" help:"List review comments"` + Edit reviewEditCmd `cmd:"" help:"Edit a draft comment"` + Resolve reviewResolveCmd `cmd:"" help:"Resolve a review thread"` + Reopen reviewReopenCmd `cmd:"" help:"Reopen a resolved review thread"` +} + +// reviewRepositoryForBranch resolves the change and review-capable repository +// shared by review commands that operate on remote state. +func reviewRepositoryForBranch( + ctx context.Context, + svc *spice.Service, + forgeRepo forge.Repository, + branch string, +) (*spice.LookupBranchResponse, forge.ReviewRepository, error) { + b, err := svc.LookupBranch(ctx, branch) + if err != nil { + if errors.Is(err, state.ErrNotExist) { + return nil, nil, fmt.Errorf("branch not tracked: %s", branch) + } + return nil, nil, fmt.Errorf("get branch: %w", err) + } + if b.Change == nil { + return nil, nil, fmt.Errorf( + "no change request for %s; "+ + "submit the branch first with "+ + "'gs branch submit'", + branch, + ) + } + + reviewRepo, ok := forgeRepo.(forge.ReviewRepository) + if !ok { + return nil, nil, errors.New( + "forge does not support review comments", + ) + } + return b, reviewRepo, nil +} + +// loadReviewThreadIDs indexes the forge's native thread identifiers by their +// command-line representation. ReviewThreadID is intentionally opaque, so a +// command must recover the provider-owned value before replying to or resolving +// a thread named by the user. +func loadReviewThreadIDs( + ctx context.Context, + repo forge.ReviewRepository, + changeID forge.ChangeID, +) (map[string]forge.ReviewThreadID, error) { + ids := make(map[string]forge.ReviewThreadID) + for thread, err := range repo.ListReviewThreads(ctx, changeID) { + if err != nil { + return nil, fmt.Errorf("list review threads: %w", err) + } + ids[thread.ID.String()] = thread.ID + } + return ids, nil +} + +// reviewThreadID resolves a user-supplied thread string to the opaque ID value +// returned by the current forge. +func reviewThreadID( + ids map[string]forge.ReviewThreadID, + id string, +) (forge.ReviewThreadID, error) { + threadID, ok := ids[id] + if !ok { + return nil, fmt.Errorf("review thread %q not found", id) + } + return threadID, nil +} diff --git a/review_comment.go b/review_comment.go new file mode 100644 index 000000000..cdd6f19e9 --- /dev/null +++ b/review_comment.go @@ -0,0 +1,365 @@ +package main + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/reviewdiff" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/text" + "go.abhg.dev/gs/internal/xec" +) + +type reviewCommentCmd struct { + Anchor string `arg:"" optional:"" help:"Comment anchor: file.go, file.go:42, or file.go:42-50."` + Message string `short:"m" placeholder:"MSG" help:"Comment body. Opens editor if not provided."` + Draft bool `negatable:"" default:"true" help:"Save the comment as a local draft instead of posting it."` + Branch string `short:"b" placeholder:"BRANCH" predictor:"trackedBranches" help:"Branch to comment on. Defaults to the current branch."` +} + +func (*reviewCommentCmd) Help() string { + return text.Dedent(` + Adds a review comment to the change request + for the current branch. + The anchor controls the comment scope: + + file.go:42 anchored to that line + file.go:42-50 anchored to that line range + file.go anchored to the file + + Comments are saved as local drafts by default. + Use --no-draft to post immediately. + + If no message is given with -m, an editor is opened. + `) +} + +func (cmd *reviewCommentCmd) Run( + ctx context.Context, + log *silog.Logger, + wt *git.Worktree, + svc *spice.Service, + store *state.Store, + repo *git.Repository, + forgeRepo forge.Repository, +) error { + branch, err := reviewBranch(ctx, wt, cmd.Branch) + if err != nil { + return err + } + + anchor, err := parseReviewCommentAnchor(cmd.Anchor) + if err != nil { + return err + } + body, err := reviewCommentBody(ctx, repo, cmd.Message, "") + if err != nil { + return err + } + + if cmd.Draft { + if anchor.Range.IsZero() || anchor.Range.StartLine != anchor.Range.EndLine { + return errors.New( + "draft comments require a single-line file:line anchor", + ) + } + return saveReviewDraft(ctx, log, store, branch, state.StagedComment{ + File: anchor.Path, + Line: anchor.Range.StartLine, + Body: body, + }) + } + + b, reviewRepo, err := reviewRepositoryForBranch( + ctx, svc, forgeRepo, branch, + ) + if err != nil { + return err + } + diff, err := wt.DiffBranchBytes(ctx, b.Base, branch) + if err != nil { + return fmt.Errorf("get diff: %w", err) + } + patch, err := reviewdiff.Parse(diff) + if err != nil { + return fmt.Errorf("parse diff: %w", err) + } + if anchor.Range.IsZero() && !patch.ContainsFile(anchor.Path) { + return fmt.Errorf( + "review diff does not contain file %q", + anchor.Path, + ) + } + if !anchor.Range.IsZero() && !patch.ContainsLineRange( + anchor.Path, + anchor.Range.StartLine, + anchor.Range.EndLine, + ) { + return fmt.Errorf( + "review diff does not contain %s:%d-%d", + anchor.Path, + anchor.Range.StartLine, + anchor.Range.EndLine, + ) + } + + return postReviewComment( + ctx, + log, + reviewRepo, + b.Change.ChangeID(), + forge.SubmitReviewCommentRequest{ + Path: anchor.Path, + Range: anchor.Range, + Body: body, + Side: forge.ReviewThreadSideRight, + }, + ) +} + +// reviewCommentAnchor is the parsed file or inclusive line range accepted by +// review comment. A zero range identifies the whole file. +type reviewCommentAnchor struct { + Path string + Range forge.ReviewThreadRange +} + +func parseReviewCommentAnchor(value string) (reviewCommentAnchor, error) { + if value == "" { + return reviewCommentAnchor{}, errors.New("comment anchor is required") + } + if !strings.Contains(value, ":") { + return reviewCommentAnchor{Path: value}, nil + } + + file, start, end, err := parseFileAndRange(value) + if err != nil { + return reviewCommentAnchor{}, err + } + return reviewCommentAnchor{ + Path: file, + Range: forge.ReviewThreadRange{ + StartLine: start, + EndLine: end, + }, + }, nil +} + +func reviewBranch( + ctx context.Context, + wt *git.Worktree, + branch string, +) (string, error) { + if branch != "" { + return branch, nil + } + branch, err := wt.CurrentBranch(ctx) + if err != nil { + return "", fmt.Errorf("get current branch: %w", err) + } + return branch, nil +} + +func reviewCommentBody( + ctx context.Context, + repo *git.Repository, + message string, + initial string, +) (string, error) { + body := message + if body == "" { + var err error + body, err = editReviewCommentBody(ctx, repo, initial) + if err != nil { + return "", err + } + } + if strings.TrimSpace(body) == "" { + return "", errors.New("empty comment body, aborting") + } + return body, nil +} + +func saveReviewDraft( + ctx context.Context, + log *silog.Logger, + store *state.Store, + branch string, + comment state.StagedComment, +) error { + staged, err := store.LoadStagedComments(ctx, branch) + if err != nil { + return fmt.Errorf("load draft comments: %w", err) + } + if staged == nil { + staged = &state.StagedComments{NextID: 1} + } + + comment.ID = staged.NextID + staged.Comments = append(staged.Comments, comment) + staged.NextID++ + if err := store.SaveStagedComments(ctx, branch, staged); err != nil { + return fmt.Errorf("save draft comments: %w", err) + } + + if comment.ThreadID != "" { + log.Infof( + "Drafted reply %d to thread %s.", + comment.ID, + comment.ThreadID, + ) + } else { + log.Infof( + "Drafted comment %d on %s:%d.", + comment.ID, + comment.File, + comment.Line, + ) + } + return nil +} + +func postReviewComment( + ctx context.Context, + log *silog.Logger, + reviewRepo forge.ReviewRepository, + changeID forge.ChangeID, + comment forge.SubmitReviewCommentRequest, +) error { + result, err := reviewRepo.SubmitReview( + ctx, + changeID, + forge.SubmitReviewRequest{ + Comments: []forge.SubmitReviewCommentRequest{comment}, + }, + ) + if err != nil { + return fmt.Errorf("post review comment: %w", err) + } + if len(result.Comments) != 1 { + return fmt.Errorf( + "post review comment: forge returned %d comment results", + len(result.Comments), + ) + } + + log.Infof( + "Posted comment %s on %s.", + result.Comments[0].ThreadID.String(), + changeID, + ) + return nil +} + +// parseFileAndLine parses a file.go:42 argument into its path and line. +func parseFileAndLine(value string) (string, int, error) { + idx := strings.LastIndex(value, ":") + if idx < 0 { + return "", 0, fmt.Errorf( + "expected file:line format, got %q", value, + ) + } + file := value[:idx] + line, err := strconv.Atoi(value[idx+1:]) + if err != nil { + return "", 0, fmt.Errorf( + "invalid line number in %q: %w", value, err, + ) + } + if line <= 0 { + return "", 0, fmt.Errorf( + "line number must be positive, got %d", line, + ) + } + return file, line, nil +} + +// parseFileAndRange parses file.go:42 or file.go:42-50. +// The returned end equals start for a single-line anchor. +func parseFileAndRange( + value string, +) (file string, start, end int, err error) { + idx := strings.LastIndex(value, ":") + if idx < 0 { + return "", 0, 0, fmt.Errorf( + "expected file:line or file:start-end, got %q", value, + ) + } + file = value[:idx] + lineSpec := value[idx+1:] + + before, after, hasRange := strings.Cut(lineSpec, "-") + if !hasRange { + start, err = strconv.Atoi(lineSpec) + if err != nil { + return "", 0, 0, fmt.Errorf( + "invalid line number in %q: %w", value, err, + ) + } + if start <= 0 { + return "", 0, 0, fmt.Errorf( + "line number must be positive, got %d", start, + ) + } + return file, start, start, nil + } + + start, err = strconv.Atoi(before) + if err != nil { + return "", 0, 0, fmt.Errorf( + "invalid range start in %q: %w", value, err, + ) + } + end, err = strconv.Atoi(after) + if err != nil { + return "", 0, 0, fmt.Errorf( + "invalid range end in %q: %w", value, err, + ) + } + if start <= 0 || end <= 0 { + return "", 0, 0, fmt.Errorf( + "line numbers must be positive in %q", value, + ) + } + if end <= start { + return "", 0, 0, fmt.Errorf( + "range end must be greater than start in %q", value, + ) + } + return file, start, end, nil +} + +// editReviewCommentBody opens the configured editor with initial as its +// starting contents and returns the edited comment body. +func editReviewCommentBody( + ctx context.Context, + repo *git.Repository, + initial string, +) (string, error) { + tmpFile := filepath.Join(os.TempDir(), "GS_REVIEW_EDITMSG") + if err := os.WriteFile(tmpFile, []byte(initial), 0o644); err != nil { + return "", fmt.Errorf("write temp file: %w", err) + } + defer func() { _ = os.Remove(tmpFile) }() + + editor := gitEditor(ctx, repo) + cmd := xec.EditCommand(editor, tmpFile) + if err := cmd.Run(); err != nil { + return "", fmt.Errorf("run editor: %w", err) + } + + content, err := os.ReadFile(tmpFile) + if err != nil { + return "", fmt.Errorf("read temp file: %w", err) + } + return string(content), nil +} diff --git a/review_edit.go b/review_edit.go new file mode 100644 index 000000000..a30c4cf67 --- /dev/null +++ b/review_edit.go @@ -0,0 +1,78 @@ +package main + +import ( + "context" + "fmt" + + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/text" +) + +type reviewEditCmd struct { + ID int `arg:"" help:"Draft comment ID to edit."` + Message string `short:"m" placeholder:"MSG" help:"New comment body. Opens editor if not provided."` + Branch string `short:"b" placeholder:"BRANCH" predictor:"trackedBranches" help:"Branch containing the draft. Defaults to the current branch."` +} + +func (*reviewEditCmd) Help() string { + return text.Dedent(` + Edits a local draft comment. + + Use 'gs review list --draft-only' + to find the branch-local draft ID. + + If no message is given with -m, an editor is opened + with the current comment body pre-filled. + `) +} + +func (cmd *reviewEditCmd) Run( + ctx context.Context, + log *silog.Logger, + wt *git.Worktree, + store *state.Store, + repo *git.Repository, +) error { + branch, err := reviewBranch(ctx, wt, cmd.Branch) + if err != nil { + return err + } + + staged, err := store.LoadStagedComments(ctx, branch) + if err != nil { + return fmt.Errorf("load draft comments: %w", err) + } + if staged == nil { + staged = &state.StagedComments{} + } + + idx := -1 + for i, comment := range staged.Comments { + if comment.ID == cmd.ID { + idx = i + break + } + } + if idx < 0 { + return fmt.Errorf("draft comment %d not found", cmd.ID) + } + + body, err := reviewCommentBody( + ctx, + repo, + cmd.Message, + staged.Comments[idx].Body, + ) + if err != nil { + return err + } + staged.Comments[idx].Body = body + if err := store.SaveStagedComments(ctx, branch, staged); err != nil { + return fmt.Errorf("save draft comments: %w", err) + } + + log.Infof("Updated draft comment %d.", cmd.ID) + return nil +} diff --git a/review_list.go b/review_list.go new file mode 100644 index 000000000..a64c3a5c9 --- /dev/null +++ b/review_list.go @@ -0,0 +1,378 @@ +package main + +import ( + "bufio" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "strings" + "time" + + "github.com/alecthomas/kong" + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/sliceutil" + "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/text" +) + +type reviewListCmd struct { + Branch string `short:"b" placeholder:"BRANCH" predictor:"trackedBranches" help:"Branch to list comments for. Defaults to the current branch."` + DraftOnly bool `name:"draft-only" help:"Show only draft comments."` + Unresolved bool `help:"Show only unresolved comments."` + JSON bool `name:"json" released:"unreleased" help:"Write to stdout as a stream of JSON objects."` +} + +func (*reviewListCmd) Help() string { + return text.Dedent(` + Lists comments on the change request + associated with the current branch. + Use --branch to target a different branch. + + Draft comments are identified by a branch-local integer. + + Use --draft-only to show only draft comments. + Use --unresolved to show only unresolved comments. + + With --json, prints output to stdout + as a stream of JSON objects. + `) +} + +func (cmd *reviewListCmd) Run( + ctx context.Context, + kctx *kong.Context, + log *silog.Logger, + wt *git.Worktree, + svc *spice.Service, + store *state.Store, + forgeRepo forge.Repository, +) error { + branch, err := cmd.resolveBranch(ctx, wt) + if err != nil { + return err + } + + staged, forgeComments, err := cmd.loadComments( + ctx, log, svc, store, forgeRepo, branch, + ) + if err != nil { + return err + } + + if cmd.JSON { + return cmd.writeJSON( + kctx.Stdout, staged, forgeComments, + ) + } + return cmd.writeText(log, branch, staged, forgeComments) +} + +func (cmd *reviewListCmd) resolveBranch( + ctx context.Context, wt *git.Worktree, +) (string, error) { + if cmd.Branch != "" { + return cmd.Branch, nil + } + branch, err := wt.CurrentBranch(ctx) + if err != nil { + return "", fmt.Errorf("get current branch: %w", err) + } + return branch, nil +} + +func (cmd *reviewListCmd) loadComments( + ctx context.Context, + log *silog.Logger, + svc *spice.Service, + store *state.Store, + forgeRepo forge.Repository, + branch string, +) ([]*state.StagedComment, []*listedReviewComment, error) { + staged, err := loadStagedComments(ctx, store, branch) + if err != nil { + return nil, nil, err + } + + if cmd.DraftOnly { + return staged, nil, nil + } + + forgeComments, err := loadForgeComments( + ctx, log, svc, forgeRepo, branch, + ) + if err != nil { + return nil, nil, err + } + + return staged, cmd.filterForge(forgeComments), nil +} + +func loadStagedComments( + ctx context.Context, + store *state.Store, + branch string, +) ([]*state.StagedComment, error) { + staged, err := store.LoadStagedComments(ctx, branch) + if err != nil { + return nil, fmt.Errorf("load draft comments: %w", err) + } + if staged == nil { + return nil, nil + } + + refs := make( + []*state.StagedComment, len(staged.Comments), + ) + for i := range staged.Comments { + refs[i] = &staged.Comments[i] + } + return refs, nil +} + +func loadForgeComments( + ctx context.Context, + log *silog.Logger, + svc *spice.Service, + forgeRepo forge.Repository, + branch string, +) ([]*listedReviewComment, error) { + b, err := svc.LookupBranch(ctx, branch) + if err != nil { + if errors.Is(err, state.ErrNotExist) { + return nil, fmt.Errorf( + "branch not tracked: %s", branch, + ) + } + return nil, fmt.Errorf("get branch: %w", err) + } + + if b.Change == nil { + log.Infof( + "No change request found for %s.", branch, + ) + return nil, nil + } + + reviewRepo, ok := forgeRepo.(forge.ReviewRepository) + if !ok { + log.Infof( + "Forge does not support review comments.", + ) + return nil, nil + } + + threads, err := sliceutil.CollectErr( + reviewRepo.ListReviewThreads(ctx, b.Change.ChangeID()), + ) + if err != nil { + return nil, fmt.Errorf( + "list review threads: %w", err, + ) + } + + var comments []*listedReviewComment + for _, thread := range threads { + for i := range thread.Comments { + comments = append(comments, &listedReviewComment{ + Thread: thread, + Comment: &thread.Comments[i], + }) + } + } + return comments, nil +} + +// listedReviewComment keeps a comment paired with the thread that owns its +// location and status. +type listedReviewComment struct { + Thread *forge.ReviewThread + Comment *forge.ReviewComment +} + +func (cmd *reviewListCmd) filterForge( + comments []*listedReviewComment, +) []*listedReviewComment { + if !cmd.Unresolved { + return comments + } + + var filtered []*listedReviewComment + for _, c := range comments { + if c.Thread.Resolved == nil || !*c.Thread.Resolved { + filtered = append(filtered, c) + } + } + return filtered +} + +// writeText prints comments in human-readable format. +func (cmd *reviewListCmd) writeText( + log *silog.Logger, + branch string, + staged []*state.StagedComment, + forgeComments []*listedReviewComment, +) error { + if len(staged) > 0 { + log.Infof("Draft comments:") + for _, c := range staged { + writeStagedText(log, c) + } + } + + if cmd.DraftOnly && len(staged) == 0 { + log.Infof("No draft comments for %s.", branch) + return nil + } + + if len(forgeComments) > 0 { + log.Infof("Comments:") + for _, c := range forgeComments { + writeForgeText(log, c) + } + } + + if len(forgeComments) == 0 && len(staged) == 0 { + log.Infof("No comments on %s.", branch) + } + return nil +} + +func writeStagedText( + log *silog.Logger, c *state.StagedComment, +) { + location := fmt.Sprintf("%s:%d", c.File, c.Line) + if c.ThreadID != "" { + location = "reply:" + c.ThreadID + } + log.Infof(" %-4d %s", c.ID, location) + writeBodyIndented(log, c.Body) +} + +func writeForgeText( + log *silog.Logger, c *listedReviewComment, +) { + location := fmt.Sprintf( + "%s:%d", c.Thread.Path, c.Thread.Range.StartLine, + ) + threadInfo := "" + if c.Thread.ID != nil { + threadInfo = " [" + c.Thread.ID.String() + "]" + } + log.Infof( + " %-12s %s %s %s%s", + c.Comment.ID.String(), location, c.Comment.Author, + commentStatus(c), threadInfo, + ) + writeBodyIndented(log, c.Comment.Body) +} + +func writeBodyIndented(log *silog.Logger, body string) { + for line := range strings.SplitSeq(body, "\n") { + log.Infof(" %s", line) + } +} + +func commentStatus(c *listedReviewComment) string { + if c.Thread.Outdated != nil && *c.Thread.Outdated { + return "outdated" + } + if c.Thread.Resolved != nil && *c.Thread.Resolved { + return "resolved" + } + return "open" +} + +// writeJSON encodes comments as NDJSON to stdout. +func (cmd *reviewListCmd) writeJSON( + w io.Writer, + staged []*state.StagedComment, + forgeComments []*listedReviewComment, +) (retErr error) { + bufw := bufio.NewWriter(w) + defer func() { + retErr = errors.Join(retErr, bufw.Flush()) + }() + + enc := json.NewEncoder(bufw) + for _, c := range staged { + if err := enc.Encode(stagedToJSON(c)); err != nil { + return fmt.Errorf("encode staged: %w", err) + } + } + for _, c := range forgeComments { + if err := enc.Encode(forgeToJSON(c)); err != nil { + return fmt.Errorf("encode forge: %w", err) + } + } + return nil +} + +func stagedToJSON(c *state.StagedComment) jsonComment { + return jsonComment{ + Kind: "draft", + ID: fmt.Sprintf("%d", c.ID), + Path: c.File, + Line: c.Line, + Body: c.Body, + ThreadID: c.ThreadID, + } +} + +func forgeToJSON(c *listedReviewComment) jsonComment { + var createdAt *time.Time + if !c.Comment.CreatedAt.IsZero() { + createdAt = &c.Comment.CreatedAt + } + return jsonComment{ + Kind: "forge", + ID: c.Comment.ID.String(), + Path: c.Thread.Path, + Line: c.Thread.Range.StartLine, + Body: c.Comment.Body, + ThreadID: c.Thread.ID.String(), + Author: c.Comment.Author, + Status: commentStatus(c), + CreatedAt: createdAt, + } +} + +// jsonComment is the JSON representation +// of a comment for --json output. +type jsonComment struct { + // Kind is "draft" or "forge". + Kind string `json:"kind"` + + // ID is the comment identifier. + // For draft comments: a branch-local integer encoded as a string. + // For forge comments: forge-specific ID. + ID string `json:"id"` + + // Path is the file path relative to the repo root. + Path string `json:"path,omitempty"` + + // Line is the line number in the file. + Line int `json:"line,omitempty"` + + // Body is the full markdown body of the comment. + Body string `json:"body"` + + // ThreadID is the thread identifier, if any. + ThreadID string `json:"threadID,omitempty"` + + // Author is the username of the comment author. + // Only set for forge comments. + Author string `json:"author,omitempty"` + + // Status is "open", "resolved", or "outdated". + // Only set for forge comments. + Status string `json:"status,omitempty"` + + // CreatedAt is the time the comment was created. + // Only set for forge comments. + CreatedAt *time.Time `json:"createdAt,omitempty"` +} diff --git a/review_publish.go b/review_publish.go new file mode 100644 index 000000000..dd3335879 --- /dev/null +++ b/review_publish.go @@ -0,0 +1,184 @@ +package main + +import ( + "context" + "errors" + "fmt" + + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/reviewdiff" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/text" +) + +type reviewPublishCmd struct { + Body string `placeholder:"BODY" help:"Overall review body."` + Approve bool `help:"Mark the review as approved."` + RequestChanges bool `name:"request-changes" help:"Mark the review as requesting changes."` + Branch string `short:"b" placeholder:"BRANCH" predictor:"trackedBranches" help:"Branch whose draft comments to publish. Defaults to the current branch."` +} + +func (*reviewPublishCmd) Help() string { + return text.Dedent(` + Publishes all draft comments for the current branch + as a single review on the change request. + + Use --approve or --request-changes + to set the review event type. + Defaults to a comment-only review. + + Use --body to add an overall review body. + `) +} + +func (cmd *reviewPublishCmd) Run( + ctx context.Context, + log *silog.Logger, + wt *git.Worktree, + svc *spice.Service, + store *state.Store, + forgeRepo forge.Repository, +) error { + branch := cmd.Branch + if branch == "" { + var err error + branch, err = wt.CurrentBranch(ctx) + if err != nil { + return fmt.Errorf("get current branch: %w", err) + } + } + + staged, err := store.LoadStagedComments(ctx, branch) + if err != nil { + return fmt.Errorf("load draft comments: %w", err) + } + if staged == nil { + staged = &state.StagedComments{} + } + + if len(staged.Comments) == 0 { + log.Infof("No draft comments to publish.") + return nil + } + + b, err := svc.LookupBranch(ctx, branch) + if err != nil { + if errors.Is(err, state.ErrNotExist) { + return fmt.Errorf( + "branch not tracked: %s", branch, + ) + } + return fmt.Errorf("get branch: %w", err) + } + + if b.Change == nil { + return fmt.Errorf( + "no change request for %s; "+ + "submit the branch first with "+ + "'gs branch submit'", + branch, + ) + } + + reviewRepo, ok := forgeRepo.(forge.ReviewRepository) + if !ok { + return errors.New( + "forge does not support review comments", + ) + } + + // Draft roots use the selected branch's postimage coordinates. Parse the + // review diff once so every root can be checked before anything is sent. + diff, err := wt.DiffBranchBytes(ctx, b.Base, branch) + if err != nil { + return fmt.Errorf("get diff: %w", err) + } + + patch, err := reviewdiff.Parse(diff) + if err != nil { + return fmt.Errorf("parse diff: %w", err) + } + + var threadIDs map[string]forge.ReviewThreadID + for _, comment := range staged.Comments { + if comment.ThreadID == "" { + continue + } + threadIDs, err = loadReviewThreadIDs( + ctx, reviewRepo, b.Change.ChangeID(), + ) + if err != nil { + return err + } + break + } + + var comments []forge.SubmitReviewCommentRequest + for _, sc := range staged.Comments { + if sc.ThreadID != "" { + threadID, err := reviewThreadID(threadIDs, sc.ThreadID) + if err != nil { + return fmt.Errorf("draft %d: %w", sc.ID, err) + } + comments = append(comments, + forge.SubmitReviewCommentRequest{ + Body: sc.Body, + ReplyTo: threadID, + }, + ) + continue + } + + if !patch.ContainsLine(sc.File, sc.Line) { + return fmt.Errorf( + "draft %d: review diff does not contain %s:%d", + sc.ID, + sc.File, + sc.Line, + ) + } + comments = append(comments, + forge.SubmitReviewCommentRequest{ + Path: sc.File, + Range: forge.ReviewThreadLine(sc.Line), + Body: sc.Body, + Side: forge.ReviewThreadSideRight, + }, + ) + } + + disposition := forge.ReviewDispositionNone + if cmd.Approve { + disposition = forge.ReviewDispositionApprove + } else if cmd.RequestChanges { + disposition = forge.ReviewDispositionRequestChanges + } + + if _, err := reviewRepo.SubmitReview( + ctx, + b.Change.ChangeID(), + forge.SubmitReviewRequest{ + Body: cmd.Body, + Disposition: disposition, + Comments: comments, + }, + ); err != nil { + return fmt.Errorf("submit review: %w", err) + } + + if err := store.ClearStagedComments( + ctx, branch, + ); err != nil { + return fmt.Errorf("clear draft comments: %w", err) + } + + log.Infof( + "Published %d comment(s) as review on %s.", + len(comments), + b.Change.ChangeID(), + ) + return nil +} diff --git a/review_reply.go b/review_reply.go new file mode 100644 index 000000000..926a7da75 --- /dev/null +++ b/review_reply.go @@ -0,0 +1,87 @@ +package main + +import ( + "context" + + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/spice/state" + "go.abhg.dev/gs/internal/text" +) + +type reviewReplyCmd struct { + ThreadID string `arg:"" help:"Thread ID to reply to."` + Message string `short:"m" placeholder:"MSG" help:"Reply body. Opens editor if not provided."` + Draft bool `negatable:"" default:"true" help:"Save the reply as a local draft instead of posting it."` + Branch string `short:"b" placeholder:"BRANCH" predictor:"trackedBranches" help:"Branch containing the thread. Defaults to the current branch."` +} + +func (*reviewReplyCmd) Help() string { + return text.Dedent(` + Replies to a review thread on the change request + for the current branch. + + Replies are saved as local drafts by default. + Use --no-draft to post immediately. + + If no message is given with -m, an editor is opened. + `) +} + +func (cmd *reviewReplyCmd) Run( + ctx context.Context, + log *silog.Logger, + wt *git.Worktree, + svc *spice.Service, + store *state.Store, + repo *git.Repository, + forgeRepo forge.Repository, +) error { + branch, err := reviewBranch(ctx, wt, cmd.Branch) + if err != nil { + return err + } + body, err := reviewCommentBody(ctx, repo, cmd.Message, "") + if err != nil { + return err + } + + if cmd.Draft { + return saveReviewDraft(ctx, log, store, branch, state.StagedComment{ + Body: body, + ThreadID: cmd.ThreadID, + }) + } + + b, reviewRepo, err := reviewRepositoryForBranch( + ctx, svc, forgeRepo, branch, + ) + if err != nil { + return err + } + threadIDs, err := loadReviewThreadIDs( + ctx, + reviewRepo, + b.Change.ChangeID(), + ) + if err != nil { + return err + } + threadID, err := reviewThreadID(threadIDs, cmd.ThreadID) + if err != nil { + return err + } + + return postReviewComment( + ctx, + log, + reviewRepo, + b.Change.ChangeID(), + forge.SubmitReviewCommentRequest{ + Body: body, + ReplyTo: threadID, + }, + ) +} diff --git a/review_resolution.go b/review_resolution.go new file mode 100644 index 000000000..bb0cd19b6 --- /dev/null +++ b/review_resolution.go @@ -0,0 +1,133 @@ +package main + +import ( + "context" + "errors" + "fmt" + + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/silog" + "go.abhg.dev/gs/internal/spice" + "go.abhg.dev/gs/internal/text" +) + +type reviewResolveCmd struct { + ThreadID string `arg:"" help:"Thread ID to resolve."` + Branch string `short:"b" placeholder:"BRANCH" predictor:"trackedBranches" help:"Branch containing the thread. Defaults to the current branch."` +} + +func (*reviewResolveCmd) Help() string { + return text.Dedent(` + Resolves a review thread on the change request + for the current branch. + + The thread ID is shown in 'gs review list'. + `) +} + +func (cmd *reviewResolveCmd) Run( + ctx context.Context, + log *silog.Logger, + wt *git.Worktree, + svc *spice.Service, + forgeRepo forge.Repository, +) error { + return setReviewThreadResolved( + ctx, + log, + wt, + svc, + forgeRepo, + cmd.Branch, + cmd.ThreadID, + true, + ) +} + +type reviewReopenCmd struct { + ThreadID string `arg:"" help:"Thread ID to reopen."` + Branch string `short:"b" placeholder:"BRANCH" predictor:"trackedBranches" help:"Branch containing the thread. Defaults to the current branch."` +} + +func (*reviewReopenCmd) Help() string { + return text.Dedent(` + Reopens a resolved review thread on the change request + for the current branch. + + The thread ID is shown in 'gs review list'. + `) +} + +func (cmd *reviewReopenCmd) Run( + ctx context.Context, + log *silog.Logger, + wt *git.Worktree, + svc *spice.Service, + forgeRepo forge.Repository, +) error { + return setReviewThreadResolved( + ctx, + log, + wt, + svc, + forgeRepo, + cmd.Branch, + cmd.ThreadID, + false, + ) +} + +func setReviewThreadResolved( + ctx context.Context, + log *silog.Logger, + wt *git.Worktree, + svc *spice.Service, + forgeRepo forge.Repository, + branch string, + thread string, + resolved bool, +) error { + branch, err := reviewBranch(ctx, wt, branch) + if err != nil { + return err + } + b, reviewRepo, err := reviewRepositoryForBranch( + ctx, svc, forgeRepo, branch, + ) + if err != nil { + return err + } + resolver, ok := forgeRepo.(forge.ReviewThreadResolver) + if !ok { + return errors.New( + "forge does not support review thread resolution", + ) + } + + threadIDs, err := loadReviewThreadIDs( + ctx, + reviewRepo, + b.Change.ChangeID(), + ) + if err != nil { + return err + } + threadID, err := reviewThreadID(threadIDs, thread) + if err != nil { + return err + } + + if resolved { + if err := resolver.ResolveReviewThread(ctx, threadID); err != nil { + return fmt.Errorf("resolve thread: %w", err) + } + log.Infof("Resolved thread %s.", thread) + return nil + } + if err := resolver.UnresolveReviewThread(ctx, threadID); err != nil { + return fmt.Errorf("reopen thread: %w", err) + } + log.Infof("Reopened thread %s.", thread) + return nil +} diff --git a/testdata/help/gs.txt b/testdata/help/gs.txt index 43406fde2..cedd4c2e7 100644 --- a/testdata/help/gs.txt +++ b/testdata/help/gs.txt @@ -69,6 +69,15 @@ Commit commit (c) fixup (f) Fixup a commit below the current commit commit (c) pick (p) Cherry-pick a commit +Review + review comment Draft or post a review comment + review reply Draft or post a reply to a review thread + review publish Publish draft comments as a review + review list (ls) List review comments + review edit Edit a draft comment + review resolve Resolve a review thread + review reopen Reopen a resolved review thread + Rebase rebase (rb) continue (c) Continue an interrupted operation rebase (rb) abort (a) Abort an operation diff --git a/testdata/help/review_comment.txt b/testdata/help/review_comment.txt new file mode 100644 index 000000000..4141f0b23 --- /dev/null +++ b/testdata/help/review_comment.txt @@ -0,0 +1,31 @@ +Usage: gs review comment [] [flags] + +Draft or post a review comment + +Adds a review comment to the change request for the current branch. The anchor +controls the comment scope: + + file.go:42 anchored to that line + file.go:42-50 anchored to that line range + file.go anchored to the file + +Comments are saved as local drafts by default. Use --no-draft to post +immediately. + +If no message is given with -m, an editor is opened. + +Arguments: + [] Comment anchor: file.go, file.go:42, or file.go:42-50. + +Flags: + -m, --message=MSG Comment body. Opens editor if not provided. + --[no-]draft Save the comment as a local draft instead of posting + it. + -b, --branch=BRANCH Branch to comment on. Defaults to the current branch. + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/review_edit.txt b/testdata/help/review_edit.txt new file mode 100644 index 000000000..7337e0344 --- /dev/null +++ b/testdata/help/review_edit.txt @@ -0,0 +1,25 @@ +Usage: gs review edit [flags] + +Edit a draft comment + +Edits a local draft comment. + +Use 'gs review list --draft-only' to find the branch-local draft ID. + +If no message is given with -m, an editor is opened with the current comment +body pre-filled. + +Arguments: + Draft comment ID to edit. + +Flags: + -m, --message=MSG New comment body. Opens editor if not provided. + -b, --branch=BRANCH Branch containing the draft. Defaults to the current + branch. + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/review_list.txt b/testdata/help/review_list.txt new file mode 100644 index 000000000..69ce7b2e9 --- /dev/null +++ b/testdata/help/review_list.txt @@ -0,0 +1,27 @@ +Usage: gs review list (ls) [flags] + +List review comments + +Lists comments on the change request associated with the current branch. +Use --branch to target a different branch. + +Draft comments are identified by a branch-local integer. + +Use --draft-only to show only draft comments. Use --unresolved to show only +unresolved comments. + +With --json, prints output to stdout as a stream of JSON objects. + +Flags: + -b, --branch=BRANCH Branch to list comments for. Defaults to the current + branch. + --draft-only Show only draft comments. + --unresolved Show only unresolved comments. + --json Write to stdout as a stream of JSON objects. + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/review_publish.txt b/testdata/help/review_publish.txt new file mode 100644 index 000000000..6cfe6e13a --- /dev/null +++ b/testdata/help/review_publish.txt @@ -0,0 +1,25 @@ +Usage: gs review publish [flags] + +Publish draft comments as a review + +Publishes all draft comments for the current branch as a single review on the +change request. + +Use --approve or --request-changes to set the review event type. Defaults to a +comment-only review. + +Use --body to add an overall review body. + +Flags: + --body=BODY Overall review body. + --approve Mark the review as approved. + --request-changes Mark the review as requesting changes. + -b, --branch=BRANCH Branch whose draft comments to publish. Defaults to + the current branch. + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/review_reopen.txt b/testdata/help/review_reopen.txt new file mode 100644 index 000000000..ee7b2206d --- /dev/null +++ b/testdata/help/review_reopen.txt @@ -0,0 +1,21 @@ +Usage: gs review reopen [flags] + +Reopen a resolved review thread + +Reopens a resolved review thread on the change request for the current branch. + +The thread ID is shown in 'gs review list'. + +Arguments: + Thread ID to reopen. + +Flags: + -b, --branch=BRANCH Branch containing the thread. Defaults to the current + branch. + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/review_reply.txt b/testdata/help/review_reply.txt new file mode 100644 index 000000000..5c365ec7a --- /dev/null +++ b/testdata/help/review_reply.txt @@ -0,0 +1,26 @@ +Usage: gs review reply [flags] + +Draft or post a reply to a review thread + +Replies to a review thread on the change request for the current branch. + +Replies are saved as local drafts by default. Use --no-draft to post +immediately. + +If no message is given with -m, an editor is opened. + +Arguments: + Thread ID to reply to. + +Flags: + -m, --message=MSG Reply body. Opens editor if not provided. + --[no-]draft Save the reply as a local draft instead of posting it. + -b, --branch=BRANCH Branch containing the thread. Defaults to the current + branch. + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/help/review_resolve.txt b/testdata/help/review_resolve.txt new file mode 100644 index 000000000..70c10371f --- /dev/null +++ b/testdata/help/review_resolve.txt @@ -0,0 +1,21 @@ +Usage: gs review resolve [flags] + +Resolve a review thread + +Resolves a review thread on the change request for the current branch. + +The thread ID is shown in 'gs review list'. + +Arguments: + Thread ID to resolve. + +Flags: + -b, --branch=BRANCH Branch containing the thread. Defaults to the current + branch. + +Global Flags: + -h, --help Show help for the command + --version Print version information and quit + -v, --verbose Enable verbose output ($GIT_SPICE_VERBOSE) + -C, --dir=DIR Change to DIR before doing anything + --[no-]prompt Whether to prompt for missing information diff --git a/testdata/script/review_comment.txt b/testdata/script/review_comment.txt new file mode 100644 index 000000000..821b49088 --- /dev/null +++ b/testdata/script/review_comment.txt @@ -0,0 +1,48 @@ +# Draft and post a review comment or reply. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +# set up a fake GitHub remote +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch with a file and submit it +git add handler.go +gs bc -m 'Add handler' feature1 +gs branch submit --fill +stderr 'Created #' + +# comments are drafts by default +gs review comment handler.go:5 -m 'This needs a context parameter.' +stderr 'Drafted comment 1' + +# --no-draft posts a comment immediately +gs review comment handler.go:5 -m 'This needs a context parameter.' --no-draft +stderr 'Posted comment' + +# replies follow the same draft convention +gs review reply thread-2 -m 'Agreed.' +stderr 'Drafted reply 2' +gs review reply thread-2 -m 'Agreed.' --no-draft +stderr 'Posted comment' + +-- repo/handler.go -- +package main + +import "fmt" + +func handle() { + fmt.Println("handling") +} diff --git a/testdata/script/review_edit.txt b/testdata/script/review_edit.txt new file mode 100644 index 000000000..5c6dac749 --- /dev/null +++ b/testdata/script/review_edit.txt @@ -0,0 +1,44 @@ +# Edit a draft comment before publication. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +# set up a fake GitHub remote +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch with a file and submit it +git add main.go +gs bc -m 'Add main' feature1 +gs branch submit --fill +stderr 'Created #' + +# draft a comment +gs review comment main.go:3 -m 'Original comment.' +stderr 'Drafted comment 1' + +# edit the draft comment with -m +gs review edit 1 -m 'Updated comment.' +stderr 'Updated draft comment 1' + +# verify the edit took effect by listing drafts +gs review list --draft-only +stderr 'Updated comment' + +-- repo/main.go -- +package main + +func main() { + println("hello") +} diff --git a/testdata/script/review_list.txt b/testdata/script/review_list.txt new file mode 100644 index 000000000..bda00cbc8 --- /dev/null +++ b/testdata/script/review_list.txt @@ -0,0 +1,47 @@ +# List comments on a change request. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +# set up a fake GitHub remote +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch with a file and submit it +git add main.go +gs bc -m 'Add main' feature1 +gs branch submit --fill +stderr 'Created #' + +# add an inline comment so there is something to list +gs review comment main.go:3 -m 'Consider using a constant.' --no-draft +stderr 'Posted comment' + +# list should show the comment +gs review list +stderr 'Comments:' +stderr 'main.go:3' +stderr 'Consider using a constant' + +# list on a branch with no CR should say so +gs bc -m 'Another branch' feature2 +gs review list +stderr 'No change request' + +-- repo/main.go -- +package main + +func main() { + println("hello") +} diff --git a/testdata/script/review_list_json.txt b/testdata/script/review_list_json.txt new file mode 100644 index 000000000..438d1702d --- /dev/null +++ b/testdata/script/review_list_json.txt @@ -0,0 +1,64 @@ +# List comments on a change request with --json output. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +# set up a fake GitHub remote +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch with a file and submit it +git add main.go +gs bc -m 'Add main' feature1 +gs branch submit --fill +stderr 'Created #' + +# add an inline comment so there is something to list +gs review comment main.go:3 -m 'Consider using a constant here because it would make the code much more maintainable and readable.' --no-draft +stderr 'Posted comment' + +# --json should output NDJSON to stdout with forge comment +gs review list --json +stdout '"kind":"forge"' +stdout '"body":"Consider using a constant here because it would make the code much more maintainable and readable."' +stdout '"path":"main.go"' +stdout '"line":3' +stdout '"status":"open"' + +# text mode shows the full body without truncation +gs review list +stderr 'Consider using a constant here because it would make the code much more maintainable and readable.' + +# draft a comment +gs review comment main.go:5 -m 'Add error handling here.' +stderr 'Drafted comment' + +# --draft-only --json should include only draft comments +gs review list --draft-only --json +cmpenvJSON stdout $WORK/golden/draft_only.json + +# --json without --draft-only includes both draft and forge comments +gs review list --json +stdout '"kind":"draft"' +stdout '"kind":"forge"' + +-- repo/main.go -- +package main + +func main() { + println("hello") +} + +-- golden/draft_only.json -- +{"kind":"draft","id":"1","path":"main.go","line":5,"body":"Add error handling here."} diff --git a/testdata/script/review_publish.txt b/testdata/script/review_publish.txt new file mode 100644 index 000000000..51d6e1e37 --- /dev/null +++ b/testdata/script/review_publish.txt @@ -0,0 +1,57 @@ +# Draft comments and publish them as a review. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +# set up a fake GitHub remote +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch with a file and submit it +git add feature.go +gs bc -m 'Add feature' feature1 +gs branch submit --fill +stderr 'Created #' + +# draft a comment on the file +gs review comment feature.go:3 -m 'Consider renaming this function.' +stderr 'Drafted comment 1 on feature.go:3' + +# draft another comment +gs review comment feature.go:7 -m 'Add error handling here.' +stderr 'Drafted comment 2 on feature.go:7' + +# list draft comments +gs review list --draft-only +stderr 'feature.go:3' +stderr 'feature.go:7' + +# publish draft comments as a review +gs review publish +stderr 'Published 2 comment' + +# draft comments should be cleared after publication +gs review list --draft-only +stderr 'No draft comments' + +-- repo/feature.go -- +package main + +func doWork() { + // does some work +} + +func handleRequest() { + // handles a request +} diff --git a/testdata/script/review_resolve.txt b/testdata/script/review_resolve.txt new file mode 100644 index 000000000..721098ad4 --- /dev/null +++ b/testdata/script/review_resolve.txt @@ -0,0 +1,47 @@ +# Resolve and reopen a review thread. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +# setup +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +# set up a fake GitHub remote +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# create a branch with a file and submit it +git add util.go +gs bc -m 'Add util' feature1 +gs branch submit --fill +stderr 'Created #' + +# post an inline comment to create a thread +gs review comment util.go:3 -m 'Rename this function.' --no-draft +stderr 'Posted comment' + +# resolve and reopen the deterministic ShamHub thread +gs review resolve thread-2 +stderr 'Resolved thread thread-2' +gs review list +stderr 'resolved' + +gs review reopen thread-2 +stderr 'Reopened thread thread-2' +gs review list +stderr 'open' + +-- repo/util.go -- +package main + +func doStuff() { + // stuff +} diff --git a/testdata/script/review_scopes.txt b/testdata/script/review_scopes.txt new file mode 100644 index 000000000..ba8b1de0c --- /dev/null +++ b/testdata/script/review_scopes.txt @@ -0,0 +1,107 @@ +# Verifies line, line-range, and file review-comment anchors. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +cd repo +git init +git commit --allow-empty -m 'Initial commit' + +gs repo init +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +git add main.go +gs bc -m 'Add main' feature1 +gs branch submit --fill +stderr 'Created #' + +# Line scope: existing form. +gs review comment main.go:4 -m 'Line-scope comment.' --no-draft +stderr 'Posted comment' + +# Line-range scope: inclusive start and end lines. +gs review comment main.go:4-5 -m 'Range-scope comment.' --no-draft +stderr 'Posted comment' + +# File scope: path with no ':'. +gs review comment main.go -m 'File-scope comment.' --no-draft +stderr 'Posted comment' + +# Text output lists the line and file-level anchors. +gs review list +stderr 'main\.go:4' +stderr 'main\.go:0' +stderr 'Line-scope comment' +stderr 'Range-scope comment' +stderr 'File-scope comment' + +# ShamHub's stable YAML dump preserves the complete inclusive range. +shamhub dump reviews 1 +cmp stdout $WORK/golden/reviews.yaml + +# Missing anchor is rejected. +! gs review comment -m 'oops' --no-draft +stderr 'comment anchor is required' + +-- repo/main.go -- +package main + +func main() { + println("hello") + println("world") +} +-- golden/reviews.yaml -- +changes: + - change: 1 + submissions: + - submitter: alice + disposition: comment + commentIDs: + - 2 + - submitter: alice + disposition: comment + commentIDs: + - 3 + - submitter: alice + disposition: comment + commentIDs: + - 4 + threads: + - id: thread-2 + path: main.go + range: + start: 4 + end: 4 + side: right + resolved: false + outdated: false + comments: + - id: 2 + author: alice + body: Line-scope comment. + - id: thread-3 + path: main.go + range: + start: 4 + end: 5 + side: right + resolved: false + outdated: false + comments: + - id: 3 + author: alice + body: Range-scope comment. + - id: thread-4 + path: main.go + resolved: false + outdated: false + comments: + - id: 4 + author: alice + body: File-scope comment. diff --git a/testdata/script/review_stale.txt b/testdata/script/review_stale.txt new file mode 100644 index 000000000..980ab9094 --- /dev/null +++ b/testdata/script/review_stale.txt @@ -0,0 +1,75 @@ +# Verifies that the 'outdated' flag on review comments is computed +# automatically from the diff between the comment's commit and the +# change's current head, not just a static seed value. +# +# A comment becomes outdated when a subsequent commit deletes or replaces its +# anchored line. Comments on other lines stay live. + +as 'Test ' +at '2024-04-05T16:40:32Z' + +cd repo +git init +git commit --allow-empty -m 'Initial commit' +gs repo init + +shamhub-setup +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +git add main.go +gs bc -m 'Add main' feature1 +gs branch submit --fill +stderr 'Created #' + +# Two inline comments at different lines on the same head commit. +gs review comment main.go:4 -m 'Comment on line 4.' --no-draft +stderr 'Posted comment' +gs review comment main.go:5 -m 'Comment on line 5.' --no-draft +stderr 'Posted comment' + +# Both comments are fresh: neither is outdated yet. +gs review list +! stderr 'outdated' + +# Amend the file so line 4 changes but line 5 does not, then +# update the change with 'gs ca' + 'gs branch submit --force'. +cp $WORK/extra/main-edited.go main.go +git add main.go +gs ca --no-edit +gs branch submit --force + +# Comment 1 (on line 4) is now outdated; comment 2 (on line 5) +# is not. The list output reflects this: +gs review list +stderr 'Comment on line 4' +stderr 'outdated' +stderr 'Comment on line 5' + +# Sanity: the JSON list (used by the VSCode extension) reports +# outdated as the per-comment status. Comment 1 outdated, comment +# 2 still open. +gs review list --json +stdout '"body":"Comment on line 4\."' +stdout '"body":"Comment on line 5\."' +stdout '"status":"outdated"' +stdout '"status":"open"' + +-- repo/main.go -- +package main + +func main() { + println("hello") + println("world") +} +-- extra/main-edited.go -- +package main + +func main() { + println("greetings") + println("world") +}