From cd90a2b9bcbf7b22d19731986a2e87bb9fe4756e Mon Sep 17 00:00:00 2001 From: Amolith Date: Sun, 23 Aug 2026 17:38:39 -0600 Subject: [PATCH] cli: compose comments in external editor --- internal/cli/comment_editor.go | 58 +++++++++++ internal/cli/comment_editor_test.go | 145 ++++++++++++++++++++++++++++ internal/cli/editor.go | 78 +++++++++++++++ internal/cli/editor_test.go | 42 ++++++++ internal/cli/issue_comment.go | 32 +++++- internal/cli/issue_create.go | 11 +-- internal/cli/issue_editor.go | 46 ++------- internal/cli/issue_editor_test.go | 8 +- internal/cli/pr_comment.go | 32 +++++- internal/cli/target.go | 6 +- 10 files changed, 397 insertions(+), 61 deletions(-) create mode 100644 internal/cli/comment_editor.go create mode 100644 internal/cli/comment_editor_test.go create mode 100644 internal/cli/editor.go create mode 100644 internal/cli/editor_test.go diff --git a/internal/cli/comment_editor.go b/internal/cli/comment_editor.go new file mode 100644 index 0000000..58d5da4 --- /dev/null +++ b/internal/cli/comment_editor.go @@ -0,0 +1,58 @@ +package cli + +import ( + "context" + "errors" + "fmt" + "io" + "strings" +) + +const ( + commentDraftSentinel = "" + commentDraftTemplate = "\n" + commentDraftSentinel + "\n" + + "\n" +) + +var errCommentCreationCanceled = errors.New("comment creation canceled") + +type commentDraft struct { + Body string + Path string +} + +func editCommentDraft(ctx context.Context, input io.Reader, output, errorOutput io.Writer) (commentDraft, error) { + edited, err := editDraft(ctx, "comment", "tg-comment-*.md", commentDraftTemplate, input, output, errorOutput) + if err != nil { + return commentDraft{}, err + } + body := parseCommentDraft(edited.Contents) + if strings.TrimSpace(body) == "" { + removeCommentDraft(edited.Path, errorOutput) + return commentDraft{}, errCommentCreationCanceled + } + return commentDraft{Body: body, Path: edited.Path}, nil +} + +func parseCommentDraft(document string) string { + document = strings.ReplaceAll(document, "\r\n", "\n") + lines := strings.Split(document, "\n") + for index, line := range lines { + if line == commentDraftSentinel { + lines = lines[:index] + break + } + } + return strings.Trim(strings.Join(lines, "\n"), "\n") +} + +func removeCommentDraft(path string, errorOutput io.Writer) { + removeDraft(path, "comment", errorOutput) +} + +func commentSubmissionError(err error, draft commentDraft) error { + if draft.Path == "" { + return err + } + return fmt.Errorf("%w; draft saved to %s", err, draft.Path) +} diff --git a/internal/cli/comment_editor_test.go b/internal/cli/comment_editor_test.go new file mode 100644 index 0000000..5c32cc1 --- /dev/null +++ b/internal/cli/comment_editor_test.go @@ -0,0 +1,145 @@ +package cli + +import ( + "context" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/alyraffauf/tg/internal/app" + "github.com/spf13/cobra" +) + +func TestParseCommentDraft(t *testing.T) { + tests := []struct { + name string + document string + want string + }{ + {name: "body", document: "A comment\n\nWith detail\n", want: "A comment\n\nWith detail"}, + {name: "indented code", document: " code\n", want: " code"}, + {name: "trailing spaces preserved", document: "A comment \n", want: "A comment "}, + {name: "CRLF", document: "A comment\r\n\r\nWith detail\r\n", want: "A comment\n\nWith detail"}, + {name: "instructions removed", document: "A comment\n" + commentDraftSentinel + "\nignored", want: "A comment"}, + {name: "other HTML comment retained", document: "", want: ""}, + {name: "empty", document: commentDraftTemplate}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := parseCommentDraft(test.document); got != test.want { + t.Fatalf("parseCommentDraft() = %q, want %q", got, test.want) + } + }) + } +} + +func TestCommentCommandsOpenEditorWhenBodyIsOmitted(t *testing.T) { + tests := []struct { + name string + newCommand func(*testCommentService) *cobra.Command + wantKind string + }{ + {name: "issue", newCommand: func(service *testCommentService) *cobra.Command { return newIssueCommentCommand(service) }, wantKind: "issue"}, + {name: "pull request", newCommand: func(service *testCommentService) *cobra.Command { return newPRCommentCommand(service) }, wantKind: "pull"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + pathLog := filepath.Join(t.TempDir(), "path") + t.Setenv("EDITOR", writeDraftEditor(t, pathLog, "A comment\n", 0)) + service := &testCommentService{} + command := test.newCommand(service) + command.SetArgs([]string{"123", "--repo", "alice.example/project"}) + command.SetIn(nil) + command.SetOut(io.Discard) + command.SetErr(io.Discard) + + if err := command.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if service.kind != test.wantKind || service.body != "A comment" { + t.Fatalf("submission = kind %q, body %q", service.kind, service.body) + } + pathBytes, err := os.ReadFile(pathLog) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(string(pathBytes)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("submitted draft still exists: %v", err) + } + }) + } +} + +func TestCommentCommandExplicitBodyDoesNotOpenEditor(t *testing.T) { + t.Setenv("EDITOR", filepath.Join(t.TempDir(), "missing-editor")) + service := &testCommentService{} + command := newIssueCommentCommand(service) + command.SetArgs([]string{"123", "--body", "Explicit comment", "--repo", "alice.example/project"}) + command.SetOut(io.Discard) + command.SetErr(io.Discard) + + if err := command.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if service.body != "Explicit comment" { + t.Fatalf("body = %q", service.body) + } +} + +func TestCommentCommandRetainsDraftWhenSubmissionFails(t *testing.T) { + pathLog := filepath.Join(t.TempDir(), "path") + t.Setenv("EDITOR", writeDraftEditor(t, pathLog, "Unsubmitted comment\n", 0)) + service := &testCommentService{commentError: errors.New("network unavailable")} + command := newIssueCommentCommand(service) + command.SetArgs([]string{"123", "--repo", "alice.example/project"}) + command.SetOut(io.Discard) + command.SetErr(io.Discard) + + err := command.Execute() + if err == nil || !strings.Contains(err.Error(), "network unavailable; draft saved to") { + t.Fatalf("Execute() error = %v", err) + } + pathBytes, readError := os.ReadFile(pathLog) + if readError != nil { + t.Fatal(readError) + } + path := string(pathBytes) + t.Cleanup(func() { _ = os.Remove(path) }) + if _, statError := os.Stat(path); statError != nil { + t.Fatalf("saved draft %q: %v", path, statError) + } +} + +type testCommentService struct { + commentError error + kind string + body string +} + +func (service *testCommentService) CommentIssue(_ context.Context, _ app.Target, _, body string) (*app.CreatedRecordResult, error) { + service.kind = "issue" + service.body = body + return service.commentResult() +} + +func (service *testCommentService) CommentPull(_ context.Context, _ app.Target, _, body string) (*app.CreatedRecordResult, error) { + service.kind = "pull" + service.body = body + return service.commentResult() +} + +func (service *testCommentService) commentResult() (*app.CreatedRecordResult, error) { + if service.commentError != nil { + return nil, service.commentError + } + return &app.CreatedRecordResult{URI: "at://did:plc:owner/sh.tangled.feed.comment/123"}, nil +} + +func (*testCommentService) TargetFromCWD(context.Context) (app.Target, error) { + return app.Target{Handle: "alice.example", Repo: "project"}, nil +} diff --git a/internal/cli/editor.go b/internal/cli/editor.go new file mode 100644 index 0000000..4715688 --- /dev/null +++ b/internal/cli/editor.go @@ -0,0 +1,78 @@ +package cli + +import ( + "context" + "fmt" + "io" + "os" + + "github.com/charmbracelet/x/editor" +) + +const ( + editorBytesPerMiB = 1 << 20 + maxEditorDraftSize = 100 * editorBytesPerMiB +) + +type editedDraft struct { + Contents string + Path string +} + +func editDraft(ctx context.Context, kind, pattern, template string, input io.Reader, output, errorOutput io.Writer) (editedDraft, error) { + file, err := os.CreateTemp("", pattern) + if err != nil { + return editedDraft{}, fmt.Errorf("create %s draft: %w", kind, err) + } + path := file.Name() + if _, err := file.WriteString(template); err != nil { + _ = file.Close() + _ = os.Remove(path) + return editedDraft{}, fmt.Errorf("write %s draft: %w", kind, err) + } + if err := file.Close(); err != nil { + _ = os.Remove(path) + return editedDraft{}, fmt.Errorf("close %s draft: %w", kind, err) + } + + command, err := editor.CommandContext(ctx, "tg", path) + if err != nil { + _ = os.Remove(path) + return editedDraft{}, fmt.Errorf("open %s editor: %w", kind, err) + } + command.Stdin = input + command.Stdout = output + command.Stderr = errorOutput + if err := command.Run(); err != nil { + return editedDraft{}, fmt.Errorf("run %s editor: %w; draft saved to %s", kind, err, path) + } + + contents, err := readDraftContents(path, kind, maxEditorDraftSize) + if err != nil { + return editedDraft{}, err + } + return editedDraft{Contents: contents, Path: path}, nil +} + +func readDraftContents(path, kind string, sizeMax int64) (string, error) { + file, err := os.Open(path) + if err != nil { + return "", fmt.Errorf("read %s draft: %w; draft saved to %s", kind, err, path) + } + defer file.Close() + + contents, err := io.ReadAll(io.LimitReader(file, sizeMax+1)) + if err != nil { + return "", fmt.Errorf("read %s draft: %w; draft saved to %s", kind, err, path) + } + if int64(len(contents)) > sizeMax { + return "", fmt.Errorf("read %s draft: contents exceed the %d MiB limit; draft saved to %s", kind, sizeMax/editorBytesPerMiB, path) + } + return string(contents), nil +} + +func removeDraft(path, kind string, errorOutput io.Writer) { + if err := os.Remove(path); err != nil { + fmt.Fprintf(errorOutput, "warning: remove %s draft %s: %v\n", kind, path, err) + } +} diff --git a/internal/cli/editor_test.go b/internal/cli/editor_test.go new file mode 100644 index 0000000..4ebb016 --- /dev/null +++ b/internal/cli/editor_test.go @@ -0,0 +1,42 @@ +package cli + +import ( + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestReadDraftContentsStopsAtLimit(t *testing.T) { + path := filepath.Join(t.TempDir(), "draft") + if err := os.WriteFile(path, []byte("123456789"), 0o600); err != nil { + t.Fatal(err) + } + + _, err := readDraftContents(path, "string", 8) + if err == nil || !strings.Contains(err.Error(), "contents exceed") || !strings.Contains(err.Error(), path) { + t.Fatalf("readDraftContents() error = %v", err) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("retained draft %q: %v", path, err) + } +} + +func TestEditDraftRemovesUntouchedDraftWhenEditorCannotOpen(t *testing.T) { + t.Setenv("SNAP_REVISION", "test") + t.Setenv("TMPDIR", t.TempDir()) + + _, err := editDraft(context.Background(), "issue", "tg-issue-*.md", "template", nil, io.Discard, io.Discard) + if err == nil || !strings.Contains(err.Error(), "open issue editor") { + t.Fatalf("editDraft() error = %v", err) + } + entries, err := os.ReadDir(os.Getenv("TMPDIR")) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Fatalf("temporary directory contains %v after editor setup failure", entries) + } +} diff --git a/internal/cli/issue_comment.go b/internal/cli/issue_comment.go index 43bc568..1890845 100644 --- a/internal/cli/issue_comment.go +++ b/internal/cli/issue_comment.go @@ -1,35 +1,59 @@ package cli import ( + "context" + "errors" "fmt" "github.com/alyraffauf/tg/internal/app" "github.com/spf13/cobra" ) -func newIssueCommentCommand(service *app.Service) *cobra.Command { +type issueCommentService interface { + CommentIssue(context.Context, app.Target, string, string) (*app.CreatedRecordResult, error) + TargetFromCWD(context.Context) (app.Target, error) +} + +func newIssueCommentCommand(service issueCommentService) *cobra.Command { var bodyText, bodyFile, repository string command := &cobra.Command{ Use: "comment ", Short: "Add a comment to an issue", + Long: "Add a comment to an issue. When the body is omitted, tg opens $EDITOR.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() body, err := commandBody(bodyText, bodyFile) if err != nil { return err } - if body == "" { + draft := commentDraft{} + openEditor := !cmd.Flags().Changed("body") && !cmd.Flags().Changed("body-file") + if !openEditor && body == "" { return fmt.Errorf("provide --body or --body-file") } - ctx := cmd.Context() target, err := resolveTargetFlag(ctx, repository, service) if err != nil { return err } + if openEditor { + draft, err = editCommentDraft(ctx, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr()) + if errors.Is(err, errCommentCreationCanceled) { + fmt.Fprintln(cmd.ErrOrStderr(), "Comment creation canceled.") + return nil + } + if err != nil { + return err + } + body = draft.Body + } result, err := service.CommentIssue(ctx, target, args[0], body) if err != nil { - return err + return commentSubmissionError(err, draft) + } + if draft.Path != "" { + removeCommentDraft(draft.Path, cmd.ErrOrStderr()) } return output(cmd, result, func(result *app.CreatedRecordResult) { fmt.Fprintf(cmd.OutOrStdout(), "Added comment %s\n", result.URI) diff --git a/internal/cli/issue_create.go b/internal/cli/issue_create.go index 9305ad6..adba68f 100644 --- a/internal/cli/issue_create.go +++ b/internal/cli/issue_create.go @@ -30,7 +30,7 @@ must be blank, and the remaining text is the required body.`, if cmd.Flags().Changed("body") || cmd.Flags().Changed("body-file") { return fmt.Errorf("title is required when --body or --body-file is used") } - target, err := resolveIssueCreateTarget(ctx, repository, service) + target, err := resolveTargetFlag(ctx, repository, service) if err != nil { return err } @@ -56,7 +56,7 @@ must be blank, and the remaining text is the required body.`, if err != nil { return err } - target, err := resolveIssueCreateTarget(ctx, repository, service) + target, err := resolveTargetFlag(ctx, repository, service) if err != nil { return err } @@ -74,10 +74,3 @@ must be blank, and the remaining text is the required body.`, command.Flags().StringVarP(&repository, "repo", "R", "", "Target repository as handle/repo") return command } - -func resolveIssueCreateTarget(ctx context.Context, repository string, service issueCreateService) (app.Target, error) { - if repository != "" { - return app.ParseTarget(repository) - } - return service.TargetFromCWD(ctx) -} diff --git a/internal/cli/issue_editor.go b/internal/cli/issue_editor.go index 789d230..8f20324 100644 --- a/internal/cli/issue_editor.go +++ b/internal/cli/issue_editor.go @@ -5,10 +5,7 @@ import ( "errors" "fmt" "io" - "os" "strings" - - "github.com/charmbracelet/x/editor" ) const ( @@ -26,46 +23,19 @@ type issueDraft struct { } func editIssueDraft(ctx context.Context, input io.Reader, output, errorOutput io.Writer) (issueDraft, error) { - file, err := os.CreateTemp("", "tg-issue-*.md") - if err != nil { - return issueDraft{}, fmt.Errorf("create issue draft: %w", err) - } - path := file.Name() - if _, err := file.WriteString(issueDraftTemplate); err != nil { - _ = file.Close() - _ = os.Remove(path) - return issueDraft{}, fmt.Errorf("write issue draft: %w", err) - } - if err := file.Close(); err != nil { - _ = os.Remove(path) - return issueDraft{}, fmt.Errorf("close issue draft: %w", err) - } - - command, err := editor.CommandContext(ctx, "tg", path) + edited, err := editDraft(ctx, "issue", "tg-issue-*.md", issueDraftTemplate, input, output, errorOutput) if err != nil { - _ = os.Remove(path) - return issueDraft{}, fmt.Errorf("open issue editor: %w", err) - } - command.Stdin = input - command.Stdout = output - command.Stderr = errorOutput - if err := command.Run(); err != nil { - return issueDraft{}, fmt.Errorf("run issue editor: %w; draft saved to %s", err, path) - } - - contents, err := os.ReadFile(path) - if err != nil { - return issueDraft{}, fmt.Errorf("read issue draft: %w; draft saved to %s", err, path) + return issueDraft{}, err } - title, body, err := parseIssueDraft(string(contents)) + title, body, err := parseIssueDraft(edited.Contents) if errors.Is(err, errIssueCreationCanceled) { - removeIssueDraft(path, errorOutput) + removeIssueDraft(edited.Path, errorOutput) return issueDraft{}, err } if err != nil { - return issueDraft{}, fmt.Errorf("parse issue draft: %w; draft saved to %s", err, path) + return issueDraft{}, fmt.Errorf("parse issue draft: %w; draft saved to %s", err, edited.Path) } - return issueDraft{Title: title, Body: body, Path: path}, nil + return issueDraft{Title: title, Body: body, Path: edited.Path}, nil } func parseIssueDraft(document string) (string, string, error) { @@ -106,7 +76,5 @@ func parseIssueDraft(document string) (string, string, error) { } func removeIssueDraft(path string, errorOutput io.Writer) { - if err := os.Remove(path); err != nil { - fmt.Fprintf(errorOutput, "warning: remove issue draft %s: %v\n", path, err) - } + removeDraft(path, "issue", errorOutput) } diff --git a/internal/cli/issue_editor_test.go b/internal/cli/issue_editor_test.go index 0e19268..b9c8313 100644 --- a/internal/cli/issue_editor_test.go +++ b/internal/cli/issue_editor_test.go @@ -85,7 +85,7 @@ func TestEditIssueDraftLifecycle(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { pathLog := filepath.Join(t.TempDir(), "path") - editorPath := writeIssueEditor(t, pathLog, test.document, test.exitStatus) + editorPath := writeDraftEditor(t, pathLog, test.document, test.exitStatus) t.Setenv("EDITOR", editorPath) draft, err := editIssueDraft(context.Background(), nil, io.Discard, io.Discard) @@ -128,7 +128,7 @@ func TestEditIssueDraftLifecycle(t *testing.T) { func TestIssueCreateRetainsEditedDraftWhenSubmissionFails(t *testing.T) { pathLog := filepath.Join(t.TempDir(), "path") - t.Setenv("EDITOR", writeIssueEditor(t, pathLog, "Bug report\n\nDetails\n", 0)) + t.Setenv("EDITOR", writeDraftEditor(t, pathLog, "Bug report\n\nDetails\n", 0)) service := &testIssueCreateService{createError: errors.New("network unavailable")} command := newIssueCreateCommand(service) command.SetArgs([]string{"--repo", "alice.example/project"}) @@ -156,7 +156,7 @@ func TestIssueCreateRetainsEditedDraftWhenSubmissionFails(t *testing.T) { func TestIssueCreateRemovesEditedDraftAfterSubmission(t *testing.T) { pathLog := filepath.Join(t.TempDir(), "path") - t.Setenv("EDITOR", writeIssueEditor(t, pathLog, "Bug report\n\nDetails\n", 0)) + t.Setenv("EDITOR", writeDraftEditor(t, pathLog, "Bug report\n\nDetails\n", 0)) service := &testIssueCreateService{} command := newIssueCreateCommand(service) command.SetArgs([]string{"--repo", "alice.example/project"}) @@ -239,7 +239,7 @@ func (*testIssueCreateService) TargetFromCWD(context.Context) (app.Target, error return app.Target{Handle: "alice.example", Repo: "project"}, nil } -func writeIssueEditor(t *testing.T, pathLog, document string, exitStatus int) string { +func writeDraftEditor(t *testing.T, pathLog, document string, exitStatus int) string { t.Helper() script := filepath.Join(t.TempDir(), "editor.sh") contents := "#!/bin/sh\n" + diff --git a/internal/cli/pr_comment.go b/internal/cli/pr_comment.go index 604d92d..06a486c 100644 --- a/internal/cli/pr_comment.go +++ b/internal/cli/pr_comment.go @@ -1,35 +1,59 @@ package cli import ( + "context" + "errors" "fmt" "github.com/alyraffauf/tg/internal/app" "github.com/spf13/cobra" ) -func newPRCommentCommand(service *app.Service) *cobra.Command { +type pullCommentService interface { + CommentPull(context.Context, app.Target, string, string) (*app.CreatedRecordResult, error) + TargetFromCWD(context.Context) (app.Target, error) +} + +func newPRCommentCommand(service pullCommentService) *cobra.Command { var bodyText, bodyFile, repository string command := &cobra.Command{ Use: "comment ", Short: "Add a comment to a pull request", + Long: "Add a comment to a pull request. When the body is omitted, tg opens $EDITOR.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() body, err := commandBody(bodyText, bodyFile) if err != nil { return err } - if body == "" { + draft := commentDraft{} + openEditor := !cmd.Flags().Changed("body") && !cmd.Flags().Changed("body-file") + if !openEditor && body == "" { return fmt.Errorf("provide --body or --body-file") } - ctx := cmd.Context() target, err := resolveTargetFlag(ctx, repository, service) if err != nil { return err } + if openEditor { + draft, err = editCommentDraft(ctx, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr()) + if errors.Is(err, errCommentCreationCanceled) { + fmt.Fprintln(cmd.ErrOrStderr(), "Comment creation canceled.") + return nil + } + if err != nil { + return err + } + body = draft.Body + } result, err := service.CommentPull(ctx, target, args[0], body) if err != nil { - return err + return commentSubmissionError(err, draft) + } + if draft.Path != "" { + removeCommentDraft(draft.Path, cmd.ErrOrStderr()) } return output(cmd, result, func(result *app.CreatedRecordResult) { fmt.Fprintf(cmd.OutOrStdout(), "Added comment %s\n", result.URI) diff --git a/internal/cli/target.go b/internal/cli/target.go index cf15b45..36199af 100644 --- a/internal/cli/target.go +++ b/internal/cli/target.go @@ -9,6 +9,10 @@ import ( "github.com/alyraffauf/tg/internal/app" ) +type cwdTargetResolver interface { + TargetFromCWD(context.Context) (app.Target, error) +} + // getwd returns the current working directory, wrapping the common error. func getwd() (string, error) { dir, err := os.Getwd() @@ -29,7 +33,7 @@ func resolveTarget(ctx context.Context, args []string, service *app.Service) (ap // resolveTargetFlag returns the target from a --repo flag value, or by // detecting the git remote in the CWD when the flag is unset. -func resolveTargetFlag(ctx context.Context, repoFlag string, service *app.Service) (app.Target, error) { +func resolveTargetFlag(ctx context.Context, repoFlag string, service cwdTargetResolver) (app.Target, error) { if repoFlag != "" { return app.ParseTarget(repoFlag) } -- 2.51.2