From 45c564811ece7c317cc02b43738a2be9e9b7f26f Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 18 Oct 2026 17:38:51 +0200 Subject: [PATCH] fix: address verified review findings (comments-views-search-1) Signed-off-by: bircni --- docs/migration-1.0.md | 1 + internal/app/comments.go | 13 ++++++++----- internal/app/comments_test.go | 24 ++++++++++++++++++++++++ internal/app/discussion_view.go | 5 +++-- internal/app/discussion_view_test.go | 6 ++++++ internal/app/issues.go | 3 +++ internal/app/pulls.go | 3 +++ internal/services/comments.go | 13 ++++++++++++- internal/services/comments_test.go | 8 ++++++++ 9 files changed, 68 insertions(+), 8 deletions(-) diff --git a/docs/migration-1.0.md b/docs/migration-1.0.md index 15122f2c..3d69c7cc 100644 --- a/docs/migration-1.0.md +++ b/docs/migration-1.0.md @@ -219,6 +219,7 @@ Subcommands: | `actions workflows dispatch` | `workflow run` | | `webhooks update` | `webhook edit` | | `comments add [body]` | `issue comment --body TEXT` (also `pr comment`); `--edit-last` and `--delete-last` change your last comment | +| `comments edit [body]`, `comments delete ...`, a body piped on stdin | `issue comment edit --body TEXT`, one `issue comment delete ` per comment, `--body-file -` to read stdin | | `notifications read\|unread [all\|id]` | `notification mark-read\|mark-unread [id]...`; use `--all` for every filtered notification | | `ssh-keys add` | `ssh-key create` | | `login helper setup` | `auth setup-git` | diff --git a/internal/app/comments.go b/internal/app/comments.go index 3e23eed5..0348d275 100644 --- a/internal/app/comments.go +++ b/internal/app/comments.go @@ -246,12 +246,12 @@ func (a *application) writeComments(cmd *cobra.Command, t target, n int64, items text.WriteString(terminal.Table(nil, [][]terminal.Cell{{heading, {Text: meta, Muted: true}}}, d)) body := strings.TrimSpace(comment.Body) if d.Markdown { - body = strings.TrimSpace(terminal.Markdown(body, d)) + // Glamour already indents every line by its margin, which TrimSpace would strip from the first. + text.WriteString(strings.Trim(terminal.Markdown(body, d), "\n")) } else { - body = terminal.SafeText(body) - } - for line := range strings.Lines(body) { - text.WriteString(" " + line) + for line := range strings.Lines(terminal.SafeText(body)) { + text.WriteString(" " + line) + } } text.WriteString("\n") } @@ -283,6 +283,9 @@ $ tea %[1]s comment list %[2]s`, words.group, words.example), Annotations: map[s if deleteLast && (cmd.Flags().Changed("body") || cmd.Flags().Changed("body-file") || flags.useEditor) { return clierr.Wrap(clierr.Usage, errors.New("--delete-last takes no comment text")) } + if deleteLast && a.options.json != "" { + return clierr.Wrap(clierr.Usage, errors.New("comment deletion does not return a resource; --json and --jq are not supported")) + } t, n, err := a.resolveRef(cmd, args[0], kind) if err != nil { return err diff --git a/internal/app/comments_test.go b/internal/app/comments_test.go index fa66c453..f0b8bc65 100644 --- a/internal/app/comments_test.go +++ b/internal/app/comments_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "gitea.dev/tea/internal/settings" "github.com/stretchr/testify/require" ) @@ -151,6 +152,7 @@ func TestCommentGroupCommentsLikeGh(t *testing.T) { {[]string{"issue", "comment", "craete", "1"}, `unknown command "craete" for "tea issue comment"; did you mean create?`}, {[]string{"issue", "comment", "-b", "x"}, "missing issue number"}, {[]string{"issue", "comment", "1", "--edit-last", "--delete-last"}, "--edit-last and --delete-last cannot be combined"}, + {[]string{"issue", "comment", "1", "--delete-last", "--json"}, "comment deletion does not return a resource; --json and --jq are not supported"}, {[]string{"issue", "comment", "1"}, "missing required input: supply --body, --body-file or --editor"}, {[]string{"pr", "comment", "1", "-b", "x"}, "#1 in alice/project is an issue, not a pull request; use tea issue comment 1"}, } { @@ -167,4 +169,26 @@ func TestCommentGroupCommentsLikeGh(t *testing.T) { require.Equal(t, "#79 alice • about 2 hours ago\n First **one**\n\n"+ "#80 bob • about 1 hour ago (edited)\n Second\n line\n\n"+ "#81 bob • about 50 minutes ago\n Third\n", out.String()) + // Rendered Markdown carries its own two-space margin, so every line of a body is indented alike. + require.NoError(t, deps.Settings.Update(settings.TargetPreferences, func(s *settings.Snapshot) error { + s.Preferences.Markdown, s.Preferences.Hyperlinks = "auto", "never" + return nil + })) + require.NoError(t, run("issue", "comment", "list", "1")) + require.Contains(t, out.String(), "\n Second\n line\n") + require.NotContains(t, out.String(), " line") +} + +func TestViewCommentsCannotBeCombinedWithJSON(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + deps, out, _ := serverDeps(t, server.URL) + err := Execute(context.Background(), deps, []string{"issue", "view", "1", "-R", "alice/project", "--comments", "--json"}) + require.ErrorContains(t, err, "use tea issue comment list --json") + err = Execute(context.Background(), deps, []string{"pr", "view", "13", "-R", "alice/project", "--comments", "--json"}) + require.ErrorContains(t, err, "tea pr review list --json") + require.Empty(t, out.String()) } diff --git a/internal/app/discussion_view.go b/internal/app/discussion_view.go index a65a09f0..4a8b4998 100644 --- a/internal/app/discussion_view.go +++ b/internal/app/discussion_view.go @@ -88,13 +88,14 @@ func issueView(item services.Issue, repo string, dependencies services.IssueDepe } } -// dueText words a due date for people: "Dec 24, 2026 (in about 2 months)". +// dueText words a due date for people: "Dec 24, 2026 (in about 2 months)". The day is the one in the +// offset Gitea returned: it stores a due date as the end of a day in its own time zone. func dueText(now time.Time, raw string) string { due := terminal.ParseTime(raw) if due.IsZero() { return raw } - return due.UTC().Format("Jan 2, 2006") + " (" + terminal.RelativeTime(now, due) + ")" + return due.Format("Jan 2, 2006") + " (" + terminal.RelativeTime(now, due) + ")" } // issueReferences lists issues as #N, qualifying those from other repositories. diff --git a/internal/app/discussion_view_test.go b/internal/app/discussion_view_test.go index c5c8d9da..0f3caa8f 100644 --- a/internal/app/discussion_view_test.go +++ b/internal/app/discussion_view_test.go @@ -189,3 +189,9 @@ func TestPullViewStates(t *testing.T) { }) } } + +func TestDueTextKeepsTheDayInTheServerOffset(t *testing.T) { + now := time.Date(2026, 10, 11, 20, 0, 0, 0, time.UTC) + require.Equal(t, "Dec 24, 2026 (in about 2 months)", dueText(now, "2026-12-24T23:59:59-05:00")) + require.Equal(t, "Dec 24, 2026 (in about 2 months)", dueText(now, "2026-12-24T23:59:59Z")) +} diff --git a/internal/app/issues.go b/internal/app/issues.go index 0975f1cc..c8fe3f9b 100644 --- a/internal/app/issues.go +++ b/internal/app/issues.go @@ -168,6 +168,9 @@ func (a *application) issueViewCommand() *cobra.Command { conversation follows the body.`, Example: `$ tea issue view 12 $ tea issue view 12 --comments $ tea issue view https://gitea.com/OWNER/REPO/issues/12 --web`, Annotations: map[string]string{"tea.arguments": issueArguments}, Args: argsExact(1), RunE: func(cmd *cobra.Command, args []string) error { + if comments && a.options.json != "" { + return clierr.Wrap(clierr.Usage, errors.New("--comments cannot be combined with --json or --jq; use tea issue comment list --json")) + } target, n, err := a.resolveRef(cmd, args[0], issueRef) if err != nil { return err diff --git a/internal/app/pulls.go b/internal/app/pulls.go index fb06f358..26faf194 100644 --- a/internal/app/pulls.go +++ b/internal/app/pulls.go @@ -143,6 +143,9 @@ func (a *application) jsonSelects(fields ...string) bool { func (a *application) pullViewCommand() *cobra.Command { var web, comments bool cmd := &cobra.Command{Use: "view [ | | ]", Short: "View a pull request", Annotations: pullArgumentsAnnotation(nil), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { + if comments && a.options.json != "" { + return clierr.Wrap(clierr.Usage, errors.New("--comments cannot be combined with --json or --jq; use tea pr comment list --json or tea pr review list --json")) + } t, n, err := a.pullSelector(cmd, args) if err != nil { return err diff --git a/internal/services/comments.go b/internal/services/comments.go index c3e7701a..58efcd71 100644 --- a/internal/services/comments.go +++ b/internal/services/comments.go @@ -5,6 +5,8 @@ package services import ( "context" + "fmt" + "net/http" "strconv" gitea "gitea.dev/sdk" @@ -61,9 +63,18 @@ func (s Comments) Page(ctx context.Context, n int64, limit, page int) ([]Comment // View retrieves a comment by its repository-scoped ID. func (s Comments) View(ctx context.Context, id int64) (Comment, error) { + status := 0 item, err := repoConvert(ctx, s.Client, s.Repository, commentFrom, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Comment, *gitea.Response, error) { - return c.Issues.GetIssueComment(ctx, owner, repo, id) + comment, resp, err := c.Issues.GetIssueComment(ctx, owner, repo, id) + if resp != nil && resp.Response != nil { + status = resp.StatusCode + } + return comment, resp, err }) + if status == http.StatusNoContent { + // Gitea answers without a body for the comments that are not plain ones, such as inline review comments. + return Comment{}, fmt.Errorf("comment %d in %s is not a plain comment, such as an inline review comment, which this command cannot show", id, s.Repository) + } return item, Classify(err, "comment", strconv.FormatInt(id, 10), s.Repository) } diff --git a/internal/services/comments_test.go b/internal/services/comments_test.go index 7c9d93eb..07ee32a6 100644 --- a/internal/services/comments_test.go +++ b/internal/services/comments_test.go @@ -61,3 +61,11 @@ func TestRemoveMilestoneRejectsDifferentMembership(t *testing.T) { require.EqualError(t, err, `#7 is not in milestone "v1.0"`) require.Zero(t, mutations) } + +func TestCommentViewExplainsAnEmptyAnswerForReviewComments(t *testing.T) { + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + _, err := Comments{Client: client, Repository: "alice/project"}.View(context.Background(), 9) + require.EqualError(t, err, "comment 9 in alice/project is not a plain comment, such as an inline review comment, which this command cannot show") +}