feat: display replies in a blockquote #112
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds functionality to display email replies in styled blockquotes within the terminal email client. The feature processes both HTML blockquote elements and plain text quoted replies (lines starting with ">") and renders them in bordered boxes with optional header information showing the sender and date.
Changes:
- Added blockquote HTML element processing with placeholder-based rendering
- Added plain text quote detection and styling for "> " prefixed lines
- Added date parsing and formatting utilities to display timestamps in a consistent format
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if from != "" || date != "" { | ||
| if from != "" && date != "" { | ||
| header = quoteHeaderStyle.Render(from + " " + date) | ||
| } else if from != "" { | ||
| header = quoteHeaderStyle.Render(from) | ||
| } else { | ||
| header = quoteHeaderStyle.Render(date) | ||
| } |
There was a problem hiding this comment.
The nested conditional logic for building the header can be simplified. The outer condition if from != "" || date != "" is redundant because if both are empty, the inner conditions would result in an empty header anyway. Consider simplifying this logic for better readability.
| if from != "" || date != "" { | |
| if from != "" && date != "" { | |
| header = quoteHeaderStyle.Render(from + " " + date) | |
| } else if from != "" { | |
| header = quoteHeaderStyle.Render(from) | |
| } else { | |
| header = quoteHeaderStyle.Render(date) | |
| } | |
| if from != "" && date != "" { | |
| header = quoteHeaderStyle.Render(from + " " + date) | |
| } else if from != "" { | |
| header = quoteHeaderStyle.Render(from) | |
| } else if date != "" { | |
| header = quoteHeaderStyle.Render(date) |
| // Common date formats to try | ||
| formats := []string{ | ||
| "Jan 2, 2006 at 3:04 PM", | ||
| "02:01:06 15:04", |
There was a problem hiding this comment.
The input format "02:01:06 15:04" on line 743 is ambiguous and likely incorrect. This format would parse day:month:year-2000 hour:minute, but the year component "06" represents the last two digits of the year (2006 in Go's reference time). Consider using a clearer format like "02:01:2006 15:04" or removing this format if it's not a valid input pattern you expect to receive.
| text = expandImageRowPlaceholders(text) | ||
|
|
||
| // Replace quote placeholders with styled quote boxes | ||
| quoteRegex := regexp.MustCompile(`\[\[MATCHA_QUOTE:(\d+)\]\]`) |
There was a problem hiding this comment.
The regex is compiled on every call to the replace function. This is inefficient for performance. Consider compiling the regex once at the package level or function level and reusing it.
| // parseDateForDisplay converts various date formats to DD:MM:YY HH:MM | ||
| func parseDateForDisplay(dateStr string) string { | ||
| // Common date formats to try | ||
| formats := []string{ | ||
| "Jan 2, 2006 at 3:04 PM", | ||
| "02:01:06 15:04", | ||
| "2006-01-02 15:04:05", | ||
| "Mon, 02 Jan 2006 15:04:05 -0700", | ||
| "Mon, 2 Jan 2006 15:04:05 -0700", | ||
| "2 Jan 2006 15:04:05", | ||
| "January 2, 2006 at 3:04 PM", | ||
| "Jan 2, 2006 3:04 PM", | ||
| time.RFC1123Z, | ||
| time.RFC1123, | ||
| time.RFC822Z, | ||
| time.RFC822, | ||
| } | ||
|
|
||
| for _, format := range formats { | ||
| if t, err := time.Parse(format, dateStr); err == nil { | ||
| return t.Format("02:01:06 15:04") | ||
| } | ||
| } | ||
|
|
||
| // Return original if parsing fails | ||
| return dateStr | ||
| } |
There was a problem hiding this comment.
The parseDateForDisplay function lacks test coverage. Given that the test file contains comprehensive tests for other functions, tests should be added to verify that various date formats are correctly parsed and formatted, and that invalid dates are handled gracefully by returning the original string.
| // renderQuoteBox renders a quoted section in a styled box | ||
| func renderQuoteBox(from, date string, lines []string) string { | ||
| // Build header with email on left and date on right | ||
| var header string | ||
| if from != "" || date != "" { | ||
| if from != "" && date != "" { | ||
| header = quoteHeaderStyle.Render(from + " " + date) | ||
| } else if from != "" { | ||
| header = quoteHeaderStyle.Render(from) | ||
| } else { | ||
| header = quoteHeaderStyle.Render(date) | ||
| } | ||
| } | ||
|
|
||
| // Join the quoted content | ||
| content := strings.Join(lines, "\n") | ||
|
|
||
| // Build the box content | ||
| var boxContent string | ||
| if header != "" { | ||
| boxContent = header + "\n\n" + content | ||
| } else { | ||
| boxContent = content | ||
| } | ||
|
|
||
| return quoteBoxStyle.Render(boxContent) | ||
| } |
There was a problem hiding this comment.
The renderQuoteBox function lacks test coverage. Given that the test file contains comprehensive tests for other functions, tests should be added to verify the quote box rendering with various combinations of from/date parameters (both present, only one present, both absent) and different content structures.
| // Handle blockquote elements (quoted replies) | ||
| // We collect quote data and use placeholders, then render after doc.Text() | ||
| var quoteData []struct { | ||
| from, date string | ||
| content string | ||
| } | ||
| doc.Find("blockquote").Each(func(i int, s *goquery.Selection) { | ||
| // Try to extract sender info from cite attribute or preceding text | ||
| cite, _ := s.Attr("cite") | ||
| quoteText := strings.TrimSpace(s.Text()) | ||
|
|
||
| // Look for "On DATE, EMAIL wrote:" pattern in previous sibling or cite | ||
| var from, date string | ||
| prevText := "" | ||
| if prev := s.Prev(); prev.Length() > 0 { | ||
| prevText = strings.TrimSpace(prev.Text()) | ||
| } | ||
|
|
||
| onWroteRegex := regexp.MustCompile(`On\s+(.+?),\s+(.+?)\s+wrote:`) | ||
| if matches := onWroteRegex.FindStringSubmatch(prevText); matches != nil { | ||
| date = parseDateForDisplay(matches[1]) | ||
| from = matches[2] | ||
| // Remove the "On ... wrote:" from the previous element | ||
| s.Prev().Remove() | ||
| } else if matches := onWroteRegex.FindStringSubmatch(cite); matches != nil { | ||
| date = parseDateForDisplay(matches[1]) | ||
| from = matches[2] | ||
| } | ||
|
|
||
| // Store quote data and use placeholder | ||
| quoteData = append(quoteData, struct { | ||
| from, date string | ||
| content string | ||
| }{from, date, quoteText}) | ||
| placeholder := fmt.Sprintf("\n[[MATCHA_QUOTE:%d]]\n", len(quoteData)-1) | ||
| s.ReplaceWithHtml(placeholder) | ||
| }) |
There was a problem hiding this comment.
The new blockquote handling logic lacks test coverage. Given that the test file contains comprehensive tests for other functions like decodeQuotedPrintable and markdownToHTML, tests should be added to verify the blockquote processing behavior, including edge cases like missing cite attributes, malformed date patterns, and nested blockquotes.
| func styleQuotedReplies(text string) string { | ||
| lines := strings.Split(text, "\n") | ||
| var result []string | ||
| var quoteBlock []string | ||
| var quoteFrom, quoteDate string | ||
| inQuote := false | ||
|
|
||
| // Regex to match "On DATE, EMAIL wrote:" pattern | ||
| // Matches various date formats | ||
| onWroteRegex := regexp.MustCompile(`^On\s+(.+?),\s+(.+?)\s+wrote:$`) | ||
|
|
||
| for i := 0; i < len(lines); i++ { | ||
| line := lines[i] | ||
| trimmedLine := strings.TrimSpace(line) | ||
|
|
||
| // Check for "On DATE, EMAIL wrote:" header | ||
| if matches := onWroteRegex.FindStringSubmatch(trimmedLine); matches != nil { | ||
| // If we were already in a quote block, render it first | ||
| if inQuote && len(quoteBlock) > 0 { | ||
| result = append(result, renderQuoteBox(quoteFrom, quoteDate, quoteBlock)) | ||
| quoteBlock = nil | ||
| } | ||
|
|
||
| // Parse the date and email from the match | ||
| dateStr := matches[1] | ||
| quoteFrom = matches[2] | ||
| quoteDate = parseDateForDisplay(dateStr) | ||
| inQuote = true | ||
| continue | ||
| } | ||
|
|
||
| // Check if line starts with ">" (quoted text) | ||
| if strings.HasPrefix(trimmedLine, ">") { | ||
| if !inQuote { | ||
| // Start a new quote block without header info | ||
| inQuote = true | ||
| quoteFrom = "" | ||
| quoteDate = "" | ||
| } | ||
| // Remove the leading "> " and add to quote block | ||
| quotedContent := strings.TrimPrefix(trimmedLine, ">") | ||
| quotedContent = strings.TrimPrefix(quotedContent, " ") | ||
| quoteBlock = append(quoteBlock, quotedContent) | ||
| } else if inQuote { | ||
| // End of quote block - check if it's just whitespace | ||
| if trimmedLine == "" && i+1 < len(lines) && strings.HasPrefix(strings.TrimSpace(lines[i+1]), ">") { | ||
| // Empty line within quote block, keep it | ||
| quoteBlock = append(quoteBlock, "") | ||
| } else if trimmedLine == "" && len(quoteBlock) == 0 { | ||
| // Empty line before any quoted content, skip | ||
| continue | ||
| } else { | ||
| // End of quote block | ||
| if len(quoteBlock) > 0 { | ||
| result = append(result, renderQuoteBox(quoteFrom, quoteDate, quoteBlock)) | ||
| quoteBlock = nil | ||
| } | ||
| inQuote = false | ||
| quoteFrom = "" | ||
| quoteDate = "" | ||
| result = append(result, line) | ||
| } | ||
| } else { | ||
| result = append(result, line) | ||
| } | ||
| } | ||
|
|
||
| // Handle any remaining quote block | ||
| if inQuote && len(quoteBlock) > 0 { | ||
| result = append(result, renderQuoteBox(quoteFrom, quoteDate, quoteBlock)) | ||
| } | ||
|
|
||
| return strings.Join(result, "\n") | ||
| } |
There was a problem hiding this comment.
The styleQuotedReplies function lacks test coverage. Given that the test file contains comprehensive tests for other functions, tests should be added to verify the plain text quote styling behavior, including various edge cases like quotes without headers, nested quotes, empty lines within quotes, and mixed quoted/unquoted content.
|
|
||
| // Regex to match "On DATE, EMAIL wrote:" pattern | ||
| // Matches various date formats | ||
| onWroteRegex := regexp.MustCompile(`^On\s+(.+?),\s+(.+?)\s+wrote:$`) |
There was a problem hiding this comment.
The same regex pattern On\s+(.+?),\s+(.+?)\s+wrote: is defined in two places (line 547 and line 672). This duplicates logic and creates a maintenance burden. Consider extracting this to a package-level variable that can be shared between both functions.
| // End of quote block - check if it's just whitespace | ||
| if trimmedLine == "" && i+1 < len(lines) && strings.HasPrefix(strings.TrimSpace(lines[i+1]), ">") { | ||
| // Empty line within quote block, keep it | ||
| quoteBlock = append(quoteBlock, "") | ||
| } else if trimmedLine == "" && len(quoteBlock) == 0 { | ||
| // Empty line before any quoted content, skip | ||
| continue | ||
| } else { | ||
| // End of quote block | ||
| if len(quoteBlock) > 0 { | ||
| result = append(result, renderQuoteBox(quoteFrom, quoteDate, quoteBlock)) | ||
| quoteBlock = nil | ||
| } | ||
| inQuote = false | ||
| quoteFrom = "" | ||
| quoteDate = "" | ||
| result = append(result, line) | ||
| } |
There was a problem hiding this comment.
The complex conditional logic on line 708 with multiple nested conditions (lines 708-724) makes the function harder to understand and maintain. Consider extracting this logic into a helper function with a descriptive name like shouldKeepEmptyLineInQuote or refactoring to make the intent clearer.
| text = quoteRegex.ReplaceAllStringFunc(text, func(match string) string { | ||
| idxStr := quoteRegex.FindStringSubmatch(match)[1] | ||
| var idx int | ||
| fmt.Sscanf(idxStr, "%d", &idx) |
There was a problem hiding this comment.
The error return value from fmt.Sscanf is ignored. If the parsing fails, idx will remain 0, which could lead to incorrect behavior if the placeholder index is malformed. Consider handling the error or adding validation.
| fmt.Sscanf(idxStr, "%d", &idx) | |
| if n, err := fmt.Sscanf(idxStr, "%d", &idx); err != nil || n != 1 { | |
| // If parsing fails, leave the original placeholder unchanged. | |
| return match | |
| } |
1 participant
Quoted Reply Handling and Styling:
blockquoteelements, extracting sender (from), date, and quoted content, and inserting placeholders to be rendered later as styled quote boxes.lipgloss, displaying sender and date headers when available.>and "On DATE, EMAIL wrote:" patterns), grouping them into quote boxes with extracted metadata.Utility Functions and Styling:
parseDateForDisplay) and rendering quoted sections in styled boxes (renderQuoteBox), as well as styling definitions for quote boxes and headersTop: old, Bottom: new