feat: add HTML body support for Gmail send/drafts#16
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds HTML body support to Gmail send and draft creation commands, allowing users to send emails with HTML formatting while maintaining backward compatibility with plain-text-only emails.
Key Changes:
- Added
--body-htmlflag togmail sendandgmail drafts createcommands - Implemented multipart/alternative MIME structure when both plain and HTML bodies are provided
- Enhanced base64URL decoding to handle whitespace and padding variations in attachment data
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/cmd/gmail_send.go | Added bodyHTML flag and updated validation to require either --body or --body-html |
| internal/cmd/gmail_drafts.go | Added bodyHTML flag with same validation logic as send command |
| internal/cmd/gmail_mime.go | Implemented multipart/alternative support, added Message-ID generation, CRLF normalization, and helper functions for text parts |
| internal/cmd/gmail_mime_test.go | Added comprehensive tests for HTML-only, plain+HTML, and attachment scenarios including UTF-8 subject encoding |
| internal/cmd/gmail_thread.go | Enhanced base64URL decoding with whitespace stripping and padding support |
| internal/cmd/gmail_thread_test.go | Added tests for improved base64URL decoding with padding and whitespace handling |
| docs/spec.md | Updated command documentation to reflect optional body flags |
| README.md | Updated example to demonstrate HTML body usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func randomMessageID(from string) (string, error) { | ||
| domain := "gogcli.local" | ||
| if addr, err := mail.ParseAddress(strings.TrimSpace(from)); err == nil && addr != nil { | ||
| if at := strings.LastIndex(addr.Address, "@"); at != -1 && at+1 < len(addr.Address) { | ||
| domain = strings.TrimSpace(addr.Address[at+1:]) | ||
| } | ||
| } else if at := strings.LastIndex(from, "@"); at != -1 && at+1 < len(from) { | ||
| domain = strings.TrimSpace(from[at+1:]) | ||
| domain = strings.Trim(domain, " >") | ||
| } | ||
|
|
||
| var b [16]byte | ||
| if _, err := rand.Read(b[:]); err != nil { | ||
| return "", err | ||
| } | ||
| local := base64.RawURLEncoding.EncodeToString(b[:]) | ||
| return fmt.Sprintf("<%s@%s>", local, domain), nil | ||
| } |
There was a problem hiding this comment.
The randomMessageID function lacks direct test coverage. Consider adding a test to verify:
- Message-ID format is correct (enclosed in angle brackets)
- Domain extraction works correctly for various email formats (simple email, email with display name, malformed emails)
- The fallback to "gogcli.local" works when domain extraction fails
- Generated Message-IDs are unique and contain valid base64url-encoded local parts
While the function is indirectly tested through buildRFC822 tests, explicit unit tests would improve maintainability and make the domain extraction logic more transparent.
| func hasHeader(headers map[string]string, name string) bool { | ||
| for k := range headers { | ||
| if strings.EqualFold(k, name) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
The hasHeader function lacks direct test coverage. Consider adding a test to verify case-insensitive header matching works correctly for various scenarios:
- Exact case match
- Different case variations (e.g., "Message-ID", "message-id", "MESSAGE-ID")
- Non-existent headers
- Empty header maps
This would ensure the case-insensitive comparison works as expected and prevent regressions.
| func normalizeCRLF(s string) string { | ||
| // Normalize to CRLF for RFC 5322 / MIME messages. | ||
| s = strings.ReplaceAll(s, "\r\n", "\n") | ||
| s = strings.ReplaceAll(s, "\r", "\n") | ||
| return strings.ReplaceAll(s, "\n", "\r\n") | ||
| } |
There was a problem hiding this comment.
The normalizeCRLF function lacks direct test coverage. Consider adding a dedicated test case to verify the normalization logic handles edge cases correctly, such as:
- Mixed line endings (combinations of \r\n, \r, and \n)
- Empty strings
- Strings with only whitespace
- Strings already in CRLF format
While this function is indirectly tested through the buildRFC822 tests, explicit unit tests would make the behavior clearer and catch potential regressions more easily.
steipete
force-pushed
the
feat/gmail-html-body
branch
from
9ebd74e to
c8ec61e
Compare
Summary\n- add --body-html flag to gmail send and drafts create\n- build multipart/alternative when both plain and HTML provided; allow HTML-only as text/html\n- include base64url attachment fix (stacked on PR #15)\n- update docs and MIME unit tests\n\n## Testing\n- go test ./...\n- manual Gmail sends: plain-only, HTML-only, plain+HTML, attachment+HTML, draft create/send, UTF-8 subject (verified message parts via gog gmail thread)