refactor(gmail): dedupe attachment output/path logic

Author: steipete

internal/cmd/gmail_attachment.go

@@ -48,7 +48,7 @@ func resolveAttachmentOutPath(outPathFlag string, name string) (string, error) {
 	// Directory intent:
 	// - existing directory path
 	// - or explicit trailing slash for a (possibly non-existent) directory
-	isDir := strings.HasSuffix(outPathFlag, "/") || strings.HasSuffix(outPathFlag, "\\")
+	isDir := strings.HasSuffix(outPathFlag, string(os.PathSeparator)) || strings.HasSuffix(outPathFlag, "/") || strings.HasSuffix(outPathFlag, "\\")
 	if !isDir {
 		if info, statErr := os.Stat(outPath); statErr == nil && info.IsDir() {
 			isDir = true
@@ -61,6 +61,16 @@ func resolveAttachmentOutPath(outPathFlag string, name string) (string, error) {
 	return outPath, nil
 }
 
+func printAttachmentDownloadResult(ctx context.Context, u *ui.UI, path string, cached bool, bytes int64) error {
+	if outfmt.IsJSON(ctx) {
+		return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"path": path, "cached": cached, "bytes": bytes})
+	}
+	u.Out().Printf("path\t%s", path)
+	u.Out().Printf("cached\t%t", cached)
+	u.Out().Printf("bytes\t%d", bytes)
+	return nil
+}
+
 func (c *GmailAttachmentCmd) Run(ctx context.Context, flags *RootFlags) error {
 	u := ui.FromContext(ctx)
 	messageID := normalizeGmailMessageID(c.MessageID)
@@ -124,13 +134,7 @@ func (c *GmailAttachmentCmd) Run(ctx context.Context, flags *RootFlags) error {
 		if dlErr != nil {
 			return dlErr
 		}
-		if outfmt.IsJSON(ctx) {
-			return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"path": path, "cached": cached, "bytes": bytes})
-		}
-		u.Out().Printf("path\t%s", path)
-		u.Out().Printf("cached\t%t", cached)
-		u.Out().Printf("bytes\t%d", bytes)
-		return nil
+		return printAttachmentDownloadResult(ctx, u, path, cached, bytes)
 	}
 
 	outPath, err := resolveAttachmentOutPath(outPathFlag, c.Name)
@@ -141,13 +145,7 @@ func (c *GmailAttachmentCmd) Run(ctx context.Context, flags *RootFlags) error {
 	if err != nil {
 		return err
 	}
-	if outfmt.IsJSON(ctx) {
-		return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"path": path, "cached": cached, "bytes": bytes})
-	}
-	u.Out().Printf("path\t%s", path)
-	u.Out().Printf("cached\t%t", cached)
-	u.Out().Printf("bytes\t%d", bytes)
-	return nil
+	return printAttachmentDownloadResult(ctx, u, path, cached, bytes)
 }
 
 func downloadAttachmentToPath(

internal/cmd/gmail_attachment_more_test.go

@@ -150,12 +150,13 @@ func TestDownloadAttachmentToPath_DirectoryNotCachedBySize(t *testing.T) {
 	}
 }
 
-func TestGmailAttachmentCmd_DryRun_OutDir_UsesName(t *testing.T) {
-	outDir := t.TempDir()
+func mustDryRunAttachmentPath(t *testing.T, args ...string) string {
+	t.Helper()
+
 	ctx := outfmt.WithMode(context.Background(), outfmt.Mode{JSON: true})
 
 	out := captureStdout(t, func() {
-		err := runKong(t, &GmailAttachmentCmd{}, []string{"m1", "a1", "--out", outDir, "--name", "invoice.pdf"}, ctx, &RootFlags{DryRun: true})
+		err := runKong(t, &GmailAttachmentCmd{}, args, ctx, &RootFlags{DryRun: true})
 		var exitErr *ExitError
 		if !errors.As(err, &exitErr) || exitErr.Code != 0 {
 			t.Fatalf("expected exit code 0, got: %v", err)
@@ -174,40 +175,25 @@ func TestGmailAttachmentCmd_DryRun_OutDir_UsesName(t *testing.T) {
 	if !ok {
 		t.Fatalf("expected request.path string, got=%T", req["path"])
 	}
+	return path
+}
+
+func TestGmailAttachmentCmd_DryRun_OutDir_UsesName(t *testing.T) {
+	outDir := t.TempDir()
+	got := mustDryRunAttachmentPath(t, "m1", "a1", "--out", outDir, "--name", "invoice.pdf")
 	want := filepath.Join(outDir, "invoice.pdf")
-	if path != want {
-		t.Fatalf("unexpected path: got=%q want=%q", path, want)
+	if got != want {
+		t.Fatalf("unexpected path: got=%q want=%q", got, want)
 	}
 }
 
 func TestGmailAttachmentCmd_DryRun_OutDirTrailingSlash_UsesNameEvenIfMissing(t *testing.T) {
 	base := t.TempDir()
 	outDir := filepath.Join(base, "newdir") + string(os.PathSeparator)
-	ctx := outfmt.WithMode(context.Background(), outfmt.Mode{JSON: true})
-
-	out := captureStdout(t, func() {
-		err := runKong(t, &GmailAttachmentCmd{}, []string{"m1", "a1", "--out", outDir, "--name", "invoice.pdf"}, ctx, &RootFlags{DryRun: true})
-		var exitErr *ExitError
-		if !errors.As(err, &exitErr) || exitErr.Code != 0 {
-			t.Fatalf("expected exit code 0, got: %v", err)
-		}
-	})
-
-	var got map[string]any
-	if err := json.Unmarshal([]byte(out), &got); err != nil {
-		t.Fatalf("unmarshal: %v\noutput=%q", err, out)
-	}
-	req, ok := got["request"].(map[string]any)
-	if !ok {
-		t.Fatalf("expected request object, got=%T", got["request"])
-	}
-	path, ok := req["path"].(string)
-	if !ok {
-		t.Fatalf("expected request.path string, got=%T", req["path"])
-	}
+	got := mustDryRunAttachmentPath(t, "m1", "a1", "--out", outDir, "--name", "invoice.pdf")
 	want := filepath.Join(filepath.Join(base, "newdir"), "invoice.pdf")
-	if path != want {
-		t.Fatalf("unexpected path: got=%q want=%q", path, want)
+	if got != want {
+		t.Fatalf("unexpected path: got=%q want=%q", got, want)
 	}
 }