From e5796af9e97fe833eb1b09b4fe9d1bdad8a15971 Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 12 Oct 2026 00:02:25 +0200 Subject: [PATCH] feat(attachment): download attachments under their own name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit issue attachment download (and pr attachment download) required --output-file. It now writes ./ like gh release download, refusing to replace an existing file unless --clobber (or --skip-existing to keep it), with -O/--output and -D/--dir from the shared download flags. attachment list shows ID NAME SIZE DOWNLOADS CREATED with readable sizes, attachment view is a view, and upload, rename and delete report ✓ lines; delete names the attachment in its confirmation. The commands accept issue and pull request URLs and have help text with examples. Signed-off-by: bircni --- internal/app/app_test.go | 14 -- internal/app/issue_attachments.go | 241 ++++++++++++++----------- internal/app/issue_attachments_test.go | 51 ++++++ internal/app/validation_test.go | 2 - internal/services/issue_attachments.go | 10 +- 5 files changed, 192 insertions(+), 126 deletions(-) diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 6d5555f2..f09c5f6e 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -418,23 +418,9 @@ const ( shortsToFix = ` ` examplesToWrite = ` - issue attachment delete - issue attachment download - issue attachment edit - issue attachment list - issue attachment upload - issue attachment view - pr attachment delete - pr attachment download - pr attachment edit - pr attachment list - pr attachment upload - pr attachment view search issue search pr ` longsToWrite = ` - issue attachment - pr attachment ` ) diff --git a/internal/app/issue_attachments.go b/internal/app/issue_attachments.go index 4184562a..c4dc1f39 100644 --- a/internal/app/issue_attachments.go +++ b/internal/app/issue_attachments.go @@ -16,104 +16,81 @@ import ( "github.com/spf13/cobra" ) +// attachmentName names an attachment in messages: "attachment 9 (log.txt) on alice/x#1". +func attachmentName(t target, n int64, item services.IssueAttachment) string { + return fmt.Sprintf("attachment %d (%s) on %s#%d", item.ID, item.Name, t.Repo, n) +} + +func attachmentRow(item services.IssueAttachment) []terminal.Cell { + return []terminal.Cell{ + {Text: strconv.FormatInt(item.ID, 10), Link: item.URL}, + {Text: item.Name}, + {Text: terminal.Bytes(item.Size)}, + {Text: strconv.FormatInt(item.DownloadCount, 10)}, + {Time: terminal.ParseTime(item.CreatedAt)}, + } +} + +func attachmentView(item services.IssueAttachment) terminal.View { + return terminal.View{ + Title: item.Name, ID: "#" + strconv.FormatInt(item.ID, 10), + Fields: []terminal.Field{ + {Label: "Size", Value: terminal.Bytes(item.Size)}, + {Label: "Downloads", Value: strconv.FormatInt(item.DownloadCount, 10)}, + {Label: "Created", Value: item.CreatedAt}, + }, + Kind: "attachment", URL: item.URL, + } +} + func (a *application) issueAttachmentCommand(kind refKind) *cobra.Command { - group := &cobra.Command{Use: "attachment", Short: "Inspect issue or pull request attachments"} + words := commentWords(kind) + group := &cobra.Command{Use: "attachment", Short: "Manage the attachments of " + words.noun + "s", Long: fmt.Sprintf(`List, view, upload, download, rename and delete the files attached to %s. Attachments +are addressed by the %s number and the attachment ID that attachment list shows.`, article(words.noun), words.noun)} + annotations := map[string]string{"tea.arguments": words.arguments} + // discussionTarget resolves the issue or pull request argument of an attachment command. + discussionTarget := func(cmd *cobra.Command, raw string) (target, int64, error) { + t, n, err := a.resolveRef(cmd, raw, kind) + if err == nil && kind == pullRef { + err = requirePull(cmd, t, n) + } + return t, n, err + } + completeFirst := a.completeDiscussions(kind) + var limit, page int - list := &cobra.Command{Use: "list ", Short: "List attachments", Args: argsExact(1), RunE: func(cmd *cobra.Command, args []string) error { - n, err := number(args[0]) + list := &cobra.Command{Use: "list { | }", Short: "List attachments", Example: fmt.Sprintf("$ tea %s attachment list %s", words.group, words.example), Annotations: annotations, Args: argsExact(1), ValidArgsFunction: completeFirst, RunE: func(cmd *cobra.Command, args []string) error { + t, n, err := discussionTarget(cmd, args[0]) if err != nil { return err } - target, err := a.resolve(cmd.Context(), true) + items, err := (services.Issues{Client: t.client, Repository: t.Repo}).Attachments(cmd.Context(), n, limit, page) if err != nil { return err } - if kind == pullRef { - if err := requirePull(cmd, target, n); err != nil { - return err - } - } - items, err := (services.Issues{Client: target.client, Repository: target.Repo}).Attachments(cmd.Context(), n, limit, page) - if err != nil { - return err - } - return writeList(a, cmd, items, attachmentRow) + where := fmt.Sprintf("on %s#%d", t.Repo, n) + return writeTable(a, cmd, items, listSpec[services.IssueAttachment]{ + Columns: []terminal.Column{{Header: "ID"}, {Header: "Name", Flex: true}, {Header: "Size"}, {Header: "Downloads"}, {Header: "Created"}}, + Row: attachmentRow, Empty: "no attachments " + where, Context: "attachments " + where, + }, listPage{Number: page}) }} pageFlags(list, &limit, &page) - view := &cobra.Command{Use: "view ", Short: "View attachment metadata", Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error { - n, err := number(args[0]) + + view := &cobra.Command{Use: "view { | } ", Short: "View an attachment", Example: fmt.Sprintf("$ tea %s attachment view %s 9", words.group, words.example), Annotations: annotations, Args: argsExact(2), ValidArgsFunction: completeFirst, RunE: func(cmd *cobra.Command, args []string) error { + t, n, id, err := a.attachmentArgs(cmd, args, discussionTarget) if err != nil { return err } - id, err := number(args[1]) + item, err := (services.Issues{Client: t.client, Repository: t.Repo}).Attachment(cmd.Context(), n, id) if err != nil { return err } - target, err := a.resolve(cmd.Context(), true) - if err != nil { - return err - } - if kind == pullRef { - if err := requirePull(cmd, target, n); err != nil { - return err - } - } - item, err := (services.Issues{Client: target.client, Repository: target.Repo}).Attachment(cmd.Context(), n, id) - if err != nil { - return err - } - return writeItem(a, cmd, item, attachmentRow) + return writeView(a, cmd, item, attachmentView) }} - for _, verb := range []string{"edit", "delete"} { - var name string - command := &cobra.Command{Use: verb + " ", Short: titleCase(verb) + " an attachment", Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error { - n, err := number(args[0]) - if err != nil { - return err - } - id, err := number(args[1]) - if err != nil { - return err - } - if verb == "edit" && strings.TrimSpace(name) == "" { - return clierr.Wrap(clierr.Usage, errors.New("supply a nonempty --name")) - } - target, err := a.resolve(cmd.Context(), true) - if err != nil { - return err - } - if kind == pullRef { - if err := requirePull(cmd, target, n); err != nil { - return err - } - } - service := services.Issues{Client: target.client, Repository: target.Repo} - if verb == "delete" { - if err := a.confirm(cmd, fmt.Sprintf("Delete attachment %d on %s#%d", id, target.Repo, n)); err != nil { - return err - } - return service.DeleteAttachment(cmd.Context(), n, id) - } - item, err := service.RenameAttachment(cmd.Context(), n, id, name) - if err != nil { - return err - } - return writeItem(a, cmd, item, attachmentRow) - }} - if verb == "edit" { - command.Flags().StringVar(&name, "name", "", "New attachment filename") - } - if verb == "delete" { - command.Annotations = textOutput("attachment deletion does not return a resource") - } - group.AddCommand(command) - } + var uploadName string - upload := &cobra.Command{Use: "upload ", Short: "Upload one attachment", Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error { - n, err := number(args[0]) - if err != nil { - return err - } + upload := &cobra.Command{Use: "upload { | } ", Short: "Attach a file", Example: fmt.Sprintf(`$ tea %[1]s attachment upload %[2]s crash.log +$ tea %[1]s attachment upload %[2]s screenshot.png --name before.png`, words.group, words.example), Annotations: annotations, Args: argsExact(2), ValidArgsFunction: completeFirst, RunE: func(cmd *cobra.Command, args []string) error { name := uploadName if cmd.Flags().Changed("name") && strings.TrimSpace(name) == "" { return clierr.Wrap(clierr.Usage, errors.New("--name cannot be empty")) @@ -126,56 +103,108 @@ func (a *application) issueAttachmentCommand(kind refKind) *cobra.Command { if err != nil { return err } - target, err := a.resolve(cmd.Context(), true) + t, n, err := discussionTarget(cmd, args[0]) if err != nil { return err } - if kind == pullRef { - if err := requirePull(cmd, target, n); err != nil { - return err - } - } - item, err := (services.Issues{Client: target.client, Repository: target.Repo}).UploadAttachment(cmd.Context(), n, name, files[0]) + item, err := (services.Issues{Client: t.client, Repository: t.Repo}).UploadAttachment(cmd.Context(), n, name, files[0]) if err != nil { return err } - return writeItem(a, cmd, item, attachmentRow) + if a.options.json != "" { + return a.writeJSON(cmd, item) + } + a.done(cmd, "Uploaded %s (%s) to %s#%d", item.Name, terminal.Bytes(item.Size), t.Repo, n) + return a.printURL(cmd, item.URL) }} - upload.Flags().StringVar(&uploadName, "name", "", "Uploaded filename (default: local filename)") - var output string - download := &cobra.Command{Use: "download ", Short: "Download an attachment to a file or stdout", Annotations: textOutput("attachment downloads are binary"), Args: argsExact(2), RunE: func(cmd *cobra.Command, args []string) error { - n, err := number(args[0]) + upload.Flags().StringVar(&uploadName, "name", "", "File `name` of the attachment (default: the local file name)") + + var newName string + edit := &cobra.Command{Use: "edit { | } ", Short: "Rename an attachment", Example: fmt.Sprintf("$ tea %s attachment edit %s 9 --name crash-2026-10-11.log", words.group, words.example), Annotations: annotations, Args: argsExact(2), ValidArgsFunction: completeFirst, RunE: func(cmd *cobra.Command, args []string) error { + if strings.TrimSpace(newName) == "" { + return clierr.Wrap(clierr.Usage, errors.New("supply a nonempty --name")) + } + t, n, id, err := a.attachmentArgs(cmd, args, discussionTarget) if err != nil { return err } - id, err := number(args[1]) + item, err := (services.Issues{Client: t.client, Repository: t.Repo}).RenameAttachment(cmd.Context(), n, id, newName) if err != nil { return err } - if err := validateDownloadDestination(output); err != nil { - return err + if a.options.json != "" { + return a.writeJSON(cmd, item) } - target, err := a.resolve(cmd.Context(), true) + a.done(cmd, "Renamed %s", attachmentName(t, n, item)) + return nil + }} + edit.Flags().StringVar(&newName, "name", "", "New file `name`") + + remove := &cobra.Command{Use: "delete { | } ", Short: "Delete an attachment", Example: fmt.Sprintf("$ tea %s attachment delete %s 9 --yes", words.group, words.example), Annotations: map[string]string{"tea.arguments": words.arguments, "tea.confirm": "true", "tea.text": "attachment deletion does not return a resource"}, Args: argsExact(2), ValidArgsFunction: completeFirst, RunE: func(cmd *cobra.Command, args []string) error { + t, n, id, err := a.attachmentArgs(cmd, args, discussionTarget) if err != nil { return err } - if kind == pullRef { - if err := requirePull(cmd, target, n); err != nil { - return err - } + service := services.Issues{Client: t.client, Repository: t.Repo} + item, err := service.Attachment(cmd.Context(), n, id) + if err != nil { + return err } - body, err := (services.Issues{Client: target.client, Repository: target.Repo}).DownloadAttachment(cmd.Context(), n, id) + name := attachmentName(t, n, item) + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + name + "?", NonTTY: "delete " + name}); err != nil { + return err + } + if err := service.DeleteAttachment(cmd.Context(), n, id); err != nil { + return err + } + a.done(cmd, "Deleted %s", name) + return nil + }} + + var destination downloadFlags + download := &cobra.Command{Use: "download { | } ", Short: "Download an attachment", Long: `Download an attachment into the current directory under its own name, or where -O/--output +or -D/--dir say. An existing file is kept unless --clobber replaces it or --skip-existing +skips the download.`, Example: fmt.Sprintf(`$ tea %[1]s attachment download %[2]s 9 +$ tea %[1]s attachment download %[2]s 9 --output crash.log --clobber +# Write the file to standard output +$ tea %[1]s attachment download %[2]s 9 -O - | less`, words.group, words.example), Annotations: map[string]string{"tea.arguments": words.arguments, "tea.text": "attachment downloads are binary"}, Args: argsExact(2), ValidArgsFunction: completeFirst, RunE: func(cmd *cobra.Command, args []string) error { + t, n, id, err := a.attachmentArgs(cmd, args, discussionTarget) + if err != nil { + return err + } + item, body, err := (services.Issues{Client: t.client, Repository: t.Repo}).DownloadAttachment(cmd.Context(), n, id) if err != nil { return err } defer body.Close() - return writeDownload(cmd, body, output) + path, skip, err := destination.destination(item.Name) + switch { + case err != nil: + return err + case skip: + a.warn(cmd, "Skipped %s: %s exists", attachmentName(t, n, item), path) + return nil + } + if err := a.download(cmd, destination, path, body); err != nil { + return err + } + if path != "-" { + a.done(cmd, "Downloaded %s (%s) to %s", item.Name, terminal.Bytes(item.Size), path) + } + return nil }} - download.Flags().StringVar(&output, "output-file", "", "Destination file (must not exist), or - for stdout") - group.AddCommand(list, view, upload, download) + destination.register(download) + group.AddCommand(list, view, upload, edit, remove, download) return group } -func attachmentRow(item services.IssueAttachment) []terminal.Cell { - return terminal.Row(strconv.FormatInt(item.ID, 10), item.Name, strconv.FormatInt(item.Size, 10), item.URL) +// attachmentArgs resolves the issue or pull request and attachment ID arguments of an attachment +// command, checking the ID before any request. +func (a *application) attachmentArgs(cmd *cobra.Command, args []string, resolve func(*cobra.Command, string) (target, int64, error)) (target, int64, int64, error) { + id, err := number(args[1]) + if err != nil { + return target{}, 0, 0, err + } + t, n, err := resolve(cmd, args[0]) + return t, n, id, err } diff --git a/internal/app/issue_attachments_test.go b/internal/app/issue_attachments_test.go index c1ff740c..9bf111d9 100644 --- a/internal/app/issue_attachments_test.go +++ b/internal/app/issue_attachments_test.go @@ -7,7 +7,9 @@ import ( "context" "encoding/json" "net/http" + "os" "testing" + "time" "gitea.dev/tea/internal/clierr" "github.com/stretchr/testify/require" @@ -49,6 +51,10 @@ func TestAttachmentRenameAndConfirmedDeletion(t *testing.T) { _, _ = w.Write([]byte(`{"number":1,"pull_request":{}}`)) return } + if r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/alice/project/issues/1/assets/1" { + _, _ = w.Write([]byte(`{"id":1,"name":"log.txt"}`)) + return + } require.Equal(t, "/api/v1/repos/alice/project/issues/1/assets/1", r.URL.Path) mutations++ if r.Method == http.MethodPatch { @@ -81,3 +87,48 @@ func TestAttachmentRenameAndConfirmedDeletion(t *testing.T) { server.Close() } } + +func TestAttachmentTablesDownloadsAndDoneLines(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method + " " + r.URL.Path { + case "GET /api/v1/repos/alice/project/issues/1/assets": + _, _ = w.Write([]byte(`[{"id":9,"name":"crash.log","size":2048,"download_count":3,"created_at":"2026-10-11T18:00:00Z","browser_download_url":"https://example.org/a/9"}]`)) + case "GET /api/v1/repos/alice/project/issues/1/assets/9": + _, _ = w.Write([]byte(`{"id":9,"name":"crash.log","size":5,"uuid":"abc"}`)) + case "GET /attachments/abc": + _, _ = w.Write([]byte("hello")) + case "DELETE /api/v1/repos/alice/project/issues/1/assets/9": + w.WriteHeader(http.StatusNoContent) + default: + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) + } + })) + defer server.Close() + deps, out, diagnostics := serverDeps(t, server.URL) + deps.Now = func() time.Time { return time.Date(2026, 10, 11, 20, 0, 0, 0, time.UTC) } + deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1", "TEA_HYPERLINKS": "never"}) + run := func(tty bool, args ...string) error { + out.Reset() + diagnostics.Reset() + deps.Streams.OutTTY = tty + return Execute(context.Background(), deps, append(args, "-R", "alice/project", "--no-input")) + } + require.NoError(t, run(true, "issue", "attachment", "list", "1")) + require.Equal(t, "Showing 1 of 1 attachments on alice/project#1\n\n"+ + "ID NAME SIZE DOWNLOADS CREATED\n"+ + "9 crash.log 2.0 KiB 3 about 2 hours ago\n", out.String()) + t.Chdir(t.TempDir()) + require.NoError(t, run(true, "issue", "attachment", "download", "1", "9")) + require.Equal(t, "✓ Downloaded crash.log (5 B) to crash.log\n", diagnostics.String()) + data, err := os.ReadFile("crash.log") + require.NoError(t, err) + require.Equal(t, "hello", string(data)) + require.ErrorContains(t, run(true, "issue", "attachment", "download", "1", "9"), "crash.log already exists; pass --clobber to replace it or --skip-existing to keep it") + require.NoError(t, run(true, "issue", "attachment", "download", "1", "9", "--skip-existing")) + require.Equal(t, "! Skipped attachment 9 (crash.log) on alice/project#1: crash.log exists\n", diagnostics.String()) + require.NoError(t, run(false, "issue", "attachment", "download", "1", "9", "-O", "-")) + require.Equal(t, "hello", out.String()) + require.EqualError(t, run(true, "issue", "attachment", "delete", "1", "9"), "confirmation required to delete attachment 9 (crash.log) on alice/project#1; rerun with --yes") + require.NoError(t, run(true, "issue", "attachment", "delete", "1", "9", "--yes")) + require.Equal(t, "✓ Deleted attachment 9 (crash.log) on alice/project#1\n", diagnostics.String()) +} diff --git a/internal/app/validation_test.go b/internal/app/validation_test.go index e8cb3505..92477c7d 100644 --- a/internal/app/validation_test.go +++ b/internal/app/validation_test.go @@ -40,7 +40,6 @@ func TestUsageErrorsPrecedeRequests(t *testing.T) { {[]string{"auth", "status", "--json=token"}, "unknown JSON field \"token\""}, {[]string{"config", "set", "prompt", "invalid"}, "prompt must be enabled or disabled"}, {[]string{"doctor", "--json=secret"}, "unknown JSON field \"secret\""}, - {[]string{"issue", "attachment", "download", "1", "1"}, "supply --output-file, or --output-file - for stdout"}, {[]string{"issue", "attachment", "download", "1", "1", "--json"}, "attachment downloads are binary"}, {[]string{"issue", "attachment", "download", "1", "1", "--output-file", "-", "--json=id"}, "attachment downloads are binary"}, {[]string{"issue", "close", "1", "bad"}, `invalid issue "bad": use a number, #number, OWNER/REPO#number or URL`}, @@ -100,7 +99,6 @@ func TestUsageErrorsPrecedeRequests(t *testing.T) { {[]string{"org", "edit", "team"}, "supply organization fields to edit"}, {[]string{"org", "list", "--all", "--username", "alice"}, "--all and --username are mutually exclusive"}, {[]string{"org", "member", "list", "team", "--limit", "0"}, `"-L, --limit" flag: must be 1..1000`}, - {[]string{"pr", "attachment", "download", "1", "1"}, "supply --output-file, or --output-file - for stdout"}, {[]string{"pr", "attachment", "download", "1", "1", "--json"}, "attachment downloads are binary"}, {[]string{"pr", "attachment", "download", "1", "1", "--output-file", "-", "--json=id"}, "attachment downloads are binary"}, {[]string{"pr", "checkout", "7", "--detach", "--branch", "name"}, "--detach and --branch/--update are mutually exclusive"}, diff --git a/internal/services/issue_attachments.go b/internal/services/issue_attachments.go index 674c1550..9b0acdf9 100644 --- a/internal/services/issue_attachments.go +++ b/internal/services/issue_attachments.go @@ -65,11 +65,13 @@ func (s Issues) UploadAttachment(ctx context.Context, number int64, name string, return IssueAttachment(asset), err } -// DownloadAttachment opens an attachment stream scoped to its instance and issue. -func (s Issues) DownloadAttachment(ctx context.Context, number, id int64) (io.ReadCloser, error) { +// DownloadAttachment opens an attachment stream scoped to its instance and issue, and returns the +// attachment's metadata. +func (s Issues) DownloadAttachment(ctx context.Context, number, id int64) (IssueAttachment, io.ReadCloser, error) { attachment, err := s.attachment(ctx, number, id) if err != nil { - return nil, err + return IssueAttachment{}, nil, err } - return downloadAttachment(ctx, s.Client, attachment) + body, err := downloadAttachment(ctx, s.Client, attachment) + return IssueAttachment(assetFrom(attachment)), body, err }