diff --git a/docs/tui.md b/docs/tui.md index 41c32b70..cec763c0 100644 --- a/docs/tui.md +++ b/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 diff --git a/internal/htmlutil/markdown.go b/internal/htmlutil/markdown.go index d1d7098f..73e0db3b 100644 --- a/internal/htmlutil/markdown.go +++ b/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) diff --git a/internal/htmlutil/markdown_safety_test.go b/internal/htmlutil/markdown_safety_test.go index c445c880..5803df84 100644 --- a/internal/htmlutil/markdown_safety_test.go +++ b/internal/htmlutil/markdown_safety_test.go @@ -448,7 +448,7 @@ func TestToMarkdownUnlinkableImageAltIsProse(t *testing.T) { func TestToMarkdownAttachmentNamesAreSerialized(t *testing.T) { got := toMarkdown(`
` + `
`) - 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{ + `https://bank.example/login`: "![https://bank.example/login](https://evil.example/login) ", + `bank.example`: "![bank.example](https://evil.example/pixel.png) ", + `lanterns.jpg`: "![lanterns.jpg](https://images.example.org/lanterns.jpg)", + `Lantern walk poster`: "![Lantern walk poster](https://images.example.org/p.jpg)", + `
`: "![bank.example](/rails/blobs/abc/bank.example)", + `
`: "![https://bank.example](https://evil.example/q3.png) ", + } { + if got := toMarkdown(input); got != want { + t.Errorf("ToMarkdown(%s) = %q, want %q", input, got, want) + } + } +} diff --git a/internal/htmlutil/markdown_test.go b/internal/htmlutil/markdown_test.go index fafab9f3..2e1bc2e6 100644 --- a/internal/htmlutil/markdown_test.go +++ b/internal/htmlutil/markdown_test.go @@ -278,7 +278,35 @@ func TestMarkdownRoundTripPreservesAuthoredContent(t *testing.T) { func TestToMarkdownTrixFileAttachment(t *testing.T) { got := toMarkdown(`
`) - 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(`
` + + `
` + + `
`) + want := "šŸŽ¬ walkthrough.mp4\n\nšŸŽµ voicemail.m4a\n\nšŸ“Ž receipts.zip" if got != want { t.Errorf("ToMarkdown = %q, want %q", got, want) } @@ -286,7 +314,7 @@ func TestToMarkdownTrixFileAttachment(t *testing.T) { func TestToMarkdownActionTextAttachment(t *testing.T) { got := toMarkdown(`

Attached:

`) - 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) } diff --git a/internal/markdown/render_test.go b/internal/markdown/render_test.go index e6c18e10..5d56748e 100644 --- a/internal/markdown/render_test.go +++ b/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(`
`), 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(`

Lantern walk

`), 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(`

https://bank.example/login

`), 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) + } +} diff --git a/internal/markdown/style.go b/internal/markdown/style.go index 7d7b0fba..700e9994 100644 --- a/internal/markdown/style.go +++ b/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{ diff --git a/internal/tui/mail_test.go b/internal/tui/mail_test.go index 0cf72ef1..5ad8a55a 100644 --- a/internal/tui/mail_test.go +++ b/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) } }