From fc532bfc1d897f5810d8e2b57ca0446a10551ff0 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 16:02:26 -0400 Subject: [PATCH 01/12] Suggest recipients as you type, and ask before losing a message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing in To, Cc or Bcc — in a new message, a reply or a forward — now opens a short list of people under the field, from the same list HEY's web composer suggests: recently addressed people first, then contacts, then "Everyone at" an account and contact groups. It matches names and addresses, leaves out anyone already on the line, and Tab or Enter writes the pick as Name
in place of what was being typed. With the list closed, Tab and Enter move between fields as before. The list is read in the background when a composer opens (a 304 through the SDK's cache after the first time) and filtered in memory on each keystroke, so typing never waits on the network. Esc on a message that holds more than it opened with now asks first: save it as a HEY draft, discard it, or keep editing. An untouched form still closes straight away. Needs the SDK's new Contacts().Addressable, pinned here to its commit until the SDK is released. --- AGENTS.md | 18 ++ docs/tui.md | 22 +- internal/mail/address.go | 61 +++- internal/mail/address_at_test.go | 64 ++++ internal/tui/compose.go | 190 ++++++++++- internal/tui/compose_leave_test.go | 187 +++++++++++ internal/tui/compose_test.go | 8 +- internal/tui/mail.go | 31 +- internal/tui/recipients.go | 326 +++++++++++++++++++ internal/tui/recipients_test.go | 497 +++++++++++++++++++++++++++++ internal/tui/tui.go | 4 +- 11 files changed, 1380 insertions(+), 28 deletions(-) create mode 100644 internal/mail/address_at_test.go create mode 100644 internal/tui/compose_leave_test.go create mode 100644 internal/tui/recipients.go create mode 100644 internal/tui/recipients_test.go diff --git a/AGENTS.md b/AGENTS.md index 38d79fb08..e94a29804 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -341,6 +341,24 @@ router), `section_view.go` (the interface), plus `nav.go`, `content.go`, `help.g To add a new section: implement the `sectionView` interface in a new file, add a field and constructor call in `newModel`, and add a case in `switchSection`. +### Recipients are suggested from HEY's composer list + +The To, Cc and Bcc fields of every compose form suggest people from +`Contacts().Addressable` — `/autocompletable/contacts/addressable`, the list HEY's web +composer uses, asked for `include_self` as the web asks. It is not `Contacts().List`: that +is a paged search index with no groups, no "Everyone at" and no recently-addressed order. +HEY answers the whole list with no query, so `mailView.loadRecipients` reads it in the +background whenever a composer opens (the SDK's ETag cache makes a repeat a 304) and +`matchRecipients` filters it in memory on every keystroke, against lower-cased copies made +once when the list arrives. Keep it that way: a keystroke must never wait on the network. +The finished list is routed to Mail from any section (`tui.go`), because it can arrive +after the composer has sent. A group is matched by its name only, and picking it writes its +addresses, since HEY takes addresses rather than a group's name. + +Esc on a composer holding more than it opened with asks before it closes, and `s` keeps +the message as a HEY draft — `Messages().CreateDraft`, or `Entries().CreateReplyDraft` for a +reply — which is the TUI's version of the web composer never losing a message. + ### Each calendar span gives the arrows what it is made of The three spans are not one screen with a different date on it, so the arrows mean something diff --git a/docs/tui.md b/docs/tui.md index eec134911..0c0b32ab4 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -93,7 +93,8 @@ While any threads are selected, the help bar counts them and offers only what ac all of them: `e`, `u`, `t` and Ctrl+B. The other thread actions — moving, filing, labels, collections, spam, ignoring, reply and forward — work on one thread at a time, so they are refused with a notice rather than quietly acting on the thread under the cursor; -press Escape to clear the selection first. Escape still closes an open form, cancels a +press Escape to clear the selection first. Escape still closes an open form (asking first +when it holds a message you have written), cancels a thread that is loading and leaves an open thread before it clears a selection, and on Previously Seen the first Escape clears the selection and the next one leaves. A bulk action lets go of the threads it acted on once HEY has answered, and keeps them selected if @@ -125,6 +126,25 @@ picker. HEY never chooses a default: Enter inserts the selected snippet at the b cursor, Escape returns without changing the draft, and the picker can be reopened to insert another. +Typing in To, Cc or Bcc — in a new message, a reply or a forward — opens a short list of +people under the field, from the same list HEY's web composer suggests: the people you +wrote to most recently first, then your contacts by name, then "Everyone at" your +company and your contact groups. It narrows as you type, matching a name or an address, +and leaves out anyone already on that line. ↑ and ↓ (or Ctrl+P and Ctrl+N) move through +it; Tab or Enter adds the highlighted person as `Name
, ` in place of what you +were typing, ready for the next one; a group adds every address in it. Escape closes the +list and nothing else, and moving the cursor closes it too. With the list closed, Tab and +Enter move between fields as they always have. The list is read in the background when +the form opens, so typing never waits on it, and an address typed in full works whether +it arrives or not. + +Escape on a message you have written something in asks before it goes: `s` (or Enter) +keeps it in HEY's drafts, where the web app and your phone can pick it up, `d` discards +it, and Escape takes you back to it. A form holding only what it opened with — an empty +new message, or a reply with just HEY's prefilled recipients — closes without asking. A +draft with an address HEY would not deliver to is refused the way a send is, so the +draft never comes back without that recipient. + ## Live updates The mail list follows the server. HEY tells the TUI when a box changed over the same diff --git a/internal/mail/address.go b/internal/mail/address.go index 27226a5e1..f7dfadb80 100644 --- a/internal/mail/address.go +++ b/internal/mail/address.go @@ -24,16 +24,43 @@ var encodedWord = regexp.MustCompile(`=\?[^?]*\?[bBqQ]\?[^?]*\?=`) // is one recipient. func SplitAddresses(s string) []string { var addresses []string - var b strings.Builder - quoted, depth, angled := false, 0, false - flush := func() { - if address := strings.TrimSpace(b.String()); address != "" { + start := 0 + for _, comma := range separators(s) { + if address := strings.TrimSpace(s[start:comma]); address != "" { addresses = append(addresses, address) } - b.Reset() + start = comma + 1 + } + if address := strings.TrimSpace(s[start:]); address != "" { + addresses = append(addresses, address) + } + return addresses +} + +// AddressAt returns the byte range of the recipient a cursor at byte offset pos is +// in, commas excluded, split the way SplitAddresses splits: the part of a list that +// someone typing at pos is writing. +func AddressAt(s string, pos int) (start, end int) { + pos = min(max(pos, 0), len(s)) + end = len(s) + for _, comma := range separators(s) { + if comma < pos { + start = comma + 1 + continue + } + end = comma + break } + return start, end +} + +// separators returns the byte offsets of the commas that separate recipients, leaving +// out the ones inside a quoted name, a comment or angle brackets. +func separators(s string) []int { + var commas []int + quoted, depth, angled := false, 0, false escaped := false - for _, r := range s { + for i, r := range s { switch { case escaped: escaped = false @@ -52,13 +79,25 @@ func SplitAddresses(s string) []string { case r == '>': angled = false case r == ',' && !angled: - flush() - continue + commas = append(commas, i) } - b.WriteRune(r) } - flush() - return addresses + return commas +} + +// FormatAddress writes a recipient the way a person would type it: the bare address +// when there is no name to add, and Name
otherwise, quoting a name that +// holds a character that would split or end it, as the mail gem HEY parses with does. +func FormatAddress(name, address string) string { + name = strings.TrimSpace(name) + address = strings.TrimSpace(address) + if name == "" || strings.EqualFold(name, address) { + return address + } + if strings.ContainsAny(name, `()<>[]:;@\,."`) { + name = `"` + strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(name) + `"` + } + return name + " <" + address + ">" } // InvalidAddress returns the first recipient HEY would certainly not deliver to, or "" diff --git a/internal/mail/address_at_test.go b/internal/mail/address_at_test.go new file mode 100644 index 000000000..9a2668e2a --- /dev/null +++ b/internal/mail/address_at_test.go @@ -0,0 +1,64 @@ +package mail + +import ( + "strings" + "testing" +) + +func TestAddressAtFindsTheRecipientUnderTheCursor(t *testing.T) { + list := `Jane Doe , "Bryan, Annie" , ric` + for _, tc := range []struct { + pos int + want string + }{ + {0, "Jane Doe "}, + {10, "Jane Doe "}, + // On the comma itself the cursor still belongs to the recipient before it. + {strings.Index(list, ">,") + 1, "Jane Doe "}, + {strings.Index(list, "Annie"), ` "Bryan, Annie" `}, + {len(list), " ric"}, + {len(list) + 10, " ric"}, + {-3, "Jane Doe "}, + } { + start, end := AddressAt(list, tc.pos) + if got := list[start:end]; got != tc.want { + t.Errorf("AddressAt(%d) = %q, want %q", tc.pos, got, tc.want) + } + } + + if start, end := AddressAt("", 0); start != 0 || end != 0 { + t.Errorf("AddressAt on an empty list = %d, %d", start, end) + } + if start, end := AddressAt("jane@example.com, ", 18); start != 17 || end != 18 { + t.Errorf("AddressAt after a trailing comma = %d, %d, want the empty recipient at the end", start, end) + } +} + +func TestFormatAddressWritesWhatHEYParsesBack(t *testing.T) { + for _, tc := range []struct { + name, address, want string + }{ + {"", "jane@example.com", "jane@example.com"}, + {"jane@example.com", "jane@example.com", "jane@example.com"}, + {"JANE@example.com", "jane@example.com", "jane@example.com"}, + {"Jane Doe", "jane@example.com", "Jane Doe "}, + {"Bryan, Annie", "annie@example.com", `"Bryan, Annie" `}, + {`Rick "The Man" Sanchez`, "rick@example.org", `"Rick \"The Man\" Sanchez" `}, + {"J. Smith", "j.smith@example.org", `"J. Smith" `}, + {"Zoë Martín", "zoe@example.com", "Zoë Martín "}, + } { + got := FormatAddress(tc.name, tc.address) + if got != tc.want { + t.Errorf("FormatAddress(%q, %q) = %q, want %q", tc.name, tc.address, got, tc.want) + continue + } + // Whatever is written has to survive the trip back through the list parsing + // and HEY's delivery rule as one recipient. + if split := SplitAddresses(got + ", someone@example.com"); len(split) != 2 || split[0] != got { + t.Errorf("FormatAddress(%q, %q) split into %q", tc.name, tc.address, split) + } + if bad := InvalidAddress([]string{got}); bad != "" { + t.Errorf("FormatAddress(%q, %q) = %q, which HEY would drop", tc.name, tc.address, got) + } + } +} diff --git a/internal/tui/compose.go b/internal/tui/compose.go index 3e716e056..05bf5d254 100644 --- a/internal/tui/compose.go +++ b/internal/tui/compose.go @@ -57,6 +57,12 @@ type composeSentMsg struct { err error } +// draftSavedMsg reports the outcome of keeping a message as a draft on the way out. +type draftSavedMsg struct { + form *composeForm + err error +} + // --- Compose form --- type composeMode int @@ -78,6 +84,10 @@ const ( fieldBody ) +// composeLabelWidth is the column a field's text starts at: a label right-aligned +// in eight cells and its ": ". +const composeLabelWidth = 10 + // composeForm is the in-TUI editor for a new message, reply or forward. It owns // its inputs, validation and status; sending is done by mailView so the form // stays free of SDK calls. @@ -104,6 +114,20 @@ type composeForm struct { snippetsLoaded bool snippetRequestID uint64 + // recipients is HEY's list of who can be written to, shared with the view + // and never written through. suggest is the list open under a recipient + // field, and typing says the last key edited that field, which is what + // opens it. + recipients []recipientSuggestion + suggest *recipientPopover + typing bool + + // pristine is every field as the form opened, prefill included, so that + // esc knows whether leaving would lose anything. confirmLeave is the + // question asked when it would. + pristine []string + confirmLeave bool + styles styles width int height int @@ -169,10 +193,36 @@ func newForwardForm(ctxMsg forwardContextLoadedMsg, s styles) *composeForm { func (f *composeForm) bodyIndex() int { return len(f.inputs) } func (f *composeForm) init() tea.Cmd { + f.pristine = f.fieldValues() return f.focusCurrent() } +// fieldValues answers what every field holds, in order, body last. +func (f *composeForm) fieldValues() []string { + values := make([]string, 0, len(f.inputs)+1) + for i := range f.inputs { + values = append(values, f.inputs[i].Value()) + } + return append(values, f.body.Value()) +} + +// edited reports whether the form holds anything it did not open with, which +// is what leaving it would lose. +func (f *composeForm) edited() bool { + if f.pristine == nil { + return false + } + for i, value := range f.fieldValues() { + if strings.TrimSpace(value) != strings.TrimSpace(f.pristine[i]) { + return true + } + } + return false +} + func (f *composeForm) focusCurrent() tea.Cmd { + f.suggest = nil + f.typing = false for i := range f.inputs { f.inputs[i].Blur() } @@ -191,7 +241,7 @@ func (f *composeForm) resize(width, height int) { } inner := max(width-4, 10) for i := range f.inputs { - f.inputs[i].SetWidth(inner - 9) // room for the "Subject: " label + f.inputs[i].SetWidth(inner - composeLabelWidth + 1) } f.body.SetWidth(inner) // title + fields + blank + status + blank @@ -246,6 +296,9 @@ func (f *composeForm) handleKey(view *mailView, msg tea.KeyPressMsg) (tea.Cmd, b if f.sending { return nil, true } + if f.confirmLeave { + return f.handleLeaveKey(view, msg) + } if f.snippetPicker != nil { picker := f.snippetPicker cmd, open, snippet := picker.handleKey(msg) @@ -262,11 +315,20 @@ func (f *composeForm) handleKey(view *mailView, msg tea.KeyPressMsg) (tea.Cmd, b } return cmd, true } + if f.handleSuggestionKey(msg) { + return nil, true + } switch { case msg.String() == "ctrl+t": return f.openSnippetPicker(view), true case msg.Key().Code == tea.KeyEscape: - return nil, false + // Nothing typed, nothing to lose. Otherwise ask, the way HEY never lets a + // message go without a draft of it. + if !f.edited() { + return nil, false + } + f.confirmLeave = true + return nil, true case msg.Key().Code == tea.KeyTab && msg.Key().Mod == tea.ModShift: f.focus = (f.focus + f.bodyIndex()) % (f.bodyIndex() + 1) return f.focusCurrent(), true @@ -280,7 +342,59 @@ func (f *composeForm) handleKey(view *mailView, msg tea.KeyPressMsg) (tea.Cmd, b case msg.String() == "ctrl+s": return f.submit(view), true } - return f.update(msg), true + return f.edit(msg), true +} + +// edit hands a key to the focused field. In a recipient field, a key that changes +// what is typed narrows the list under it, and one that only moves the cursor +// closes it: the list is about the recipient being written, not the one the cursor +// happens to pass through. +func (f *composeForm) edit(msg tea.KeyPressMsg) tea.Cmd { + if !isRecipientField(f.focus) { + return f.update(msg) + } + input := &f.inputs[f.focus] + value, position := input.Value(), input.Position() + cmd := f.update(msg) + switch { + case input.Value() != value: + f.typing = true + f.refreshSuggestions() + case input.Position() != position: + f.suggest = nil + f.typing = false + } + return cmd +} + +// handleLeaveKey answers the question esc asks of an edited message: keep it as a +// draft, throw it away, or go back to it. +func (f *composeForm) handleLeaveKey(view *mailView, msg tea.KeyPressMsg) (tea.Cmd, bool) { + switch msg.String() { + case "s", "S", "enter": + f.confirmLeave = false + return f.saveDraft(view), true + case "d", "D": + return nil, false + case "esc": + f.confirmLeave = false + } + return nil, true +} + +// saveDraft keeps the message in HEY's drafts, where the web app and the phone +// can pick it up. A draft needs nobody on it yet, but an address HEY would drop +// is refused here for the same reason a send refuses it: HEY drops it without +// saying, and the draft would come back without that recipient. +func (f *composeForm) saveDraft(view *mailView) tea.Cmd { + to, cc, bcc, _, _ := f.values() + if address := mail.InvalidAddress(to, cc, bcc); address != "" { + f.setStatus("Not a valid email address: "+terminal.SanitizeLine(address), true) + return nil + } + f.sending = true + f.setStatus("Saving draft…", false) + return view.saveDraft(f) } func (f *composeForm) submit(view *mailView) tea.Cmd { @@ -324,9 +438,24 @@ func (f *composeForm) update(msg tea.Msg) tea.Cmd { } func (f *composeForm) helpBindings() []helpBinding { + if f.confirmLeave { + return []helpBinding{ + {"s", "save draft"}, + {"d", "discard"}, + {"esc", "keep editing"}, + } + } if f.snippetPicker != nil { return f.snippetPicker.helpBindings() } + if f.suggest != nil { + return []helpBinding{ + {"↑/↓", "choose"}, + {"tab", "add"}, + {"esc", "close list"}, + {"ctrl+s", "send"}, + } + } return []helpBinding{ {"tab", "next field"}, {"ctrl+t", "snippets"}, @@ -343,7 +472,26 @@ func (f *composeForm) draw(view *mailView) string { if f.snippetPicker != nil { return f.snippetPicker.view(view.vc.styles, view.vc.width) } - return f.view() + form := f.view() + switch { + case f.confirmLeave: + return overlayModal(form, f.leaveView(), f.width, max(lipgloss.Height(form), f.height)) + case f.suggest != nil: + // The list hangs from the line under its field, just clear of the labels + // so that the fields it covers still say what they are. + list := f.suggestionsView() + x, y := composeLabelWidth-1, 2+int(f.suggest.field) + return overlayAt(form, list, x, y, max(lipgloss.Width(form), f.width), max(lipgloss.Height(form), y+lipgloss.Height(list))) + } + return form +} + +// leaveView is the question esc asks of a message with something in it. +func (f *composeForm) leaveView() string { + key := lipgloss.NewStyle().Foreground(colorActive).Bold(true) + body := "Keep it as a draft to finish later, here or in HEY?\n\n" + + key.Render("s") + " save draft " + key.Render("d") + " discard " + key.Render("esc") + " keep editing" + return modalFrame("Close this message?", body, f.width) } func (f *composeForm) view() string { @@ -396,9 +544,15 @@ func parseAddressList(s string) []string { // startCompose opens an empty new-message form. func (v *mailView) startCompose() tea.Cmd { - form := newComposeForm(composeNew, v.vc.styles) + return v.openComposeForm(newComposeForm(composeNew, v.vc.styles)) +} + +// openComposeForm puts a new message, a reply or a forward on screen with the +// recipient list as it stands, and reads the list again behind it. +func (v *mailView) openComposeForm(form *composeForm) tea.Cmd { + form.recipients = v.recipients v.openModal(form) - return form.init() + return tea.Batch(form.init(), v.loadRecipients()) } // loadReplyContext fetches the thread's account, the entry a reply answers — its latest @@ -618,6 +772,30 @@ func (v *mailView) loadSnippets(form *composeForm, requestID uint64) tea.Cmd { } } +// saveDraft keeps the open form as a draft through the SDK: a reply as a reply +// to its entry, a new message or a forward as a message of its own, which is +// how a forward is sent too. +func (v *mailView) saveDraft(f *composeForm) tea.Cmd { + to, cc, bcc, subject, body := f.values() + ctx := v.vc.ctx + sdk := v.vc.sdk + if f.sendSDK != nil { + sdk = f.sendSDK + } + if f.mode == composeReply { + entryID := f.entryID + actingSenderID := f.replyActingSenderID + return func() tea.Msg { + _, err := sdk.Entries().CreateReplyDraft(ctx, entryID, actingSenderID, subject, body, to, cc, bcc) + return draftSavedMsg{form: f, err: err} + } + } + return func() tea.Msg { + _, err := sdk.Messages().CreateDraft(ctx, hey.DraftContent{Subject: subject, Content: body, To: to, CC: cc, BCC: bcc}) + return draftSavedMsg{form: f, err: err} + } +} + // send submits the open form through the SDK. func (v *mailView) send(f *composeForm) tea.Cmd { to, cc, bcc, subject, body := f.values() diff --git a/internal/tui/compose_leave_test.go b/internal/tui/compose_leave_test.go new file mode 100644 index 000000000..96d0cc012 --- /dev/null +++ b/internal/tui/compose_leave_test.go @@ -0,0 +1,187 @@ +package tui + +import ( + "fmt" + "strings" + "testing" +) + +func TestEscClosesAFormWithNothingToLose(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "x") + v.HandleContentKey(keyPress("backspace")) + if form.edited() { + t.Fatal("a form typed into and emptied again has nothing in it") + } + v.HandleContentKey(keyPress("esc")) + if v.CapturingInput() { + t.Error("esc should close a form with nothing to lose") + } +} + +func TestEscLeavesAnUntouchedReplyWithoutAsking(t *testing.T) { + v, _ := recipientsTestView(t) + openThreadComposer(t, v, v.loadReplyContext(100, "Quarterly planning")) + v.HandleContentKey(keyPress("esc")) + if v.CapturingInput() { + t.Error("a reply holding only what HEY prefilled has nothing to lose") + } +} + +func TestEscAsksBeforeLosingAMessage(t *testing.T) { + v, rec := recipientsTestView(t) + form := openComposer(t, v) + form.focus = form.bodyIndex() + _ = form.focusCurrent() + typeText(v, "Lunch on Friday?") + + v.HandleContentKey(keyPress("esc")) + if !form.confirmLeave || composeModal(v) == nil { + t.Fatal("esc on an edited message should ask") + } + view := v.View() + for _, want := range []string{"Close this message?", "save draft", "discard", "keep editing"} { + if !strings.Contains(view, want) { + t.Errorf("the question should offer %q:\n%s", want, view) + } + } + if b := v.HelpBindings(); len(b) == 0 || b[0].key != "s" { + t.Errorf("the help bar should describe the question, got %v", b) + } + + // Typing is not an answer: the message is left as it was. + v.HandleContentKey(keyPress("x")) + if form.body.Value() != "Lunch on Friday?" || !form.confirmLeave { + t.Errorf("a stray key should neither type nor answer, body %q", form.body.Value()) + } + + v.HandleContentKey(keyPress("esc")) + if form.confirmLeave || composeModal(v) != form { + t.Fatal("esc on the question should go back to the message") + } + typeText(v, " Noon works.") + if form.body.Value() != "Lunch on Friday? Noon works." { + t.Errorf("editing should carry on where it was, body %q", form.body.Value()) + } + + v.HandleContentKey(keyPress("esc")) + v.HandleContentKey(keyPress("d")) + if v.CapturingInput() { + t.Error("d should discard the message") + } + if rec.writePath != "" { + t.Errorf("discarding should write nothing, got %s %s", rec.writeMethod, rec.writePath) + } +} + +func TestSavingADraftOnTheWayOut(t *testing.T) { + for _, key := range []string{"s", "enter"} { + t.Run(key, func(t *testing.T) { + v, rec := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "jan") + v.HandleContentKey(keyPress("tab")) + form.inputs[fieldSubject].SetValue("Lunch on Friday") + + v.HandleContentKey(keyPress("esc")) + cmd := v.HandleContentKey(keyPress(key)) + if !form.sending || !strings.Contains(v.View(), "Saving draft") { + t.Fatal("saving should hold the form and say so") + } + v.HandleContentKey(keyPress("esc")) + if composeModal(v) != form { + t.Fatal("a form that is saving holds on to esc") + } + saved := runCmd(cmd) + if _, ok := saved.(draftSavedMsg); !ok { + t.Fatalf("save = %#v", saved) + } + v.Update(saved) + if v.CapturingInput() { + t.Error("a saved draft should close the form") + } + + if rec.writeMethod != "POST" || rec.writePath != "/messages.json" { + t.Fatalf("draft went to %s %s", rec.writeMethod, rec.writePath) + } + entry, _ := rec.writeBody["entry"].(map[string]any) + message, _ := rec.writeBody["message"].(map[string]any) + addressed, _ := entry["addressed"].(map[string]any) + if entry["status"] != "drafted" || message["subject"] != "Lunch on Friday" || + fmt.Sprint(addressed["directly"]) != "[Jane Doe ]" { + t.Errorf("draft body = %v", rec.writeBody) + } + }) + } +} + +func TestSavingAReplyDraftFilesItUnderTheEntry(t *testing.T) { + v, rec := recipientsTestView(t) + form := openThreadComposer(t, v, v.loadReplyContext(100, "Quarterly planning")) + typeText(v, "Count me in.") + v.HandleContentKey(keyPress("esc")) + v.Update(runCmd(v.HandleContentKey(keyPress("s")))) + if v.CapturingInput() { + t.Fatal("a saved reply draft should close the form") + } + if rec.writePath != "/entries/501/replies.json" { + t.Fatalf("reply draft went to %s", rec.writePath) + } + entry, _ := rec.writeBody["entry"].(map[string]any) + message, _ := rec.writeBody["message"].(map[string]any) + if entry["status"] != "drafted" || message["subject"] != "Re: Quarterly planning" || + !strings.Contains(fmt.Sprint(message["content"]), "Count me in.") { + t.Errorf("reply draft body = %v", rec.writeBody) + } + _ = form +} + +func TestSavingAForwardDraftKeepsTheForwardedMessage(t *testing.T) { + v, rec := recipientsTestView(t) + openThreadComposer(t, v, v.loadForwardContext(100, "Quarterly planning")) + typeText(v, "morty@example.com") + v.HandleContentKey(keyPress("esc")) + v.Update(runCmd(v.HandleContentKey(keyPress("s")))) + if rec.writePath != "/messages.json" { + t.Fatalf("forward draft went to %s", rec.writePath) + } + message, _ := rec.writeBody["message"].(map[string]any) + if message["subject"] != "Fwd: Quarterly planning" || !strings.Contains(fmt.Sprint(message["content"]), "Quoted message") { + t.Errorf("forward draft body = %v", rec.writeBody) + } +} + +func TestAFailedDraftSaveKeepsTheMessage(t *testing.T) { + v, rec := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "rick@example.com") + rec.failDrafts = true + v.HandleContentKey(keyPress("esc")) + v.Update(runCmd(v.HandleContentKey(keyPress("s")))) + if composeModal(v) != form || form.sending || form.confirmLeave { + t.Fatal("a failed save should leave the message open to edit") + } + if !form.isError || !strings.Contains(form.status, "Could not save the draft") { + t.Errorf("status = %q", form.status) + } + if form.inputs[fieldTo].Value() != "rick@example.com" { + t.Errorf("the message should be as it was, To %q", form.inputs[fieldTo].Value()) + } +} + +func TestADraftWithAnAddressHEYWouldDropIsNotSaved(t *testing.T) { + v, rec := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "sam@example") + v.HandleContentKey(keyPress("esc")) + if cmd := v.HandleContentKey(keyPress("s")); cmd != nil { + t.Fatal("nothing should be sent") + } + if composeModal(v) != form || !form.isError || !strings.Contains(form.status, "sam@example") { + t.Errorf("the form should stay open naming the address, status %q", form.status) + } + if rec.writePath != "" { + t.Errorf("nothing should have been written, got %s", rec.writePath) + } +} diff --git a/internal/tui/compose_test.go b/internal/tui/compose_test.go index 21383bcdb..86a99b82b 100644 --- a/internal/tui/compose_test.go +++ b/internal/tui/compose_test.go @@ -684,10 +684,16 @@ func TestModelRoutesAllKeysToOpenForm(t *testing.T) { if composeModal(m.mailView).focus != int(fieldCc) { t.Errorf("tab should move to Cc, got %d", composeModal(m.mailView).focus) } + // The form holds a "q" now, so esc asks before it lets go of it. updated, _ = m.Update(keyPress("esc")) m = updated.(model) + if form := composeModal(m.mailView); form == nil || !form.confirmLeave { + t.Fatal("esc on an edited form should ask through the model") + } + updated, _ = m.Update(keyPress("d")) + m = updated.(model) if m.mailView.CapturingInput() { - t.Error("esc should close the form through the model") + t.Error("d should discard the form through the model") } } diff --git a/internal/tui/mail.go b/internal/tui/mail.go index 5b59cd87f..830b28651 100644 --- a/internal/tui/mail.go +++ b/internal/tui/mail.go @@ -306,8 +306,10 @@ type mailView struct { threadNotice string // what the open thread's read did not get; stays until the thread is left contentHeight int // the rows the section has, which the thread's notices and viewport share - modal modal // the form or picker over the list, and the only one there can be - cover coverPreset // the session's cover; HEY does not serve one to read + modal modal // the form or picker over the list, and the only one there can be + recipients []recipientSuggestion // HEY's list of who can be written to, read when a composer opens + recipientsLoading bool // a read of that list is on its way + cover coverPreset // the session's cover; HEY does not serve one to read searchList contentList searchActive bool searchQuery string @@ -608,9 +610,7 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { if cmd, ok := v.requests.settle(newRequestResult(msg.requestID, msg.err)); !ok { return cmd, true } - form := newReplyForm(msg, v.vc.styles) - v.openModal(form) - return form.init(), true + return v.openComposeForm(newReplyForm(msg, v.vc.styles)), true case forwardContextLoadedMsg: if msg.boxID != v.currentBoxID() { @@ -619,9 +619,7 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { if cmd, ok := v.requests.settle(newRequestResult(msg.requestID, msg.err)); !ok { return cmd, true } - form := newForwardForm(msg, v.vc.styles) - v.openModal(form) - return form.init(), true + return v.openComposeForm(newForwardForm(msg, v.vc.styles)), true case bulkReplyDraftLoadedMsg: if !v.requests.accepts(newRequestResult(msg.requestID, msg.err)) || msg.boxID != v.currentBoxID() || msg.seen != v.seenActive { @@ -705,6 +703,23 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { } return nil, true + case recipientsLoadedMsg: + v.settleRecipients(msg) + return nil, true + + case draftSavedMsg: + form := modalOf[*composeForm](v) + if form == nil || form != msg.form { + return nil, true + } + if msg.err != nil { + form.sending = false + form.setStatus(errorNotice("Could not save the draft", msg.err), true) + return nil, true + } + v.modal = nil + return notify("Draft saved"), true + case composeSentMsg: form := modalOf[*composeForm](v) if form == nil { diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go new file mode 100644 index 000000000..70a4550f1 --- /dev/null +++ b/internal/tui/recipients.go @@ -0,0 +1,326 @@ +package tui + +import ( + "strings" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + + hey "github.com/basecamp/hey-sdk/go/pkg/hey" + + "github.com/basecamp/hey-cli/internal/mail" + "github.com/basecamp/hey-cli/internal/terminal" +) + +// maxRecipientSuggestions is how many people the list under a recipient field +// offers at once. Everyone else is a few more letters away, and a short list is +// one the eye can take in without scrolling. +const maxRecipientSuggestions = 6 + +// recipientSuggestion is one row of the list HEY's own composer suggests +// recipients from: a person, a contact group, or everyone at an account. +type recipientSuggestion struct { + value string // one address, or a comma-separated list for a group + label string // the name to show + detail string // what a group or an account stands for; empty for a person + + // name and address are label and value folded to lower case once, when the + // list arrives, so matching a keystroke against thousands of rows compares + // strings without allocating any. + name string + address string +} + +// group reports whether picking the row adds more than one recipient. +func (s recipientSuggestion) group() bool { + return s.detail != "" || strings.Contains(s.value, ",") +} + +// recipientsLoadedMsg carries HEY's recipient list, already read into the +// shape the popover filters, so none of that work happens on the UI's loop. +type recipientsLoadedMsg struct { + suggestions []recipientSuggestion + err error +} + +// newRecipientSuggestions reads HEY's rows. Names and details are somebody +// else's text, so they are sanitized here, once, before anything draws them. +// HEY's order is kept: the people you wrote to most recently come first. +func newRecipientSuggestions(rows []hey.AddressableRecipient) []recipientSuggestion { + suggestions := make([]recipientSuggestion, 0, len(rows)) + for _, row := range rows { + value := strings.TrimSpace(terminal.SanitizeLine(row.Value)) + if value == "" { + continue + } + label := strings.TrimSpace(terminal.SanitizeLine(row.Label)) + if label == "" { + label = value + } + suggestion := recipientSuggestion{ + value: value, + label: label, + detail: strings.TrimSpace(terminal.SanitizeLine(row.Detail)), + name: strings.ToLower(label), + } + // A group is found by its name. Its members are people of their own, and + // matching their addresses would offer "Everyone at …" to anyone typing + // the first letters of one colleague. + if !suggestion.group() { + suggestion.address = strings.ToLower(value) + } + suggestions = append(suggestions, suggestion) + } + return suggestions +} + +// matchRecipients answers the rows worth offering for what has been typed of a +// recipient, best first, leaving out anyone the field already names. +// +// A row whose name has a word starting with the query, or whose address starts +// with it, beats one that only contains it somewhere: "an" finds Annie Bryan +// before Joanna. Within each of those HEY's order stands. The scan stops as soon +// as the better kind has filled the list, so a long address book costs a +// keystroke only as much of itself as it takes to find a few matches. +func matchRecipients(all []recipientSuggestion, query string, chosen map[string]bool) []recipientSuggestion { + query = strings.ToLower(strings.TrimSpace(query)) + if query == "" { + return nil + } + wordStart := " " + query + var best, rest []recipientSuggestion + for i := range all { + row := &all[i] + if chosen[row.address] { + continue + } + switch { + case strings.HasPrefix(row.name, query) || strings.HasPrefix(row.address, query) || + strings.Contains(row.name, wordStart): + best = append(best, *row) + if len(best) == maxRecipientSuggestions { + return best + } + case len(rest) < maxRecipientSuggestions && + (strings.Contains(row.name, query) || strings.Contains(row.address, query)): + rest = append(rest, *row) + } + } + return append(best, rest[:min(len(rest), maxRecipientSuggestions-len(best))]...) +} + +// chosenAddresses answers the bare addresses a recipient list already holds, +// lower-cased, leaving out the one being typed. +func chosenAddresses(list string, start, end int) map[string]bool { + chosen := map[string]bool{} + for _, recipient := range mail.SplitAddresses(list[:start] + list[end:]) { + address := recipient + if open := strings.LastIndexByte(recipient, '<'); open >= 0 { + address = strings.TrimSuffix(recipient[open+1:], ">") + } + chosen[strings.ToLower(strings.TrimSpace(address))] = true + } + return chosen +} + +// recipientText is what picking a row writes into the field: a person as +// Name
, and a group as the addresses it stands for, because HEY takes +// addresses rather than the name of a group. +func recipientText(s recipientSuggestion) string { + if s.group() { + return strings.Join(mail.SplitAddresses(s.value), ", ") + } + return mail.FormatAddress(s.label, s.value) +} + +// recipientPopover is the list open under a recipient field. It belongs to the +// field it was opened for and to the recipient being typed there, whose byte +// range in the field it keeps so that picking a row replaces that recipient and +// nothing else. +type recipientPopover struct { + field composeField + matches []recipientSuggestion + cursor int + start int + end int +} + +func (p *recipientPopover) selected() recipientSuggestion { + return p.matches[p.cursor] +} + +func (p *recipientPopover) move(delta int) { + p.cursor = min(max(p.cursor+delta, 0), len(p.matches)-1) +} + +// isRecipientField reports whether a form field takes addresses. +func isRecipientField(field int) bool { + return field == int(fieldTo) || field == int(fieldCc) || field == int(fieldBcc) +} + +// refreshSuggestions opens, narrows or closes the list for the recipient under +// the cursor in the focused field. +func (f *composeForm) refreshSuggestions() { + f.suggest = nil + if !isRecipientField(f.focus) || len(f.recipients) == 0 { + return + } + input := &f.inputs[f.focus] + value := input.Value() + cursor := byteOffset(value, input.Position()) + start, end := mail.AddressAt(value, cursor) + query := value[start:cursor] + matches := matchRecipients(f.recipients, query, chosenAddresses(value, start, end)) + if len(matches) == 0 { + return + } + // A recipient typed out in full needs nothing suggested for it. + if len(matches) == 1 && strings.EqualFold(strings.TrimSpace(value[start:end]), matches[0].value) { + return + } + f.suggest = &recipientPopover{field: composeField(f.focus), matches: matches, start: start, end: end} +} + +// acceptSuggestion writes the highlighted row in place of the recipient being +// typed, and leaves the cursor ready for the next one. +func (f *composeForm) acceptSuggestion() { + p := f.suggest + f.suggest = nil + f.typing = false + if p == nil || int(p.field) != f.focus { + return + } + input := &f.inputs[p.field] + value := input.Value() + if p.end > len(value) { + return + } + before := strings.TrimRight(value[:p.start], " ") + if before != "" { + before += " " + } + text := before + recipientText(p.selected()) + after := value[p.end:] + if strings.TrimSpace(after) == "" { + text += ", " + after = "" + } + input.SetValue(text + after) + input.SetCursor(len([]rune(text))) +} + +// handleSuggestionKey answers the keys the list takes while it is open: moving +// through it, picking from it and closing it. Everything else goes on to the +// field, which is what narrows the list. +func (f *composeForm) handleSuggestionKey(msg tea.KeyPressMsg) bool { + if f.suggest == nil || int(f.suggest.field) != f.focus { + return false + } + switch msg.String() { + case "up", "ctrl+p": + f.suggest.move(-1) + case "down", "ctrl+n": + f.suggest.move(1) + case "tab", "enter": + f.acceptSuggestion() + case "esc": + f.suggest = nil + f.typing = false + default: + return false + } + return true +} + +// recipientsLoaded hands the form HEY's list. Someone who started typing a +// recipient before it arrived gets their suggestions without typing again. +func (f *composeForm) recipientsLoaded(suggestions []recipientSuggestion) { + f.recipients = suggestions + if f.typing { + f.refreshSuggestions() + } +} + +// suggestionsView draws the list, to be laid over the form under its field. +func (f *composeForm) suggestionsView() string { + p := f.suggest + width := min(max(f.width-composeLabelWidth-2, 20), 72) + inner := width - 4 // border and padding + selected := lipgloss.NewStyle().Foreground(colorActive).Bold(true) + rows := make([]string, 0, len(p.matches)) + for i, match := range p.matches { + marker, name := " ", match.label + if i == p.cursor { + marker = "› " + } + secondary := match.value + if match.group() { + secondary = match.detail + } + if secondary == name { + secondary = "" + } + nameWidth := min(lipgloss.Width(name), max(inner-len(marker), 1)) + name = truncateToWidth(name, nameWidth) + line := marker + name + if i == p.cursor { + line = selected.Render(line) + } + if room := inner - len(marker) - nameWidth - 2; secondary != "" && room > 3 { + line += " " + styleMuted.Render(truncateToWidth(secondary, room)) + } + rows = append(rows, line) + } + return lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + BorderForeground(colorChrome). + Padding(0, 1). + Width(width). + Render(strings.Join(rows, "\n")) +} + +// byteOffset converts a cursor position in runes, which is how a text input +// counts, into a byte offset into its value. +func byteOffset(s string, runes int) int { + for i := range s { + if runes == 0 { + return i + } + runes-- + } + return len(s) +} + +// loadRecipients reads HEY's recipient list in the background. It is read +// every time a composer opens, so a person written to a minute ago is at the +// top of the list, but the SDK's response cache turns an unchanged list into a +// 304, and a composer that opens while the list is loading waits for that read +// rather than starting another. +func (v *mailView) loadRecipients() tea.Cmd { + if v.vc.sdk == nil || v.recipientsLoading { + return nil + } + v.recipientsLoading = true + sdk, ctx := v.vc.sdk, v.vc.ctx + return func() tea.Msg { + rows, err := sdk.Contacts().Addressable(ctx, true) + if err != nil { + return recipientsLoadedMsg{err: err} + } + return recipientsLoadedMsg{suggestions: newRecipientSuggestions(rows)} + } +} + +// settleRecipients keeps the list a read answered, and hands it to a composer +// that is open. A failed read keeps the list from before: suggestions are a +// convenience, and typing an address works without them. +func (v *mailView) settleRecipients(msg recipientsLoadedMsg) { + v.recipientsLoading = false + if msg.err != nil { + return + } + v.recipients = msg.suggestions + if form := modalOf[*composeForm](v); form != nil { + form.recipientsLoaded(v.recipients) + } +} diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go new file mode 100644 index 000000000..d42f83357 --- /dev/null +++ b/internal/tui/recipients_test.go @@ -0,0 +1,497 @@ +package tui + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "slices" + "strings" + "sync" + "testing" + + tea "charm.land/bubbletea/v2" + + hey "github.com/basecamp/hey-sdk/go/pkg/hey" +) + +const addressableJSON = `[ + ["rick@example.com","Rick Sanchez"], + ["jane@example.com","Jane Doe"], + ["annie@example.com","Bryan, Annie"], + ["joanna@example.org","Joanna Lumley"], + ["morty@example.com","morty@example.com"], + ["jane@example.com,rick@example.com,annie@example.com","Everyone at Honcho Design","@example.com"], + ["morty@example.com,summer@example.com","Book club","Contact group with 2 people"] +]` + +// recipientsRecorder is what the test server saw: how often the list was read +// and with what, and the last write. +type recipientsRecorder struct { + mu sync.Mutex + listReads int + includeSelf string + writeMethod string + writePath string + writeBody map[string]any + failDrafts bool + failList bool + addressables string +} + +func (r *recipientsRecorder) reads() int { + r.mu.Lock() + defer r.mu.Unlock() + return r.listReads +} + +// recipientsTestView is a mail view whose HEY serves a recipient list, a thread +// to reply to and forward, and accepts sends and drafts. +func recipientsTestView(t *testing.T) (*mailView, *recipientsRecorder) { + t.Helper() + rec := &recipientsRecorder{addressables: addressableJSON} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.mu.Lock() + defer rec.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/autocompletable/contacts/addressable.json": + rec.listReads++ + rec.includeSelf = r.URL.Query().Get("include_self") + if rec.failList { + w.WriteHeader(http.StatusInternalServerError) + return + } + _, _ = io.WriteString(w, rec.addressables) + case "/identity.json": + _, _ = io.WriteString(w, `{"id":1,"accounts":[{"id":9,"status":"active"}],"senders":[{"id":42,"account_id":9,"default":true}]}`) + case "/topics/100.json": + _, _ = io.WriteString(w, `{"id":100,"account_id":9,"name":"Quarterly planning","entries":[{"id":500},{"id":501}]}`) + case "/entries/501/replies/new.json": + _, _ = io.WriteString(w, `{"subject":"Re: Quarterly planning", + "addressed":{"directly":[{"id":3,"name":"Rick Sanchez","email_address":"rick@example.com"}]}}`) + case "/entries/501/forwards/new.json": + _, _ = io.WriteString(w, `{"subject":"Fwd: Quarterly planning","content":"
Quoted message
"}`) + default: + rec.writeMethod, rec.writePath = r.Method, r.URL.Path + rec.writeBody = nil + if b, _ := io.ReadAll(r.Body); len(b) > 0 { + _ = json.Unmarshal(b, &rec.writeBody) + } + if rec.failDrafts { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set("Location", "/messages/777") + w.WriteHeader(http.StatusNoContent) + } + })) + t.Cleanup(srv.Close) + sdk := hey.NewClient(&hey.Config{BaseURL: srv.URL}, &hey.StaticTokenProvider{Token: "t"}, hey.WithMaxRetries(0)) + vc := testVC() + vc.rootSDK = sdk + vc.sdk = sdk + vc.ctx = context.Background() + v := newMailView(vc) + v.boxes = orderBoxes(testBoxes()) + v.Update(currentPostingsLoaded(v, testPostings())) + v.Resize(100, 30) + return v, rec +} + +// settle runs the command a composer opened with and hands the view the list it +// read. The command is a batch whose last part is the read; the rest only focus a +// field, and running a cursor's blink would wait on its timer. +func settle(t *testing.T, v *mailView, cmd tea.Cmd) { + t.Helper() + msg := runCmd(cmd) + if batch, ok := msg.(tea.BatchMsg); ok && len(batch) > 0 { + msg = runCmd(batch[len(batch)-1]) + } + loaded, ok := msg.(recipientsLoadedMsg) + if !ok { + t.Fatalf("a composer should read the recipient list as it opens, got %T", msg) + } + v.Update(loaded) +} + +// openThreadComposer opens a reply or a forward from what HEY answered for it. +func openThreadComposer(t *testing.T, v *mailView, load tea.Cmd) *composeForm { + t.Helper() + cmd, _ := v.Update(runCmd(load)) + settle(t, v, cmd) + return composeModal(v) +} + +// openComposer opens a new message and lets the recipient list arrive. +func openComposer(t *testing.T, v *mailView) *composeForm { + t.Helper() + settle(t, v, v.HandleContentKey(keyPress("c"))) + form := composeModal(v) + if form == nil { + t.Fatal("c should open a new message") + } + if len(form.recipients) == 0 { + t.Fatal("the recipient list should have reached the form") + } + return form +} + +func suggestedLabels(f *composeForm) []string { + if f.suggest == nil { + return nil + } + var labels []string + for _, match := range f.suggest.matches { + labels = append(labels, match.label) + } + return labels +} + +func TestComposerReadsTheRecipientListOnceWithItself(t *testing.T) { + v, rec := recipientsTestView(t) + openComposer(t, v) + if rec.reads() != 1 { + t.Fatalf("list reads = %d, want 1", rec.reads()) + } + if rec.includeSelf != "true" { + t.Errorf("include_self = %q, want true, as the web composer asks", rec.includeSelf) + } + + // A second read while the first is on its way is the same read. + v.modal = nil + v.recipientsLoading = true + if cmd := v.loadRecipients(); cmd != nil { + t.Error("a composer opening while the list is loading should wait for that read") + } +} + +func TestTypingANameSuggestsAndTabAddsThePerson(t *testing.T) { + v, rec := recipientsTestView(t) + form := openComposer(t, v) + + typeText(v, "ja") + if got := suggestedLabels(form); !slices.Equal(got, []string{"Jane Doe"}) { + t.Fatalf("suggestions for ja = %q", got) + } + if view := v.View(); !strings.Contains(view, "Jane Doe") || !strings.Contains(view, "jane@example.com") { + t.Errorf("the list should be on screen with names and addresses:\n%s", view) + } + + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "Jane Doe , " { + t.Fatalf("To = %q", got) + } + if form.suggest != nil || form.focus != int(fieldTo) { + t.Fatalf("tab on the list should add the person and stay in To, focus %d", form.focus) + } + + // With the list closed tab moves on, as it always has. + v.HandleContentKey(keyPress("tab")) + if form.focus != int(fieldCc) { + t.Errorf("tab with no list open should move to Cc, got %d", form.focus) + } + + typeText(v, "bry") + v.HandleContentKey(keyPress("enter")) + if got := form.inputs[fieldCc].Value(); got != `"Bryan, Annie" , ` { + t.Fatalf("Cc = %q", got) + } + + form.inputs[fieldSubject].SetValue("Offsite agenda") + form.body.SetValue("Here is the plan for Thursday.") + if msg, ok := runCmd(v.HandleContentKey(ctrlS())).(composeSentMsg); !ok || msg.err != nil { + t.Fatalf("send = %#v", msg) + } + rec.mu.Lock() + defer rec.mu.Unlock() + entry, _ := rec.writeBody["entry"].(map[string]any) + addressed, _ := entry["addressed"].(map[string]any) + if fmt.Sprint(addressed["directly"]) != "[Jane Doe ]" || + fmt.Sprint(addressed["copied"]) != `["Bryan, Annie" ]` { + t.Errorf("sent addressed = %v", addressed) + } +} + +func TestArrowsChooseAndEscClosesOnlyTheList(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "j") + if form.suggest == nil || form.suggest.cursor != 0 { + t.Fatal("typing should open the list on its first row") + } + v.HandleContentKey(keyPress("down")) + v.HandleContentKey(keyPress("up")) + v.HandleContentKey(keyPress("down")) + if form.suggest.cursor != 1 { + t.Errorf("cursor = %d, want 1", form.suggest.cursor) + } + for range 10 { + v.HandleContentKey(tea.KeyPressMsg(tea.Key{Code: 'n', Mod: tea.ModCtrl})) + } + if form.suggest.cursor != len(form.suggest.matches)-1 { + t.Errorf("ctrl+n should stop on the last row, got %d", form.suggest.cursor) + } + + v.HandleContentKey(keyPress("esc")) + if form.suggest != nil { + t.Fatal("esc should close the list") + } + if composeModal(v) == nil || form.confirmLeave { + t.Fatal("esc on the list must not touch the message") + } +} + +func TestMovingTheCursorClosesTheList(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "ri") + if form.suggest == nil { + t.Fatal("typing should open the list") + } + v.HandleContentKey(keyPress("left")) + if form.suggest != nil { + t.Error("moving the cursor should close the list") + } + v.HandleContentKey(keyPress("right")) + if form.suggest != nil { + t.Error("moving back should not open it either") + } + typeText(v, "c") + if form.suggest == nil { + t.Error("typing again should open it again") + } +} + +func TestPickingAGroupAddsEveryAddress(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "book") + if got := suggestedLabels(form); !slices.Equal(got, []string{"Book club"}) { + t.Fatalf("suggestions for book = %q", got) + } + if view := v.View(); !strings.Contains(view, "Contact group with 2 people") { + t.Errorf("a group should say what it stands for:\n%s", view) + } + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "morty@example.com, summer@example.com, " { + t.Errorf("To = %q", got) + } +} + +func TestSuggestionsReplaceOnlyTheRecipientBeingTyped(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + to := &form.inputs[fieldTo] + to.SetValue("rick@example.com, jo, summer@example.com") + to.SetCursor(len("rick@example.com, jo")) + typeText(v, "a") + if got := suggestedLabels(form); len(got) == 0 || got[0] != "Joanna Lumley" { + t.Fatalf("suggestions for joa = %q", got) + } + v.HandleContentKey(keyPress("tab")) + if got := to.Value(); got != "rick@example.com, Joanna Lumley , summer@example.com" { + t.Errorf("To = %q", got) + } + if got := to.Position(); got != len([]rune("rick@example.com, Joanna Lumley ")) { + t.Errorf("cursor = %d, want it after the name it added", got) + } +} + +func TestSomeoneAlreadyOnTheLineIsNotSuggestedAgain(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + form.inputs[fieldTo].SetValue("Jane Doe , ") + form.inputs[fieldTo].CursorEnd() + typeText(v, "j") + if got := suggestedLabels(form); slices.Contains(got, "Jane Doe") { + t.Errorf("Jane is already on the line, suggestions = %q", got) + } +} + +func TestCcAndBccSuggestToo(t *testing.T) { + for _, field := range []composeField{fieldCc, fieldBcc} { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + form.focus = int(field) + _ = form.focusCurrent() + typeText(v, "rick") + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[field].Value(); got != "Rick Sanchez , " { + t.Errorf("field %d = %q", field, got) + } + } +} + +func TestReplyAndForwardSuggestRecipients(t *testing.T) { + v, rec := recipientsTestView(t) + form := openThreadComposer(t, v, v.loadReplyContext(100, "Quarterly planning")) + if form == nil || form.mode != composeReply { + t.Fatal("the reply form should be open") + } + form.focus = int(fieldCc) + _ = form.focusCurrent() + typeText(v, "jan") + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldCc].Value(); got != "Jane Doe , " { + t.Errorf("reply Cc = %q", got) + } + if got := form.inputs[fieldTo].Value(); got != "rick@example.com" { + t.Errorf("the prefilled To should be left alone, got %q", got) + } + + v.modal = nil + form = openThreadComposer(t, v, v.loadForwardContext(100, "Quarterly planning")) + if form == nil || form.mode != composeForward { + t.Fatal("the forward form should be open") + } + typeText(v, "mor") + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "morty@example.com, " { + t.Errorf("forward To = %q (a contact with no name goes in as the address)", got) + } + if rec.reads() != 2 { + t.Errorf("each composer should read the list again, reads = %d", rec.reads()) + } +} + +func TestTypingBeforeTheListArrivesSuggestsWhenItDoes(t *testing.T) { + v, _ := recipientsTestView(t) + cmd := v.HandleContentKey(keyPress("c")) + form := composeModal(v) + typeText(v, "jan") + if form.suggest != nil { + t.Fatal("there is nothing to suggest before the list arrives") + } + settle(t, v, cmd) + if got := suggestedLabels(form); !slices.Equal(got, []string{"Jane Doe"}) { + t.Errorf("suggestions once the list arrived = %q", got) + } +} + +func TestAFailedListReadLeavesTypingAlone(t *testing.T) { + v, rec := recipientsTestView(t) + rec.failList = true + settle(t, v, v.HandleContentKey(keyPress("c"))) + form := composeModal(v) + typeText(v, "jane@example.com") + if form.suggest != nil || form.inputs[fieldTo].Value() != "jane@example.com" { + t.Errorf("typing should work without the list, To = %q", form.inputs[fieldTo].Value()) + } + if v.recipientsLoading { + t.Error("a failed read should not stop the next composer from reading again") + } +} + +func TestAFailedReadKeepsTheListFromBefore(t *testing.T) { + v, rec := recipientsTestView(t) + openComposer(t, v) + v.modal = nil + rec.mu.Lock() + rec.failList = true + rec.mu.Unlock() + form := openComposer(t, v) + typeText(v, "rick") + if got := suggestedLabels(form); !slices.Equal(got, []string{"Rick Sanchez"}) { + t.Errorf("suggestions = %q", got) + } +} + +func TestSuggestionsAreSanitized(t *testing.T) { + v, rec := recipientsTestView(t) + rec.addressables = `[["mallory@example.com","Mallory \u001b[31mRed\u001b]8;;https://evil.example.com\u0007 Evil","\u001b[2Jwipe"]]` + form := openComposer(t, v) + typeText(v, "mal") + view := v.View() + if strings.Contains(view, "evil.example.com") || strings.Contains(view, "\x1b[2J") || strings.Contains(view, "\x1b]8") { + t.Errorf("escape sequences from a contact's name reached the screen: %q", view) + } + if got := suggestedLabels(form); len(got) != 1 || got[0] != "Mallory Red Evil" { + t.Errorf("label = %q", got) + } +} + +func TestTheListReachesMailFromAnotherSection(t *testing.T) { + m := modelWithBoxes() + m.mailView.recipientsLoading = true + updated, _ := m.switchSection(sectionCalendar) + m = updated.(model) + updated, _ = m.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions([]hey.AddressableRecipient{ + {Value: "jane@example.com", Label: "Jane Doe"}, + })}) + m = updated.(model) + if m.mailView.recipientsLoading || len(m.mailView.recipients) != 1 { + t.Errorf("a list that arrives in the calendar is still Mail's: loading %v, %d rows", + m.mailView.recipientsLoading, len(m.mailView.recipients)) + } +} + +func TestMatchRecipientsPutsWordStartsFirstAndKeepsHEYsOrder(t *testing.T) { + all := newRecipientSuggestions([]hey.AddressableRecipient{ + {Value: "joanna@example.org", Label: "Joanna Lumley"}, + {Value: "anna@example.com", Label: "Anna Karenina"}, + {Value: "susan@example.com", Label: "Susan Annaberg"}, + {Value: "hannah@example.com", Label: "Hannah Arendt"}, + {Value: "annual-report@example.com", Label: ""}, + }) + got := matchRecipients(all, "Ann", nil) + labels := make([]string, 0, len(got)) + for _, match := range got { + labels = append(labels, match.label) + } + want := []string{"Anna Karenina", "Susan Annaberg", "annual-report@example.com", "Joanna Lumley", "Hannah Arendt"} + if !slices.Equal(labels, want) { + t.Errorf("matches for Ann = %q, want %q", labels, want) + } + + if matches := matchRecipients(all, " ", nil); matches != nil { + t.Errorf("a blank query should suggest nothing, got %d", len(matches)) + } + for _, match := range matchRecipients(all, "anna", map[string]bool{"anna@example.com": true}) { + if match.value == "anna@example.com" { + t.Errorf("a chosen address should be left out, got %v", match) + } + } +} + +func TestMatchRecipientsStopsAtTheLimit(t *testing.T) { + rows := make([]hey.AddressableRecipient, 0, 50) + for i := range 50 { + rows = append(rows, hey.AddressableRecipient{Value: fmt.Sprintf("person%d@example.com", i), Label: fmt.Sprintf("Person %d", i)}) + } + matches := matchRecipients(newRecipientSuggestions(rows), "person", nil) + if len(matches) != maxRecipientSuggestions || matches[0].label != "Person 0" { + t.Errorf("matches = %d, first %q", len(matches), matches[0].label) + } +} + +func TestNewRecipientSuggestionsSkipsRowsWithNoAddress(t *testing.T) { + all := newRecipientSuggestions([]hey.AddressableRecipient{ + {Value: " ", Label: "Nobody"}, + {Value: "jane@example.com", Label: " "}, + }) + if len(all) != 1 || all[0].label != "jane@example.com" { + t.Errorf("suggestions = %+v", all) + } +} + +// BenchmarkMatchRecipients is a keystroke against a large address book: the +// scan HEY's whole list costs when the query matches almost nobody, which is the +// worst case, since a common one stops early. +func BenchmarkMatchRecipients(b *testing.B) { + rows := make([]hey.AddressableRecipient, 0, 20000) + for i := range 20000 { + rows = append(rows, hey.AddressableRecipient{ + Value: fmt.Sprintf("colleague.%d@example.com", i), + Label: fmt.Sprintf("Colleague Number %d", i), + }) + } + all := newRecipientSuggestions(rows) + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + _ = matchRecipients(all, "zelda", nil) + } +} diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 11486f75d..c07ea9357 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -416,7 +416,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.mailWatchAttempt++ return m, startMailWatchCmd(m.watchCtx, m.watchMail, m.mailWatchAttempt) - case mailRefreshDueMsg, postingsRefreshedMsg: + // The recipient list can arrive after the composer that asked for it has sent and + // the reader has gone to another section; the list is still Mail's to keep. + case mailRefreshDueMsg, postingsRefreshedMsg, recipientsLoadedMsg: cmd, _ := m.mailView.Update(msg) return m, m.stampViewCmd(cmd) From 0347f7de66f4861c7d0b7d60ded1412fae20b5c5 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 16:47:16 -0400 Subject: [PATCH 02/12] Take a picked recipient in one Backspace, and go back on Shift+Tab with a lock key on A recipient written Name
, the form the picker writes, now leaves in one Backspace with its comma, the way HEY's composer removes a recipient token. A bare address still deletes a letter at a time, since that is how a typo in one is fixed. Shift+Tab never went back a field with Num Lock or Caps Lock on. A terminal speaking the kitty keyboard protocol reports the lock keys as modifiers, every form compared the modifiers to Shift exactly, and the key fell through to plain Tab. Match the keystroke, which leaves the lock keys out, and ask whether Shift is held rather than whether it is all that is. --- docs/tui.md | 5 ++- internal/tui/calendar_settings.go | 2 +- internal/tui/compose.go | 11 +++++- internal/tui/compose_test.go | 17 +++++++++ internal/tui/contact_form.go | 2 +- internal/tui/event_form.go | 2 +- internal/tui/habit_form.go | 2 +- internal/tui/mail.go | 2 +- internal/tui/recipients.go | 29 +++++++++++++++ internal/tui/recipients_test.go | 61 +++++++++++++++++++++++++++++++ internal/tui/time_track_form.go | 2 +- internal/tui/tui.go | 2 +- 12 files changed, 128 insertions(+), 9 deletions(-) diff --git a/docs/tui.md b/docs/tui.md index 0c0b32ab4..efed1f604 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -132,7 +132,10 @@ wrote to most recently first, then your contacts by name, then "Everyone at" you company and your contact groups. It narrows as you type, matching a name or an address, and leaves out anyone already on that line. ↑ and ↓ (or Ctrl+P and Ctrl+N) move through it; Tab or Enter adds the highlighted person as `Name
, ` in place of what you -were typing, ready for the next one; a group adds every address in it. Escape closes the +were typing, ready for the next one; a group adds every address in it. Backspace right +after a recipient written `Name
` takes the whole recipient and its comma in one +press, the way HEY's composer removes a recipient; a bare address you typed still deletes +a letter at a time. Escape closes the list and nothing else, and moving the cursor closes it too. With the list closed, Tab and Enter move between fields as they always have. The list is read in the background when the form opens, so typing never waits on it, and an address typed in full works whether diff --git a/internal/tui/calendar_settings.go b/internal/tui/calendar_settings.go index 92f2c208e..306ceb054 100644 --- a/internal/tui/calendar_settings.go +++ b/internal/tui/calendar_settings.go @@ -65,7 +65,7 @@ func (f *calendarSettingsForm) handleKey(msg tea.KeyPressMsg) bool { } switch { - case msg.Key().Code == tea.KeyTab && msg.Key().Mod == tea.ModShift: + case msg.String() == "shift+tab": f.step(-1) return false case msg.Key().Code == tea.KeyTab, msg.Key().Code == tea.KeyEnter: diff --git a/internal/tui/compose.go b/internal/tui/compose.go index 05bf5d254..e80176c68 100644 --- a/internal/tui/compose.go +++ b/internal/tui/compose.go @@ -329,7 +329,7 @@ func (f *composeForm) handleKey(view *mailView, msg tea.KeyPressMsg) (tea.Cmd, b } f.confirmLeave = true return nil, true - case msg.Key().Code == tea.KeyTab && msg.Key().Mod == tea.ModShift: + case msg.String() == "shift+tab": f.focus = (f.focus + f.bodyIndex()) % (f.bodyIndex() + 1) return f.focusCurrent(), true case msg.Key().Code == tea.KeyTab: @@ -355,6 +355,15 @@ func (f *composeForm) edit(msg tea.KeyPressMsg) tea.Cmd { } input := &f.inputs[f.focus] value, position := input.Value(), input.Position() + if msg.String() == "backspace" { + if rest, cursor, ok := deleteRecipientBefore(value, byteOffset(value, position)); ok { + input.SetValue(rest) + input.SetCursor(len([]rune(rest[:cursor]))) + f.suggest = nil + f.typing = false + return nil + } + } cmd := f.update(msg) switch { case input.Value() != value: diff --git a/internal/tui/compose_test.go b/internal/tui/compose_test.go index 86a99b82b..32039a06e 100644 --- a/internal/tui/compose_test.go +++ b/internal/tui/compose_test.go @@ -133,6 +133,23 @@ func TestComposeTabCyclesFields(t *testing.T) { } } +// A terminal speaking the kitty keyboard protocol reports Num Lock and Caps Lock +// as modifiers, so Shift+Tab with Num Lock on arrives as Shift and Num Lock. It is +// still Shift+Tab, and it must not fall through to plain Tab and move forward. +func TestComposeShiftTabGoesBackWithALockKeyOn(t *testing.T) { + v := mailWithPostings() + v.HandleContentKey(keyPress("c")) + f := composeModal(v) + for _, lock := range []tea.KeyMod{tea.ModNumLock, tea.ModCapsLock, tea.ModNumLock | tea.ModCapsLock} { + f.focus = int(fieldSubject) + _ = f.focusCurrent() + v.HandleContentKey(tea.KeyPressMsg(tea.Key{Code: tea.KeyTab, Mod: tea.ModShift | lock})) + if f.focus != int(fieldBcc) { + t.Errorf("shift+tab with lock %v should move back to Bcc, got %d", lock, f.focus) + } + } +} + func TestComposeValidatesBeforeSending(t *testing.T) { v := mailWithPostings() v.HandleContentKey(keyPress("c")) diff --git a/internal/tui/contact_form.go b/internal/tui/contact_form.go index c8502cd22..b1c2c0d4b 100644 --- a/internal/tui/contact_form.go +++ b/internal/tui/contact_form.go @@ -106,7 +106,7 @@ func (f *contactForm) handleKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { return nil, false } switch { - case msg.Key().Code == tea.KeyTab && msg.Key().Mod == tea.ModShift: + case msg.String() == "shift+tab": f.focus = (f.focus + len(f.inputs) - 1) % len(f.inputs) return f.focusCurrent(), false case msg.Key().Code == tea.KeyTab || msg.Key().Code == tea.KeyEnter: diff --git a/internal/tui/event_form.go b/internal/tui/event_form.go index 952828687..61a506142 100644 --- a/internal/tui/event_form.go +++ b/internal/tui/event_form.go @@ -969,7 +969,7 @@ func (f *eventForm) handleKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { } switch { - case msg.Key().Code == tea.KeyTab && msg.Key().Mod == tea.ModShift: + case msg.String() == "shift+tab": return f.step(-1), false case msg.Key().Code == tea.KeyTab: return f.step(1), false diff --git a/internal/tui/habit_form.go b/internal/tui/habit_form.go index ba3940182..987a9f5b1 100644 --- a/internal/tui/habit_form.go +++ b/internal/tui/habit_form.go @@ -146,7 +146,7 @@ func (f *habitForm) handleKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { return nil, false } switch { - case msg.Key().Code == tea.KeyTab && msg.Key().Mod == tea.ModShift: + case msg.String() == "shift+tab": f.focus = (f.focus + habitFieldCount - 1) % habitFieldCount return f.focusCurrent(), false case msg.Key().Code == tea.KeyTab || msg.Key().Code == tea.KeyEnter: diff --git a/internal/tui/mail.go b/internal/tui/mail.go index 830b28651..ff28451d4 100644 --- a/internal/tui/mail.go +++ b/internal/tui/mail.go @@ -2324,7 +2324,7 @@ func (v *mailView) handleLinkKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { } if msg.Key().Code == tea.KeyTab { delta := 1 - if msg.Key().Mod == tea.ModShift { + if msg.Key().Mod.Contains(tea.ModShift) { delta = -1 } previous := v.selectedLink diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index 70a4550f1..a45e5604a 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -209,6 +209,35 @@ func (f *composeForm) acceptSuggestion() { input.SetCursor(len([]rune(text))) } +// deleteRecipientBefore answers the list with the recipient just before a cursor at +// byte offset pos taken out whole, and where the cursor goes, when that recipient is +// written Name
— the form the picker writes. Backspace then takes it in one +// press, the way HEY's composer takes a recipient token, with its comma. A bare +// address is left to delete a letter at a time, since that is how a typo in one is +// fixed. +func deleteRecipientBefore(value string, pos int) (string, int, bool) { + before := strings.TrimRight(value[:pos], " ") + before = strings.TrimRight(strings.TrimSuffix(before, ","), " ") + if !strings.HasSuffix(before, ">") { + return "", 0, false + } + start, end := mail.AddressAt(value, len(before)-1) + recipient := strings.TrimSpace(value[start:end]) + open := strings.LastIndexByte(recipient, '<') + if open <= 0 || strings.TrimSpace(recipient[:open]) == "" { + return "", 0, false + } + + head := strings.TrimRight(value[:start], " ") + tail := strings.TrimLeft(value[pos:], " ") + tail = strings.TrimLeft(strings.TrimPrefix(tail, ","), " ") + if head == "" { + return tail, 0, true + } + head += " " + return head + tail, len(head), true +} + // handleSuggestionKey answers the keys the list takes while it is open: moving // through it, picking from it and closing it. Everything else goes on to the // field, which is what narrows the list. diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index d42f83357..57630e9b4 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -428,6 +428,67 @@ func TestTheListReachesMailFromAnotherSection(t *testing.T) { } } +func TestDeleteRecipientBeforeTakesANamedRecipientWhole(t *testing.T) { + for _, tc := range []struct { + name string + value string + pos int + want string + wantCursor int + ok bool + }{ + {name: "the only one, after its comma", value: "Jane Doe , ", pos: 29, want: "", wantCursor: 0, ok: true}, + {name: "the only one, right after it", value: "Jane Doe ", pos: 27, want: "", wantCursor: 0, ok: true}, + {name: "the last of two", value: "Jane Doe , Rick Sanchez , ", pos: 62, + want: "Jane Doe , ", wantCursor: 29, ok: true}, + {name: "one in the middle", value: "Jane Doe , Rick Sanchez , summer@example.com", pos: 60, + want: "Jane Doe , summer@example.com", wantCursor: 29, ok: true}, + {name: "the first of two", value: "Jane Doe , rick@example.com", pos: 27, + want: "rick@example.com", wantCursor: 0, ok: true}, + {name: "a quoted name with a comma", value: `"Bryan, Annie" , `, pos: 36, want: "", wantCursor: 0, ok: true}, + {name: "a bare address deletes a letter at a time", value: "jane@example.com, ", pos: 18, ok: false}, + {name: "an address in brackets with no name", value: ", ", pos: 20, ok: false}, + {name: "typing the next one", value: "Jane Doe , ri", pos: 31, ok: false}, + {name: "inside the address", value: "Jane Doe ", pos: 20, ok: false}, + } { + got, cursor, ok := deleteRecipientBefore(tc.value, tc.pos) + if ok != tc.ok || got != tc.want || cursor != tc.wantCursor { + t.Errorf("%s: deleteRecipientBefore = %q, %d, %v; want %q, %d, %v", tc.name, got, cursor, ok, tc.want, tc.wantCursor, tc.ok) + } + } +} + +func TestBackspaceTakesAPickedRecipientInOnePress(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "jan") + v.HandleContentKey(keyPress("tab")) + typeText(v, "rick") + v.HandleContentKey(keyPress("tab")) + to := &form.inputs[fieldTo] + if to.Value() != "Jane Doe , Rick Sanchez , " { + t.Fatalf("To = %q", to.Value()) + } + + v.HandleContentKey(keyPress("backspace")) + if got := to.Value(); got != "Jane Doe , " { + t.Fatalf("one backspace should take Rick whole, To = %q", got) + } + if to.Position() != len([]rune(to.Value())) { + t.Errorf("the cursor should be ready for the next recipient, at %d", to.Position()) + } + v.HandleContentKey(keyPress("backspace")) + if got := to.Value(); got != "" { + t.Errorf("a second backspace should take Jane, To = %q", got) + } + + typeText(v, "sam@example.com") + v.HandleContentKey(keyPress("backspace")) + if got := to.Value(); got != "sam@example.co" { + t.Errorf("a typed address deletes a letter at a time, To = %q", got) + } +} + func TestMatchRecipientsPutsWordStartsFirstAndKeepsHEYsOrder(t *testing.T) { all := newRecipientSuggestions([]hey.AddressableRecipient{ {Value: "joanna@example.org", Label: "Joanna Lumley"}, diff --git a/internal/tui/time_track_form.go b/internal/tui/time_track_form.go index a19ceb651..bdfc9efcc 100644 --- a/internal/tui/time_track_form.go +++ b/internal/tui/time_track_form.go @@ -176,7 +176,7 @@ func (f *timeTrackForm) handleKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { } switch { - case msg.Key().Code == tea.KeyTab && msg.Key().Mod == tea.ModShift: + case msg.String() == "shift+tab": return f.step(-1), false case msg.Key().Code == tea.KeyTab, msg.Key().Code == tea.KeyEnter: return f.step(1), false diff --git a/internal/tui/tui.go b/internal/tui/tui.go index c07ea9357..6432fdba4 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -957,7 +957,7 @@ func (m model) handleKey(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { } if msg.Key().Code == tea.KeyTab { - if msg.Key().Mod == tea.ModShift { + if msg.Key().Mod.Contains(tea.ModShift) { m.focus = (m.focus + 2) % 3 } else { m.focus = (m.focus + 1) % 3 From 9925b179a800e7c09c84e8f0e256b12de84305f2 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 20:38:22 -0400 Subject: [PATCH 03/12] Address the review on recipient suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A selected row measured its "› " marker in bytes, four, rather than the two cells it takes, so selecting a row cut its name short. - A contact group already on the line was still suggested; it is left out once every one of its addresses is there. - The help bar now follows a list that opens because the recipients arrived while a name was being typed. - API-COVERAGE.md lists /autocompletable/contacts/addressable. - The tests read and set the test server's recorder under its lock. - Shift+Tab with a lock key on is tested in the contact and habit forms as well as the composer. --- API-COVERAGE.md | 1 + internal/tui/compose_leave_test.go | 38 +++++------ internal/tui/recipients.go | 31 +++++++-- internal/tui/recipients_test.go | 102 +++++++++++++++++++++++++---- internal/tui/shift_tab_test.go | 32 +++++++++ internal/tui/tui.go | 11 +++- 6 files changed, 180 insertions(+), 35 deletions(-) create mode 100644 internal/tui/shift_tab_test.go diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 6fccf453b..e1b7222d3 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -56,6 +56,7 @@ forward answers. The drafts index (`/entries/drafts.json`) pages the same way, w | `/contacts/{id}/note.json` | GET | SDK `Contacts().Note` | `hey contact note show`, Contacts TUI | covered | | `/contacts/{id}/note.json` | PATCH | SDK `Contacts().SetNote` | `hey contact note set`, Contacts TUI | covered | | `/contacts/{id}/note.json` | DELETE | SDK `Contacts().DeleteNote` | `hey contact note delete`, Contacts TUI | covered | +| `/autocompletable/contacts/addressable.json` | GET | SDK `Contacts().Addressable` | TUI composer's To, Cc and Bcc suggestions (new message, reply, forward) | covered | | `/calendars.json` | GET | SDK `Calendars().List` | `hey calendar list` | covered | | `/calendars/{id}/recordings.json` | GET | SDK `Calendars().GetRecordings` | `hey event list`, `hey event edit ` (reading the event back; with `--occurrence`, the occurrence's day and then the series' first day for its `Calendar::Countdown`), `hey event delete ` (checking the id is not a day of a series HEY has written out; with `--occurrence --apply-to future`, the occurrence's day), `hey todo list`, `hey journal list` | covered | | `/topics/{id}/entries.json` | GET | SDK `Topics().GetEntriesPage` | `hey thread read `, `hey attachment list `, TUI thread view; `hey reply `, `hey compose --thread-id`, `hey forward ` and TUI `r`/`f` when the topic's own page holds no emailed message (`mail.ReplyTarget`) | covered, but see the paging note above | diff --git a/internal/tui/compose_leave_test.go b/internal/tui/compose_leave_test.go index 96d0cc012..0498b139f 100644 --- a/internal/tui/compose_leave_test.go +++ b/internal/tui/compose_leave_test.go @@ -70,8 +70,8 @@ func TestEscAsksBeforeLosingAMessage(t *testing.T) { if v.CapturingInput() { t.Error("d should discard the message") } - if rec.writePath != "" { - t.Errorf("discarding should write nothing, got %s %s", rec.writeMethod, rec.writePath) + if rec.seen().writePath != "" { + t.Errorf("discarding should write nothing, got %s %s", rec.seen().writeMethod, rec.seen().writePath) } } @@ -102,15 +102,15 @@ func TestSavingADraftOnTheWayOut(t *testing.T) { t.Error("a saved draft should close the form") } - if rec.writeMethod != "POST" || rec.writePath != "/messages.json" { - t.Fatalf("draft went to %s %s", rec.writeMethod, rec.writePath) + if rec.seen().writeMethod != "POST" || rec.seen().writePath != "/messages.json" { + t.Fatalf("draft went to %s %s", rec.seen().writeMethod, rec.seen().writePath) } - entry, _ := rec.writeBody["entry"].(map[string]any) - message, _ := rec.writeBody["message"].(map[string]any) + entry, _ := rec.seen().writeBody["entry"].(map[string]any) + message, _ := rec.seen().writeBody["message"].(map[string]any) addressed, _ := entry["addressed"].(map[string]any) if entry["status"] != "drafted" || message["subject"] != "Lunch on Friday" || fmt.Sprint(addressed["directly"]) != "[Jane Doe ]" { - t.Errorf("draft body = %v", rec.writeBody) + t.Errorf("draft body = %v", rec.seen().writeBody) } }) } @@ -125,14 +125,14 @@ func TestSavingAReplyDraftFilesItUnderTheEntry(t *testing.T) { if v.CapturingInput() { t.Fatal("a saved reply draft should close the form") } - if rec.writePath != "/entries/501/replies.json" { - t.Fatalf("reply draft went to %s", rec.writePath) + if rec.seen().writePath != "/entries/501/replies.json" { + t.Fatalf("reply draft went to %s", rec.seen().writePath) } - entry, _ := rec.writeBody["entry"].(map[string]any) - message, _ := rec.writeBody["message"].(map[string]any) + entry, _ := rec.seen().writeBody["entry"].(map[string]any) + message, _ := rec.seen().writeBody["message"].(map[string]any) if entry["status"] != "drafted" || message["subject"] != "Re: Quarterly planning" || !strings.Contains(fmt.Sprint(message["content"]), "Count me in.") { - t.Errorf("reply draft body = %v", rec.writeBody) + t.Errorf("reply draft body = %v", rec.seen().writeBody) } _ = form } @@ -143,12 +143,12 @@ func TestSavingAForwardDraftKeepsTheForwardedMessage(t *testing.T) { typeText(v, "morty@example.com") v.HandleContentKey(keyPress("esc")) v.Update(runCmd(v.HandleContentKey(keyPress("s")))) - if rec.writePath != "/messages.json" { - t.Fatalf("forward draft went to %s", rec.writePath) + if rec.seen().writePath != "/messages.json" { + t.Fatalf("forward draft went to %s", rec.seen().writePath) } - message, _ := rec.writeBody["message"].(map[string]any) + message, _ := rec.seen().writeBody["message"].(map[string]any) if message["subject"] != "Fwd: Quarterly planning" || !strings.Contains(fmt.Sprint(message["content"]), "Quoted message") { - t.Errorf("forward draft body = %v", rec.writeBody) + t.Errorf("forward draft body = %v", rec.seen().writeBody) } } @@ -156,7 +156,7 @@ func TestAFailedDraftSaveKeepsTheMessage(t *testing.T) { v, rec := recipientsTestView(t) form := openComposer(t, v) typeText(v, "rick@example.com") - rec.failDrafts = true + rec.configure(func(r *recipientsRecorder) { r.failDrafts = true }) v.HandleContentKey(keyPress("esc")) v.Update(runCmd(v.HandleContentKey(keyPress("s")))) if composeModal(v) != form || form.sending || form.confirmLeave { @@ -181,7 +181,7 @@ func TestADraftWithAnAddressHEYWouldDropIsNotSaved(t *testing.T) { if composeModal(v) != form || !form.isError || !strings.Contains(form.status, "sam@example") { t.Errorf("the form should stay open naming the address, status %q", form.status) } - if rec.writePath != "" { - t.Errorf("nothing should have been written, got %s", rec.writePath) + if rec.seen().writePath != "" { + t.Errorf("nothing should have been written, got %s", rec.seen().writePath) } } diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index a45e5604a..791530a7f 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -29,6 +29,24 @@ type recipientSuggestion struct { // strings without allocating any. name string address string + + // members is a group's addresses, lower-cased, so that a group already on + // the line can be left out the way a person is. + members []string +} + +// chosen reports whether the line already holds the row: a person's address, +// or every one of a group's. +func (s *recipientSuggestion) chosen(on map[string]bool) bool { + if len(s.members) == 0 { + return on[s.address] + } + for _, member := range s.members { + if !on[member] { + return false + } + } + return true } // group reports whether picking the row adds more than one recipient. @@ -66,7 +84,11 @@ func newRecipientSuggestions(rows []hey.AddressableRecipient) []recipientSuggest // A group is found by its name. Its members are people of their own, and // matching their addresses would offer "Everyone at …" to anyone typing // the first letters of one colleague. - if !suggestion.group() { + if suggestion.group() { + for _, member := range mail.SplitAddresses(value) { + suggestion.members = append(suggestion.members, strings.ToLower(member)) + } + } else { suggestion.address = strings.ToLower(value) } suggestions = append(suggestions, suggestion) @@ -91,7 +113,7 @@ func matchRecipients(all []recipientSuggestion, query string, chosen map[string] var best, rest []recipientSuggestion for i := range all { row := &all[i] - if chosen[row.address] { + if row.chosen(chosen) { continue } switch { @@ -289,13 +311,14 @@ func (f *composeForm) suggestionsView() string { if secondary == name { secondary = "" } - nameWidth := min(lipgloss.Width(name), max(inner-len(marker), 1)) + markerWidth := lipgloss.Width(marker) + nameWidth := min(lipgloss.Width(name), max(inner-markerWidth, 1)) name = truncateToWidth(name, nameWidth) line := marker + name if i == p.cursor { line = selected.Render(line) } - if room := inner - len(marker) - nameWidth - 2; secondary != "" && room > 3 { + if room := inner - markerWidth - nameWidth - 2; secondary != "" && room > 3 { line += " " + styleMuted.Render(truncateToWidth(secondary, room)) } rows = append(rows, line) diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index 57630e9b4..2e78943a0 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -42,9 +42,36 @@ type recipientsRecorder struct { } func (r *recipientsRecorder) reads() int { + return r.seen().listReads +} + +// recorded is what the server has seen, copied out under the lock so a test +// never reads a field the handler goroutine is writing. +type recorded struct { + listReads int + includeSelf string + writeMethod string + writePath string + writeBody map[string]any +} + +func (r *recipientsRecorder) seen() recorded { + r.mu.Lock() + defer r.mu.Unlock() + return recorded{ + listReads: r.listReads, + includeSelf: r.includeSelf, + writeMethod: r.writeMethod, + writePath: r.writePath, + writeBody: r.writeBody, + } +} + +// configure changes how the server answers, under the lock the handler reads it with. +func (r *recipientsRecorder) configure(change func(r *recipientsRecorder)) { r.mu.Lock() defer r.mu.Unlock() - return r.listReads + change(r) } // recipientsTestView is a mail view whose HEY serves a recipient list, a thread @@ -156,8 +183,8 @@ func TestComposerReadsTheRecipientListOnceWithItself(t *testing.T) { if rec.reads() != 1 { t.Fatalf("list reads = %d, want 1", rec.reads()) } - if rec.includeSelf != "true" { - t.Errorf("include_self = %q, want true, as the web composer asks", rec.includeSelf) + if rec.seen().includeSelf != "true" { + t.Errorf("include_self = %q, want true, as the web composer asks", rec.seen().includeSelf) } // A second read while the first is on its way is the same read. @@ -205,9 +232,7 @@ func TestTypingANameSuggestsAndTabAddsThePerson(t *testing.T) { if msg, ok := runCmd(v.HandleContentKey(ctrlS())).(composeSentMsg); !ok || msg.err != nil { t.Fatalf("send = %#v", msg) } - rec.mu.Lock() - defer rec.mu.Unlock() - entry, _ := rec.writeBody["entry"].(map[string]any) + entry, _ := rec.seen().writeBody["entry"].(map[string]any) addressed, _ := entry["addressed"].(map[string]any) if fmt.Sprint(addressed["directly"]) != "[Jane Doe ]" || fmt.Sprint(addressed["copied"]) != `["Bryan, Annie" ]` { @@ -311,6 +336,61 @@ func TestSomeoneAlreadyOnTheLineIsNotSuggestedAgain(t *testing.T) { } } +func TestAGroupAlreadyOnTheLineIsNotSuggestedAgain(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "book") + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "morty@example.com, summer@example.com, " { + t.Fatalf("To = %q", got) + } + typeText(v, "book") + if got := suggestedLabels(form); slices.Contains(got, "Book club") { + t.Errorf("every member of Book club is already on the line, suggestions = %q", got) + } +} + +// A selected row draws "› " before its name, which is four bytes and two cells. +// Measuring it in bytes cut the name short the moment the row was selected. +func TestASelectedRowKeepsItsWholeName(t *testing.T) { + // A 42-cell form leaves 26 cells inside the list's frame; after the two-cell + // marker that is room for exactly this 24-cell name. + name := "Josephine Montgomery-Lee" + form := newComposeForm(composeNew, newStyles()) + form.width = 42 + form.recipients = newRecipientSuggestions([]hey.AddressableRecipient{{Value: "bart@example.com", Label: name}}) + form.focus = int(fieldTo) + form.inputs[fieldTo].SetValue("bart") + form.inputs[fieldTo].CursorEnd() + form.refreshSuggestions() + if form.suggest == nil { + t.Fatal("the list should be open") + } + if view := form.suggestionsView(); !strings.Contains(view, name) { + t.Errorf("the selected row should show the whole name:\n%s", view) + } +} + +func TestTheHelpBarFollowsAListThatOpensWhenTheRecipientsArrive(t *testing.T) { + m := modelWithBoxes() + updated, _ := m.Update(keyPress("c")) + m = updated.(model) + for _, r := range "jan" { + updated, _ = m.Update(tea.KeyPressMsg(tea.Key{Code: r, Text: string(r)})) + m = updated.(model) + } + updated, _ = m.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions([]hey.AddressableRecipient{ + {Value: "jane@example.com", Label: "Jane Doe"}, + })}) + m = updated.(model) + if form := composeModal(m.mailView); form == nil || form.suggest == nil { + t.Fatal("the list should open when the recipients arrive mid-word") + } + if len(m.help.bindings) == 0 || m.help.bindings[0].desc != "choose" { + t.Errorf("the help bar should describe the list, got %v", m.help.bindings) + } +} + func TestCcAndBccSuggestToo(t *testing.T) { for _, field := range []composeField{fieldCc, fieldBcc} { v, _ := recipientsTestView(t) @@ -373,7 +453,7 @@ func TestTypingBeforeTheListArrivesSuggestsWhenItDoes(t *testing.T) { func TestAFailedListReadLeavesTypingAlone(t *testing.T) { v, rec := recipientsTestView(t) - rec.failList = true + rec.configure(func(r *recipientsRecorder) { r.failList = true }) settle(t, v, v.HandleContentKey(keyPress("c"))) form := composeModal(v) typeText(v, "jane@example.com") @@ -389,9 +469,7 @@ func TestAFailedReadKeepsTheListFromBefore(t *testing.T) { v, rec := recipientsTestView(t) openComposer(t, v) v.modal = nil - rec.mu.Lock() - rec.failList = true - rec.mu.Unlock() + rec.configure(func(r *recipientsRecorder) { r.failList = true }) form := openComposer(t, v) typeText(v, "rick") if got := suggestedLabels(form); !slices.Equal(got, []string{"Rick Sanchez"}) { @@ -401,7 +479,9 @@ func TestAFailedReadKeepsTheListFromBefore(t *testing.T) { func TestSuggestionsAreSanitized(t *testing.T) { v, rec := recipientsTestView(t) - rec.addressables = `[["mallory@example.com","Mallory \u001b[31mRed\u001b]8;;https://evil.example.com\u0007 Evil","\u001b[2Jwipe"]]` + rec.configure(func(r *recipientsRecorder) { + r.addressables = `[["mallory@example.com","Mallory \u001b[31mRed\u001b]8;;https://evil.example.com\u0007 Evil","\u001b[2Jwipe"]]` + }) form := openComposer(t, v) typeText(v, "mal") view := v.View() diff --git a/internal/tui/shift_tab_test.go b/internal/tui/shift_tab_test.go new file mode 100644 index 000000000..dfabdf2a9 --- /dev/null +++ b/internal/tui/shift_tab_test.go @@ -0,0 +1,32 @@ +package tui + +import ( + "testing" + + tea "charm.land/bubbletea/v2" +) + +// A terminal speaking the kitty keyboard protocol reports Num Lock and Caps Lock +// as modifiers. Every form matches Shift+Tab by its keystroke, which leaves the +// lock keys out; TestComposeShiftTabGoesBackWithALockKeyOn holds the composer to +// that, and this holds the other forms. +func TestFormsGoBackOnShiftTabWithALockKeyOn(t *testing.T) { + locks := []tea.KeyMod{0, tea.ModNumLock, tea.ModCapsLock, tea.ModNumLock | tea.ModCapsLock} + shiftTab := func(lock tea.KeyMod) tea.KeyPressMsg { + return tea.KeyPressMsg(tea.Key{Code: tea.KeyTab, Mod: tea.ModShift | lock}) + } + + for _, lock := range locks { + contact := newContactForm(contactFormAdd, Contact{}, newStyles()) + contact.handleKey(shiftTab(lock)) + if want := len(contact.inputs) - 1; contact.focus != want { + t.Errorf("contact form, lock %v: shift+tab from the first field went to %d, want %d", lock, contact.focus, want) + } + + habit := newHabitForm(habitFormCreate, Recording{}, newStyles()) + habit.handleKey(shiftTab(lock)) + if want := habitFieldCount - 1; habit.focus != want { + t.Errorf("habit form, lock %v: shift+tab from the first field went to %d, want %d", lock, habit.focus, want) + } + } +} diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 6432fdba4..45ea5b6dd 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -418,10 +418,19 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // The recipient list can arrive after the composer that asked for it has sent and // the reader has gone to another section; the list is still Mail's to keep. - case mailRefreshDueMsg, postingsRefreshedMsg, recipientsLoadedMsg: + case mailRefreshDueMsg, postingsRefreshedMsg: cmd, _ := m.mailView.Update(msg) return m, m.stampViewCmd(cmd) + case recipientsLoadedMsg: + // Arriving mid-word opens the list under the field, and the help bar has + // to say what the list's keys do. + cmd, _ := m.mailView.Update(msg) + if m.activeView == m.mailView { + m.updateHelpBindings() + } + return m, m.stampViewCmd(cmd) + case screenerWatchStartedMsg: if msg.stream != m.screenerStream { return m, nil From 2d5972400d8f6162a31e8b6f823e345dadc900ee Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 20:51:41 -0400 Subject: [PATCH 04/12] Keep suggestions true to the address, a paste and a quoted name - A suggestion whose address sanitizing would change is left out rather than offered under an address it doesn't have: an invisible character removed from joanna@example.com leaves a different mailbox. Names are still sanitized for display. - A paste into a recipient field narrows or closes the list the way typing does, so a pick afterwards replaces the whole recipient. - Backspace takes a recipient whole only at its real end, not after a > inside a quoted name. --- internal/tui/compose.go | 15 +++++++++++- internal/tui/recipients.go | 13 ++++++++-- internal/tui/recipients_test.go | 43 +++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/internal/tui/compose.go b/internal/tui/compose.go index e80176c68..3968518b0 100644 --- a/internal/tui/compose.go +++ b/internal/tui/compose.go @@ -420,7 +420,20 @@ func (f *composeForm) handleMsg(msg tea.Msg) (tea.Cmd, bool) { if f.snippetPicker != nil { return f.snippetPicker.handleMsg(msg), true } - return f.update(msg), true + // A paste changes a recipient field without a key press, and the list + // under it keeps the byte range of the recipient being typed, so it has to + // follow the change the way typing does. + if !isRecipientField(f.focus) { + return f.update(msg), true + } + input := &f.inputs[f.focus] + value := input.Value() + cmd := f.update(msg) + if input.Value() != value { + f.typing = true + f.refreshSuggestions() + } + return cmd, true } func (f *composeForm) openSnippetPicker(view *mailView) tea.Cmd { diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index 791530a7f..e0985f5af 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -67,8 +67,12 @@ type recipientsLoadedMsg struct { func newRecipientSuggestions(rows []hey.AddressableRecipient) []recipientSuggestion { suggestions := make([]recipientSuggestion, 0, len(rows)) for _, row := range rows { - value := strings.TrimSpace(terminal.SanitizeLine(row.Value)) - if value == "" { + // The value is where mail goes, so sanitizing it may not change it: an + // address holding a control or an invisible format character would be + // rewritten into a different mailbox. Such a row is left out rather + // than offered under an address it doesn't have. + value := strings.TrimSpace(row.Value) + if value == "" || terminal.SanitizeLine(value) != value { continue } label := strings.TrimSpace(terminal.SanitizeLine(row.Label)) @@ -244,6 +248,11 @@ func deleteRecipientBefore(value string, pos int) (string, int, bool) { return "", 0, false } start, end := mail.AddressAt(value, len(before)-1) + // The > has to be the recipient's last character. One inside a quoted name, + // "Jane > Doe" , is a character like any other. + if start+len(strings.TrimRight(value[start:end], " ")) != len(before) { + return "", 0, false + } recipient := strings.TrimSpace(value[start:end]) open := strings.LastIndexByte(recipient, '<') if open <= 0 || strings.TrimSpace(recipient[:open]) == "" { diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index 2e78943a0..fab2a083b 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -391,6 +391,46 @@ func TestTheHelpBarFollowsAListThatOpensWhenTheRecipientsArrive(t *testing.T) { } } +func TestPastingWithTheListOpenKeepsItInStep(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "ja") + if form.suggest == nil { + t.Fatal("typing should open the list") + } + v.Update(tea.PasteMsg{Content: "n"}) + if got := form.inputs[fieldTo].Value(); got != "jan" { + t.Fatalf("the paste should land in the field, To = %q", got) + } + if got := suggestedLabels(form); !slices.Equal(got, []string{"Jane Doe"}) { + t.Fatalf("the list should narrow on a paste as on typing, got %q", got) + } + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "Jane Doe , " { + t.Errorf("a pick after a paste must replace the whole recipient, To = %q", got) + } + + // A paste that matches nobody closes the list. + typeText(v, "ri") + v.Update(tea.PasteMsg{Content: "x@example.com"}) + if form.suggest != nil { + t.Errorf("nobody matches rix@example.com, so the list should close, got %q", suggestedLabels(form)) + } +} + +func TestAnAddressSanitizingWouldChangeIsNotOffered(t *testing.T) { + all := newRecipientSuggestions([]hey.AddressableRecipient{ + {Value: "jo\u200banna@example.com", Label: "Joanna Lumley"}, + {Value: "jane@example.com", Label: "Jane \u001b[31mDoe"}, + }) + if len(all) != 1 || all[0].value != "jane@example.com" { + t.Fatalf("suggestions = %+v, want only Jane", all) + } + if all[0].label != "Jane Doe" { + t.Errorf("a name is still sanitized for display, got %q", all[0].label) + } +} + func TestCcAndBccSuggestToo(t *testing.T) { for _, field := range []composeField{fieldCc, fieldBcc} { v, _ := recipientsTestView(t) @@ -530,6 +570,9 @@ func TestDeleteRecipientBeforeTakesANamedRecipientWhole(t *testing.T) { {name: "an address in brackets with no name", value: ", ", pos: 20, ok: false}, {name: "typing the next one", value: "Jane Doe , ri", pos: 31, ok: false}, {name: "inside the address", value: "Jane Doe ", pos: 20, ok: false}, + {name: "after a > inside a quoted name", value: `"Jane > Doe" `, pos: 7, ok: false}, + {name: "a quoted name with a > in it, at its end", value: `"Jane > Doe" , `, pos: 33, + want: "", wantCursor: 0, ok: true}, } { got, cursor, ok := deleteRecipientBefore(tc.value, tc.pos) if ok != tc.ok || got != tc.want || cursor != tc.wantCursor { From 041b8509257fc94809da09e74ebbf1b1b8a9edca Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 20:58:04 -0400 Subject: [PATCH 05/12] Hold a paste off while the close question is up or a save is on its way A paste reached the form through handleMsg while keys were held, so it could edit the message behind the close question, or change it after a draft save had taken its values and be lost when the saved form closed. --- internal/tui/compose.go | 7 ++++++ internal/tui/compose_leave_test.go | 34 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/internal/tui/compose.go b/internal/tui/compose.go index 3968518b0..82962fcce 100644 --- a/internal/tui/compose.go +++ b/internal/tui/compose.go @@ -417,6 +417,13 @@ func (f *composeForm) submit(view *mailView) tea.Cmd { } func (f *composeForm) handleMsg(msg tea.Msg) (tea.Cmd, bool) { + // A paste is typing, so it is held off for as long as keys are: while the + // close question is up, and while a send or a draft save is on its way, + // which has already taken the form's values and would close it on top of + // whatever was pasted. + if _, paste := msg.(tea.PasteMsg); paste && (f.sending || f.confirmLeave) { + return nil, true + } if f.snippetPicker != nil { return f.snippetPicker.handleMsg(msg), true } diff --git a/internal/tui/compose_leave_test.go b/internal/tui/compose_leave_test.go index 0498b139f..7be83cf51 100644 --- a/internal/tui/compose_leave_test.go +++ b/internal/tui/compose_leave_test.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" "testing" + + tea "charm.land/bubbletea/v2" ) func TestEscClosesAFormWithNothingToLose(t *testing.T) { @@ -170,6 +172,38 @@ func TestAFailedDraftSaveKeepsTheMessage(t *testing.T) { } } +// A paste is held off exactly as keys are while the close question is up and +// while a draft save is on its way: a paste behind the question would edit a +// form the reader can't see, and one during the save would be lost when the +// saved form closes. +func TestAPasteWaitsForTheQuestionAndTheSave(t *testing.T) { + v, rec := recipientsTestView(t) + form := openComposer(t, v) + form.focus = form.bodyIndex() + _ = form.focusCurrent() + typeText(v, "Lunch on Friday?") + + v.HandleContentKey(keyPress("esc")) + v.Update(tea.PasteMsg{Content: " Noon works."}) + if got := form.body.Value(); got != "Lunch on Friday?" { + t.Errorf("a paste behind the close question changed the message: %q", got) + } + + cmd := v.HandleContentKey(keyPress("s")) + if !form.sending { + t.Fatal("s should start saving the draft") + } + v.Update(tea.PasteMsg{Content: " Noon works."}) + if got := form.body.Value(); got != "Lunch on Friday?" { + t.Errorf("a paste during the save changed the message: %q", got) + } + v.Update(runCmd(cmd)) + message, _ := rec.seen().writeBody["message"].(map[string]any) + if content := fmt.Sprint(message["content"]); !strings.Contains(content, "Lunch on Friday?") || strings.Contains(content, "Noon") { + t.Errorf("the draft should be the message as it was when saved, got %q", content) + } +} + func TestADraftWithAnAddressHEYWouldDropIsNotSaved(t *testing.T) { v, rec := recipientsTestView(t) form := openComposer(t, v) From 222e4e822548d55fe4b80467770f9f4c65455dba Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 21:12:37 -0400 Subject: [PATCH 06/12] Keep the highlighted person when the recipient list is read again A composer opens on the list it has and reads it again behind the scenes. When that read landed it rebuilt the open list and put the highlight back on the first row, so Tab could add someone other than the person picked with the arrows. The highlight now follows that person by address. --- internal/tui/recipients.go | 20 ++++++++++++++++++-- internal/tui/recipients_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index e0985f5af..7e32968d4 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -296,8 +296,24 @@ func (f *composeForm) handleSuggestionKey(msg tea.KeyPressMsg) bool { // recipient before it arrived gets their suggestions without typing again. func (f *composeForm) recipientsLoaded(suggestions []recipientSuggestion) { f.recipients = suggestions - if f.typing { - f.refreshSuggestions() + if !f.typing { + return + } + // A list re-read behind an open one must not move the highlight: the + // person picked with the arrows is the one tab adds, wherever they now sit. + var selected string + if f.suggest != nil { + selected = f.suggest.selected().value + } + f.refreshSuggestions() + if f.suggest == nil || selected == "" { + return + } + for i, match := range f.suggest.matches { + if match.value == selected { + f.suggest.cursor = i + return + } } } diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index fab2a083b..e06fa2c89 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -431,6 +431,39 @@ func TestAnAddressSanitizingWouldChangeIsNotOffered(t *testing.T) { } } +// A composer opens on the list it already has and reads it again behind the +// scenes. When that read lands, the person highlighted with the arrows stays +// highlighted, wherever the fresh list puts them. +func TestAReloadKeepsTheHighlightedPerson(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + typeText(v, "j") + v.HandleContentKey(keyPress("down")) + if got := form.suggest.selected().label; got != "Joanna Lumley" { + t.Fatalf("down should highlight Joanna, got %q", got) + } + + // The read behind the composer answers the same list it already had. + v.Update(recipientsLoadedMsg{suggestions: v.recipients}) + if form.suggest == nil || form.suggest.selected().label != "Joanna Lumley" { + t.Fatalf("the reload moved the highlight off Joanna: %q", suggestedLabels(form)) + } + + // And when the fresh list puts her somewhere else, she is still the one. + v.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions([]hey.AddressableRecipient{ + {Value: "jane@example.com", Label: "Jane Doe"}, + {Value: "jack@example.com", Label: "Jack Black"}, + {Value: "joanna@example.org", Label: "Joanna Lumley"}, + })}) + if form.suggest == nil || form.suggest.selected().label != "Joanna Lumley" { + t.Fatalf("the reload moved the highlight off Joanna: %q", suggestedLabels(form)) + } + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "Joanna Lumley , " { + t.Errorf("tab should add the person highlighted, To = %q", got) + } +} + func TestCcAndBccSuggestToo(t *testing.T) { for _, field := range []composeField{fieldCc, fieldBcc} { v, _ := recipientsTestView(t) From 370141ca6d79391757c88993004fa473f0031cb1 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 21:18:58 -0400 Subject: [PATCH 07/12] Close the list when a re-read pushes the highlighted person out of it The list shows six people. When the fresh list put the highlighted person seventh, the highlight fell back to the first row, so Tab would add someone nobody picked. The list now closes instead, and the next key opens it again from the top. --- internal/tui/recipients.go | 4 ++++ internal/tui/recipients_test.go | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index 7e32968d4..29e5b11d5 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -315,6 +315,10 @@ func (f *composeForm) recipientsLoaded(suggestions []recipientSuggestion) { return } } + // The fresh list pushed them out of the rows shown. Highlighting someone + // else would have tab add a person nobody picked, so the list closes and + // the next key opens it again from the top. + f.suggest = nil } // suggestionsView draws the list, to be laid over the form under its field. diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index e06fa2c89..3a710cf0f 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -464,6 +464,40 @@ func TestAReloadKeepsTheHighlightedPerson(t *testing.T) { } } +// When the fresh list pushes the highlighted person past the rows shown, the +// list closes rather than highlight somebody nobody picked. +func TestAReloadThatPushesTheHighlightedPersonOutClosesTheList(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + people := []hey.AddressableRecipient{ + {Value: "jack@example.com", Label: "Jack Black"}, + {Value: "jade@example.com", Label: "Jade Smith"}, + {Value: "jake@example.com", Label: "Jake Peralta"}, + {Value: "jean@example.com", Label: "Jean Grey"}, + {Value: "jill@example.com", Label: "Jill Valentine"}, + {Value: "joan@example.com", Label: "Joan Holloway"}, + } + v.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions(people)}) + typeText(v, "j") + for range maxRecipientSuggestions - 1 { + v.HandleContentKey(keyPress("down")) + } + if got := form.suggest.selected().label; got != "Joan Holloway" { + t.Fatalf("the sixth row should be highlighted, got %q", got) + } + + // Someone new wrote to us, and the fresh list puts them first. + fresh := append([]hey.AddressableRecipient{{Value: "jane@example.com", Label: "Jane Doe"}}, people...) + v.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions(fresh)}) + if form.suggest != nil { + t.Errorf("Joan is past the rows shown now, so the list should close, got %q highlighted", form.suggest.selected().label) + } + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "j" { + t.Errorf("tab with the list closed must not add anyone, To = %q", got) + } +} + func TestCcAndBccSuggestToo(t *testing.T) { for _, field := range []composeField{fieldCc, fieldBcc} { v, _ := recipientsTestView(t) From 48254fc775b66854060f6f388eb1872fb7792dcd Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 21:31:35 -0400 Subject: [PATCH 08/12] Recognise a recipient written with a comment as already on the line jane@example.com (Jane Doe) and Jane Doe (work) were read as different addresses from jane@example.com, so Jane was offered again, and a group they completed stayed suggested. mail.BareAddress exposes the parser that already understands comments, and both the line's recipients and a group's members are compared through it. --- internal/mail/address.go | 12 ++++++++++++ internal/mail/address_at_test.go | 18 ++++++++++++++++++ internal/tui/recipients.go | 8 ++------ internal/tui/recipients_test.go | 27 +++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/internal/mail/address.go b/internal/mail/address.go index f7dfadb80..61b088083 100644 --- a/internal/mail/address.go +++ b/internal/mail/address.go @@ -137,6 +137,18 @@ func deliverable(address string) bool { return deliverableDomain(spec[at+1:]) } +// BareAddress returns the address a recipient names, without its display name, +// angle brackets or comments: "Jane Doe (work)" and +// "jane@example.com (Jane Doe)" are both jane@example.com. A recipient HEY's +// parser would refuse, or text with no address in it, is returned trimmed, as +// written. +func BareAddress(recipient string) string { + if spec, _, ok := addrSpec(recipient); ok && strings.Contains(spec, "@") { + return spec + } + return strings.TrimSpace(recipient) +} + // addrSpec returns the bare address and the fewest characters HEY could write the // whole address out in, so that a size over the limit is over it for HEY too. ok is // false for what HEY's parser refuses outright: an unclosed angle bracket, or a second diff --git a/internal/mail/address_at_test.go b/internal/mail/address_at_test.go index 9a2668e2a..d52878fca 100644 --- a/internal/mail/address_at_test.go +++ b/internal/mail/address_at_test.go @@ -34,6 +34,24 @@ func TestAddressAtFindsTheRecipientUnderTheCursor(t *testing.T) { } } +func TestBareAddressDropsTheNameAndComments(t *testing.T) { + for recipient, want := range map[string]string{ + "jane@example.com": "jane@example.com", + " jane@example.com ": "jane@example.com", + "Jane Doe ": "jane@example.com", + "Jane Doe (work)": "jane@example.com", + "jane@example.com (Jane Doe)": "jane@example.com", + `"Bryan, Annie" `: "annie@example.com", + "(personal) rick@example.org": "rick@example.org", + "not an address": "not an address", + "Jane Doe = 0 { - address = strings.TrimSuffix(recipient[open+1:], ">") - } - chosen[strings.ToLower(strings.TrimSpace(address))] = true + chosen[strings.ToLower(mail.BareAddress(recipient))] = true } return chosen } diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index 3a710cf0f..5e5543e3a 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -336,6 +336,33 @@ func TestSomeoneAlreadyOnTheLineIsNotSuggestedAgain(t *testing.T) { } } +// A recipient written with a comment, the way some mail clients copy it, is +// still on the line: it isn't offered again, and neither is a group it completes. +func TestARecipientWithACommentIsNotSuggestedAgain(t *testing.T) { + for _, line := range []string{ + "jane@example.com (Jane Doe), ", + "Jane Doe (work), ", + } { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + form.inputs[fieldTo].SetValue(line) + form.inputs[fieldTo].CursorEnd() + typeText(v, "ja") + if got := suggestedLabels(form); slices.Contains(got, "Jane Doe") { + t.Errorf("with %q on the line, Jane was offered again: %q", line, got) + } + } + + v, _ := recipientsTestView(t) + form := openComposer(t, v) + form.inputs[fieldTo].SetValue("morty@example.com (Morty), Summer (school), ") + form.inputs[fieldTo].CursorEnd() + typeText(v, "book") + if got := suggestedLabels(form); slices.Contains(got, "Book club") { + t.Errorf("every member of Book club is on the line, suggestions = %q", got) + } +} + func TestAGroupAlreadyOnTheLineIsNotSuggestedAgain(t *testing.T) { v, _ := recipientsTestView(t) form := openComposer(t, v) From 433f8cfe0d5e776047b517d9e392337a8e1b8e69 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 21:49:42 -0400 Subject: [PATCH 09/12] Open no list for an address typed out in full MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A full address someone in the list has needs nothing suggested, but the guard only held when it was the one match. With another row mentioning it — jane.work@example.com labelled "jane@example.com (work)" — the list stayed open on that row and Tab swapped the typed address for it. Any exact match in the whole list now keeps the list shut. --- internal/tui/recipients.go | 22 +++++++++++++++++-- internal/tui/recipients_test.go | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index 48f458511..9827c2f8d 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -135,6 +135,22 @@ func matchRecipients(all []recipientSuggestion, query string, chosen map[string] return append(best, rest[:min(len(rest), maxRecipientSuggestions-len(best))]...) } +// knownAddress reports whether a recipient, as typed, is exactly the address of +// someone in the list, anywhere in it, not just in the rows a query would show. +// Only something with an @ in it can be, so a name being typed costs nothing. +func knownAddress(all []recipientSuggestion, recipient string) bool { + address := strings.ToLower(mail.BareAddress(recipient)) + if !strings.Contains(address, "@") { + return false + } + for i := range all { + if all[i].address == address { + return true + } + } + return false +} + // chosenAddresses answers the bare addresses a recipient list already holds, // lower-cased, leaving out the one being typed. func chosenAddresses(list string, start, end int) map[string]bool { @@ -196,8 +212,10 @@ func (f *composeForm) refreshSuggestions() { if len(matches) == 0 { return } - // A recipient typed out in full needs nothing suggested for it. - if len(matches) == 1 && strings.EqualFold(strings.TrimSpace(value[start:end]), matches[0].value) { + // A recipient typed out in full needs nothing suggested for it, however many + // other rows it also matches: tab then moves on, rather than swap the + // address for a different mailbox whose name happens to contain it. + if knownAddress(f.recipients, value[start:end]) { return } f.suggest = &recipientPopover{field: composeField(f.focus), matches: matches, start: start, end: end} diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index 5e5543e3a..356b107d9 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -525,6 +525,45 @@ func TestAReloadThatPushesTheHighlightedPersonOutClosesTheList(t *testing.T) { } } +// An address typed out in full that someone in the list has is finished: no +// list opens, however many other rows mention it, and tab moves on rather than +// swap it for one of them. +func TestATypedOutAddressOpensNoList(t *testing.T) { + work := hey.AddressableRecipient{Value: "jane.work@example.com", Label: "jane@example.com (work)"} + jane := hey.AddressableRecipient{Value: "jane@example.com", Label: "Jane Doe"} + + t.Run("more than one row matches", func(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + v.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions([]hey.AddressableRecipient{work, jane})}) + typeText(v, "jane@example.com") + if form.suggest != nil { + t.Fatalf("a typed-out address opened a list: %q", suggestedLabels(form)) + } + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != "jane@example.com" || form.focus != int(fieldCc) { + t.Errorf("tab should leave the address and move on, To = %q, focus %d", got, form.focus) + } + }) + + t.Run("the address is past the rows shown", func(t *testing.T) { + v, _ := recipientsTestView(t) + form := openComposer(t, v) + teams := []string{"design", "sales", "support", "finance", "legal", "ops"} + rows := make([]hey.AddressableRecipient, 0, len(teams)+1) + for _, team := range teams { + rows = append(rows, hey.AddressableRecipient{ + Value: "jane." + team + "@example.com", Label: "jane@example.com (" + team + ")", + }) + } + v.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions(append(rows, jane))}) + typeText(v, "jane@example.com") + if form.suggest != nil { + t.Errorf("a typed-out address opened a list: %q", suggestedLabels(form)) + } + }) +} + func TestCcAndBccSuggestToo(t *testing.T) { for _, field := range []composeField{fieldCc, fieldBcc} { v, _ := recipientsTestView(t) From bda0c50060af99e357f5cf9f130f2299b05a18c4 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 21:56:26 -0400 Subject: [PATCH 10/12] Wrap the close question to a narrow terminal The frame fits only its title to the screen, so at 40 columns the question and its choices ran past the edge and the overlay cut off the border and part of "esc keep editing". The body now wraps to the room inside the frame. --- internal/tui/compose.go | 3 +++ internal/tui/compose_leave_test.go | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/internal/tui/compose.go b/internal/tui/compose.go index 82962fcce..6ca07f0fa 100644 --- a/internal/tui/compose.go +++ b/internal/tui/compose.go @@ -520,6 +520,9 @@ func (f *composeForm) leaveView() string { key := lipgloss.NewStyle().Foreground(colorActive).Bold(true) body := "Keep it as a draft to finish later, here or in HEY?\n\n" + key.Render("s") + " save draft " + key.Render("d") + " discard " + key.Render("esc") + " keep editing" + // The frame only fits its title to the screen, so the question and the + // choices wrap to the room inside it, or a narrow terminal cuts them off. + body = lipgloss.NewStyle().Width(modalContentWidth(f.width)).Render(body) return modalFrame("Close this message?", body, f.width) } diff --git a/internal/tui/compose_leave_test.go b/internal/tui/compose_leave_test.go index 7be83cf51..599024878 100644 --- a/internal/tui/compose_leave_test.go +++ b/internal/tui/compose_leave_test.go @@ -6,6 +6,8 @@ import ( "testing" tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" ) func TestEscClosesAFormWithNothingToLose(t *testing.T) { @@ -204,6 +206,26 @@ func TestAPasteWaitsForTheQuestionAndTheSave(t *testing.T) { } } +// The close question wraps to a narrow terminal rather than run off its edge. +func TestTheCloseQuestionFitsANarrowTerminal(t *testing.T) { + form := newComposeForm(composeNew, newStyles()) + for _, width := range []int{40, 30} { + form.width = width + dialog := form.leaveView() + for _, line := range strings.Split(dialog, "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("at %d columns a line of the question is %d wide: %q", width, w, line) + } + } + plain := ansi.Strip(dialog) + for _, want := range []string{"save draft", "discard", "keep editing"} { + if !strings.Contains(strings.Join(strings.Fields(plain), " "), want) { + t.Errorf("at %d columns the question lost %q:\n%s", width, want, plain) + } + } + } +} + func TestADraftWithAnAddressHEYWouldDropIsNotSaved(t *testing.T) { v, rec := recipientsTestView(t) form := openComposer(t, v) From d0a4b2928148b25cfe9f87963090c88bb1994ea6 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 22:03:08 -0400 Subject: [PATCH 11/12] Compare a contact's address the way the line's recipients are compared A person's comparison key kept a quoted local part as written, while the recipients on the line went through the parser, so "jane doe"@example.com never matched itself: it was offered again once added, and typing it out didn't close the list. The key now goes through BareAddress too; the address written into the field is still exactly HEY's. --- internal/tui/recipients.go | 5 ++++- internal/tui/recipients_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index 9827c2f8d..d2d775293 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -93,7 +93,10 @@ func newRecipientSuggestions(rows []hey.AddressableRecipient) []recipientSuggest suggestion.members = append(suggestion.members, strings.ToLower(mail.BareAddress(member))) } } else { - suggestion.address = strings.ToLower(value) + // Compared the way the line's recipients are, through the parser, so + // a quoted local part ("jane doe"@example.com) matches itself. The + // value written into the field stays exactly as HEY gave it. + suggestion.address = strings.ToLower(mail.BareAddress(value)) } suggestions = append(suggestions, suggestion) } diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index 356b107d9..2a6c3fe1c 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -564,6 +564,33 @@ func TestATypedOutAddressOpensNoList(t *testing.T) { }) } +// An address with a quoted local part is compared in the parsed form the line's +// recipients are: once added it isn't offered again, and typed out in full it +// opens no list. +func TestAQuotedAddressMatchesItself(t *testing.T) { + quoted := hey.AddressableRecipient{Value: `"jane doe"@example.com`, Label: "Jane Doe"} + other := hey.AddressableRecipient{Value: "jane.doe.work@example.com", Label: `"jane doe"@example.com (work)`} + + v, _ := recipientsTestView(t) + form := openComposer(t, v) + v.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions([]hey.AddressableRecipient{other, quoted})}) + form.inputs[fieldTo].SetValue(`Jane Doe <"jane doe"@example.com>, `) + form.inputs[fieldTo].CursorEnd() + typeText(v, "jane d") + if got := suggestedLabels(form); slices.Contains(got, "Jane Doe") { + t.Errorf("Jane is already on the line, suggestions = %q", got) + } + + form.inputs[fieldTo].SetValue("") + typeText(v, `"jane doe"@example.com`) + if form.suggest != nil { + t.Errorf("a typed-out quoted address opened a list: %q", suggestedLabels(form)) + } + if got := newRecipientSuggestions([]hey.AddressableRecipient{quoted})[0].value; got != `"jane doe"@example.com` { + t.Errorf("the address written into the field must stay as HEY gave it, got %q", got) + } +} + func TestCcAndBccSuggestToo(t *testing.T) { for _, field := range []composeField{fieldCc, fieldBcc} { v, _ := recipientsTestView(t) From 63dc42296163d043aa989c6f1a476edb513bd9dd Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 22:10:30 -0400 Subject: [PATCH 12/12] Treat a quoted comma as one mailbox, and match the address as written A comma in the value made a row a group, so "jane,doe"@example.com lost its address matching and the guard against swapping a typed-out address. Whether a row is a group is now decided once, from HEY's detail or from the value holding more than one address. Typing is matched against the address as written, quotes and all, and the parsed address is what recognises it on the line. --- internal/tui/recipients.go | 20 +++++++++++++++----- internal/tui/recipients_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/internal/tui/recipients.go b/internal/tui/recipients.go index d2d775293..8122b8ef4 100644 --- a/internal/tui/recipients.go +++ b/internal/tui/recipients.go @@ -24,15 +24,23 @@ type recipientSuggestion struct { label string // the name to show detail string // what a group or an account stands for; empty for a person - // name and address are label and value folded to lower case once, when the + // name and written are label and value folded to lower case once, when the // list arrives, so matching a keystroke against thousands of rows compares - // strings without allocating any. + // strings without allocating any. written is the address as someone types + // it, quotes and all; address is the same address parsed, which is how it + // is recognised as already on the line. Both are empty for a group. name string + written string address string // members is a group's addresses, lower-cased, so that a group already on // the line can be left out the way a person is. members []string + + // isGroup is decided once, when the list arrives: a row HEY describes (an + // account or a contact group), or a value holding more than one address. + // A comma alone doesn't make one — "jane,doe"@example.com is one mailbox. + isGroup bool } // chosen reports whether the line already holds the row: a person's address, @@ -51,7 +59,7 @@ func (s *recipientSuggestion) chosen(on map[string]bool) bool { // group reports whether picking the row adds more than one recipient. func (s recipientSuggestion) group() bool { - return s.detail != "" || strings.Contains(s.value, ",") + return s.isGroup } // recipientsLoadedMsg carries HEY's recipient list, already read into the @@ -85,6 +93,7 @@ func newRecipientSuggestions(rows []hey.AddressableRecipient) []recipientSuggest detail: strings.TrimSpace(terminal.SanitizeLine(row.Detail)), name: strings.ToLower(label), } + suggestion.isGroup = suggestion.detail != "" || len(mail.SplitAddresses(value)) > 1 // A group is found by its name. Its members are people of their own, and // matching their addresses would offer "Everyone at …" to anyone typing // the first letters of one colleague. @@ -96,6 +105,7 @@ func newRecipientSuggestions(rows []hey.AddressableRecipient) []recipientSuggest // Compared the way the line's recipients are, through the parser, so // a quoted local part ("jane doe"@example.com) matches itself. The // value written into the field stays exactly as HEY gave it. + suggestion.written = strings.ToLower(value) suggestion.address = strings.ToLower(mail.BareAddress(value)) } suggestions = append(suggestions, suggestion) @@ -124,14 +134,14 @@ func matchRecipients(all []recipientSuggestion, query string, chosen map[string] continue } switch { - case strings.HasPrefix(row.name, query) || strings.HasPrefix(row.address, query) || + case strings.HasPrefix(row.name, query) || strings.HasPrefix(row.written, query) || strings.Contains(row.name, wordStart): best = append(best, *row) if len(best) == maxRecipientSuggestions { return best } case len(rest) < maxRecipientSuggestions && - (strings.Contains(row.name, query) || strings.Contains(row.address, query)): + (strings.Contains(row.name, query) || strings.Contains(row.written, query)): rest = append(rest, *row) } } diff --git a/internal/tui/recipients_test.go b/internal/tui/recipients_test.go index 2a6c3fe1c..fdc4aa257 100644 --- a/internal/tui/recipients_test.go +++ b/internal/tui/recipients_test.go @@ -591,6 +591,37 @@ func TestAQuotedAddressMatchesItself(t *testing.T) { } } +// A comma inside a quoted local part makes one mailbox, not a group: it is +// found by its address, picked under its name, and typed out in full it is +// finished even when another contact's label mentions it. +func TestAQuotedCommaIsOneMailbox(t *testing.T) { + mailbox := hey.AddressableRecipient{Value: `"jane,doe"@example.com`, Label: "Jane Doe"} + other := hey.AddressableRecipient{Value: "jane.doe.work@example.com", Label: `"jane,doe"@example.com (work)`} + + all := newRecipientSuggestions([]hey.AddressableRecipient{mailbox}) + if all[0].group() { + t.Fatal(`"jane,doe"@example.com is one mailbox, not a group`) + } + + v, _ := recipientsTestView(t) + form := openComposer(t, v) + v.Update(recipientsLoadedMsg{suggestions: newRecipientSuggestions([]hey.AddressableRecipient{other, mailbox})}) + typeText(v, `"jane,d`) + if got := suggestedLabels(form); !slices.Contains(got, "Jane Doe") { + t.Fatalf("the mailbox should be found by its address, suggestions = %q", got) + } + + form.inputs[fieldTo].SetValue("") + typeText(v, `"jane,doe"@example.com`) + if form.suggest != nil { + t.Errorf("a typed-out quoted-comma address opened a list: %q", suggestedLabels(form)) + } + v.HandleContentKey(keyPress("tab")) + if got := form.inputs[fieldTo].Value(); got != `"jane,doe"@example.com` { + t.Errorf("tab must leave the typed address alone, To = %q", got) + } +} + func TestCcAndBccSuggestToo(t *testing.T) { for _, field := range []composeField{fieldCc, fieldBcc} { v, _ := recipientsTestView(t)