Show attachments by name and kind instead of by URL

docs/tui.md

@@ -172,7 +172,7 @@ the iOS and Android apps each keep their own local choice too, and this is the s
 
 ## Attachments
 
-Thread attachments always appear with their filename, media type, and size. Use `[` and `]` to select an attachment, `s` to save it without replacing an existing file, and `o` to download and open it in an external application. Attachments never open automatically. Kitty and Ghostty can show inline images. Foot and other terminals use visible text markers.
+Thread attachments always appear with their filename, media type, and size. Use `[` and `]` to select an attachment, `s` to save it without replacing an existing file, and `o` to download and open it in an external application. Attachments never open automatically. Kitty and Ghostty can show inline images. In the message itself, a file is marked by its kind: 📷 an image, 🎬 a video, 🎵 audio, 📄 a PDF or document, 📎 anything else. An image shows by name rather than by its URL, unless its name looks like a URL, when the real destination is shown beside it; an image on the web is a link you can select with Tab, and the row above the shortcut bar shows its whole destination.
 
 ## Contacts
 

internal/htmlutil/markdown.go

@@ -673,7 +673,7 @@ func (m *markdownizer) image(n *html.Node) {
 	src, linkable := destination(getAttr(n, "src"))
 	switch {
 	case linkable:
-		m.write("![" + escapeText(alt, "alt") + "](" + src + ")")
+		m.write(imageMarkdown(alt, escapeText(alt, "alt"), src))
 	case alt != "":
 		m.write(escapeText(alt, m.line.String()))
 	}
@@ -724,20 +724,57 @@ func (m *markdownizer) actionTextAttachment(n *html.Node) {
 }
 
 func (m *markdownizer) attachment(filename, url, contentType string) {
-	filename = escapeText(strings.Join(strings.Fields(filename), " "), "📎 ")
+	icon := attachmentIcon(contentType)
+	name := strings.Join(strings.Fields(filename), " ")
+	filename = escapeText(name, icon)
 	if filename == "" {
 		filename = "attachment"
 	}
 	dest, linkable := destination(url)
 	m.block(func() {
 		if isImageContentType(contentType) && linkable {
-			m.write("![" + filename + "](" + dest + ")")
+			m.write(imageMarkdown(name, filename, dest))
 		} else {
-			m.write("📎 " + filename)
+			m.write(icon + filename)
 		}
 	})
 }
 
+// imageMarkdown writes an image. A terminal shows an image by its label alone, so a
+// label a reader could take for a URL gets the destination it links to written out
+// beside it, as a link's label does: "https://bank.example/login" pointed at
+// https://evil.example shows both. A label that is just the file the destination
+// names is not mistaken for anywhere else, and a relative destination is never a link.
+func imageMarkdown(label, escaped, dest string) string {
+	md := "![" + escaped + "](" + dest + ")"
+	if absolute(dest) && !strings.ContainsFunc(label, unicode.IsSpace) &&
+		strings.ContainsAny(label, ".:/@") && label != destinationFilename(dest) {
+		md += " <" + dest + ">"
+	}
+	return md
+}
+
+// attachmentIcon says what kind of file an attachment is. Each of these emoji is
+// drawn as an emoji without a variation selector, so every terminal gives it the
+// same two cells; 🖼️ and 🎞️ need U+FE0F for that, and terminals disagree on how
+// wide it makes them.
+func attachmentIcon(contentType string) string {
+	contentType = strings.ToLower(strings.TrimSpace(contentType))
+	switch {
+	case isImageContentType(contentType):
+		return "📷 "
+	case strings.HasPrefix(contentType, "video/"):
+		return "🎬 "
+	case strings.HasPrefix(contentType, "audio/"):
+		return "🎵 "
+	case contentType == "application/pdf", strings.HasPrefix(contentType, "text/"),
+		strings.Contains(contentType, "msword"), strings.Contains(contentType, "document"):
+		return "📄 "
+	default:
+		return "📎 "
+	}
+}
+
 func (m *markdownizer) children(n *html.Node) {
 	for child := n.FirstChild; child != nil; child = child.NextSibling {
 		m.walk(child)

internal/htmlutil/markdown_safety_test.go

@@ -448,7 +448,7 @@ func TestToMarkdownUnlinkableImageAltIsProse(t *testing.T) {
 func TestToMarkdownAttachmentNamesAreSerialized(t *testing.T) {
 	got := toMarkdown(`<figure data-trix-attachment='{"url":"javascript:x","filename":"*report*.png","contentType":"image/png"}'></figure>` +
 		`<figure data-trix-attachment='{"url":"/rails/blobs/q3.pdf","filename":"[q3].pdf","contentType":"application/pdf"}'></figure>`)
-	want := "📎 \\*report\\*.png\n\n📎 \\[q3\\].pdf"
+	want := "📷 \\*report\\*.png\n\n📄 \\[q3\\].pdf"
 	if got != want {
 		t.Errorf("ToMarkdown = %q, want %q", got, want)
 	}
@@ -630,3 +630,18 @@ func FuzzToMarkdownTerminalSafety(f *testing.F) {
 		assertSafeTree(t, md)
 	})
 }
+
+func TestToMarkdownImageLabelThatLooksLikeAURLShowsItsDestination(t *testing.T) {
+	for input, want := range map[string]string{
+		`<img alt="https://bank.example/login" src="https://evil.example/login">`:                                                                    "![https://bank.example/login](https://evil.example/login) <https://evil.example/login>",
+		`<img alt="bank.example" src="https://evil.example/pixel.png">`:                                                                              "![bank.example](https://evil.example/pixel.png) <https://evil.example/pixel.png>",
+		`<img alt="lanterns.jpg" src="https://images.example.org/lanterns.jpg">`:                                                                     "![lanterns.jpg](https://images.example.org/lanterns.jpg)",
+		`<img alt="Lantern walk poster" src="https://images.example.org/p.jpg">`:                                                                     "![Lantern walk poster](https://images.example.org/p.jpg)",
+		`<figure data-trix-attachment='{"url":"/rails/blobs/abc/bank.example","filename":"bank.example","contentType":"image/png"}'></figure>`:       "![bank.example](/rails/blobs/abc/bank.example)",
+		`<figure data-trix-attachment='{"url":"https://evil.example/q3.png","filename":"https://bank.example","contentType":"image/png"}'></figure>`: "![https://bank.example](https://evil.example/q3.png) <https://evil.example/q3.png>",
+	} {
+		if got := toMarkdown(input); got != want {
+			t.Errorf("ToMarkdown(%s) = %q, want %q", input, got, want)
+		}
+	}
+}

internal/htmlutil/markdown_test.go

@@ -278,15 +278,43 @@ func TestMarkdownRoundTripPreservesAuthoredContent(t *testing.T) {
 
 func TestToMarkdownTrixFileAttachment(t *testing.T) {
 	got := toMarkdown(`<figure data-trix-attachment='{"url":"/rails/blobs/q3.pdf","filename":"q3-report.pdf","contentType":"application/pdf"}'></figure>`)
-	want := "📎 q3-report.pdf"
+	want := "📄 q3-report.pdf"
+	if got != want {
+		t.Errorf("ToMarkdown = %q, want %q", got, want)
+	}
+}
+
+func TestAttachmentIconSaysWhatKindOfFileItIs(t *testing.T) {
+	for contentType, want := range map[string]string{
+		"image/jpeg":         "📷 ",
+		"video/mp4":          "🎬 ",
+		"audio/mpeg":         "🎵 ",
+		"application/pdf":    "📄 ",
+		"text/plain":         "📄 ",
+		"application/msword": "📄 ",
+		"application/vnd.openxmlformats-officedocument.wordprocessingml.document": "📄 ",
+		"application/zip": "📎 ",
+		"":                "📎 ",
+	} {
+		if got := attachmentIcon(contentType); got != want {
+			t.Errorf("attachmentIcon(%q) = %q, want %q", contentType, got, want)
+		}
+	}
+}
+
+func TestToMarkdownAttachmentsCarryTheirKind(t *testing.T) {
+	got := toMarkdown(`<figure data-trix-attachment='{"url":"/rails/blobs/walkthrough.mp4","filename":"walkthrough.mp4","contentType":"video/mp4"}'></figure>` +
+		`<figure data-trix-attachment='{"url":"/rails/blobs/voicemail.m4a","filename":"voicemail.m4a","contentType":"audio/mp4"}'></figure>` +
+		`<figure data-trix-attachment='{"url":"/rails/blobs/receipts.zip","filename":"receipts.zip","contentType":"application/zip"}'></figure>`)
+	want := "🎬 walkthrough.mp4\n\n🎵 voicemail.m4a\n\n📎 receipts.zip"
 	if got != want {
 		t.Errorf("ToMarkdown = %q, want %q", got, want)
 	}
 }
 
 func TestToMarkdownActionTextAttachment(t *testing.T) {
 	got := toMarkdown(`<p>Attached:</p><action-text-attachment url="/rails/blobs/q3.pdf" filename="q3-report.pdf" content-type="application/pdf"></action-text-attachment>`)
-	want := "Attached:\n\n📎 q3-report.pdf"
+	want := "Attached:\n\n📄 q3-report.pdf"
 	if got != want {
 		t.Errorf("ToMarkdown = %q, want %q", got, want)
 	}

internal/markdown/render_test.go

@@ -4,6 +4,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/charmbracelet/x/ansi"
+
 	"github.com/basecamp/hey-cli/internal/htmlutil"
 )
 
@@ -230,3 +232,25 @@ func TestRenderLinkedRejectsMalformedDestinationAndHandlesNoLinks(t *testing.T)
 		}
 	}
 }
+
+func TestRenderShowsAnImageByNameAndLinksTheName(t *testing.T) {
+	attached := Render(htmlutil.ToMarkdown(`<figure data-trix-attachment='{"url":"/rails/active_storage/blobs/redirect/eyJfcmFpbHMiOnsi/1_an_Gustav.jpg","filename":"1_an_Gustav.jpg","contentType":"image/jpeg"}'></figure>`), 80)
+	if text := ansi.Strip(attached); text != "📷 1_an_Gustav.jpg" {
+		t.Errorf("attached image = %q, want its name behind a camera and no URL", text)
+	}
+
+	linked := RenderLinked(htmlutil.ToMarkdown(`<p><img src="https://images.example.com/lanterns.jpg" alt="Lantern walk" width="600" height="400"></p>`), 80, -1)
+	if text := ansi.Strip(linked.Text); text != "📷 Lantern walk" {
+		t.Errorf("web image = %q, want its name behind a camera and no URL", text)
+	}
+	if len(linked.Links) != 1 || linked.Links[0].Destination != "https://images.example.com/lanterns.jpg" {
+		t.Errorf("web image links = %#v, want its name selectable with the whole destination", linked.Links)
+	}
+}
+
+func TestRenderNeverHidesWhereAURLShapedImageLabelGoes(t *testing.T) {
+	out := ansi.Strip(Render(htmlutil.ToMarkdown(`<p><img alt="https://bank.example/login" src="https://evil.example/login"></p>`), 80))
+	if !strings.Contains(out, "📷 https://bank.example/login") || !strings.Contains(out, "https://evil.example/login") {
+		t.Errorf("rendered = %q, want the label and the destination it really links to", out)
+	}
+}

internal/markdown/style.go

@@ -67,13 +67,16 @@ var terminalStyle = ansi.StyleConfig{
 	LinkText: ansi.StylePrimitive{
 		Color: stringPointer("12"),
 	},
+	// An image is its name behind a camera, not its URL: the name carries the link, and
+	// the TUI's status row shows the whole destination when the name is selected.
+	// Glamour writes the URL as a separate element, so its Format drops it.
 	Image: ansi.StylePrimitive{
-		Color:     stringPointer("14"),
-		Underline: boolPointer(true),
+		Format: "{{/* shown in the status row */}}",
 	},
 	ImageText: ansi.StylePrimitive{
-		Faint:  boolPointer(true),
-		Format: "Image: {{.text}} →",
+		BlockPrefix: "📷 ",
+		Color:       stringPointer("14"),
+		Underline:   boolPointer(true),
 	},
 	Code: ansi.StyleBlock{
 		StylePrimitive: ansi.StylePrimitive{

internal/tui/mail_test.go

@@ -3111,8 +3111,8 @@ func TestMailViewAlwaysRendersAttachmentPanelAndTextMarker(t *testing.T) {
 	})
 
 	view := v.View()
-	for _, want := range []string{"Image: ", "Attachments", "chart.png", "image/png", "1.5 KB"} {
-		if !strings.Contains(view, want) {
+	for _, want := range []string{"📷 chart.png", "Attachments", "chart.png", "image/png", "1.5 KB"} {
+		if !strings.Contains(stripANSI(view), want) {
 			t.Errorf("thread view does not contain %q: %q", want, view)
 		}
 	}