Skip to content
This repository was archived by the owner on Sep 10, 2025. It is now read-only.

Allow the email template to be replaced#25

Merged
skx merged 9 commits intomasterfrom
template-override
Aug 7, 2020
Merged

Allow the email template to be replaced#25
skx merged 9 commits intomasterfrom
template-override

Conversation

@skx
Copy link
Owner

@skx skx commented Aug 6, 2020

This pull-request makes it possible for users to override the
default template which is used to generate the email.

This replaces #17, and closes #16 by making it possible for
users to make that choice for themselves.

skx added 7 commits August 6, 2020 20:16
This pull-request makes it possible for users to override the
default template which is used to generate the email.

This replaces #17, and closes #16 by making it possible for
users to make that choice for themselves.
I'd forgotten golang templates supported comments directly, and
so this didn't require any special hacking.
@DaleFarnsworth
Copy link

DaleFarnsworth commented Aug 7, 2020

This branch meets my needs. Thank you.

Five comments.

  1. If you terminate the template comment with " -}}", the whitespace following it is stripped, so you can start the template text on the next line.

  2. With a configurable template, we don't really need fromAddr anymore.

  3. I have no interest in the value of {{now}}. I am interested in the item published date. I'd accept the string produced by i.PublishedParsed.Format(time.RFC822Z), though I'd really just like to see the date without the time, like i.PublishedParse.Format("02 Jan 2006"). Or, for maximum flexibility, pass i.PublishedParsed along with a function to do time.Format. It would have been nice to use i.Published, but the format varies with the feed, and some formats are not very user friendly.

  4. You might consider passing i (withstate.FeedItem) to SendMail() and move the assignment of content and text to within SendMail().

  5. It would be good to move the call to i.RecordSeen() after the attempt to send email, so that if the attempt fails, we could retry sending the email later.

@DaleFarnsworth
Copy link

On further reflection, having the item published date is of such limited value that I think it's not worth doing. I'm sorry for bringing it up in the first place. I still think we can remove {{now}}. :-)

@DaleFarnsworth
Copy link

It would be nice to name the template "email.tmpl", as in template.New("email.tmpl") so that the error message contains the filename on parse errors.

skx added 2 commits August 7, 2020 08:46
In brief:

* Pass `RSSItem` and `RSSFeed` to the template.
* Use a better name for the template.
* Reformatted some comments.
* Drop the `now` helper.
@skx
Copy link
Owner Author

skx commented Aug 7, 2020

  • With a configurable template, we don't really need fromAddr anymore.

Yes, removed on that basis.

  • I have no interest in the value of {{now}}. I am interested in the item published date. I'd accept the string produced by..

I was using the shortcut that cron runs every N minutes, so the current time is a reasonable proxy for the actual publication date. Still it was a bit rough and ready. I've updated the template to explicitly pass RSSFeed and RSSItem so that access to fields can be done in the template. Annoyingly the test-feeds i'm using have nothing useful there. Hrm.

  • You might consider passing i (withstate.FeedItem) to SendMail() and move the assignment of content and text to within SendMail().

Yeah that's an area that needs some thought; the email sending is currently mixing too many concerns and should be cleaned up. I'll postpone that for the moment though.

  • It would be good to move the call to i.RecordSeen() after the attempt to send email, so that if the attempt fails, we could retry sending the email later.

Good suggestion. Done.

@skx skx merged commit 022b060 into master Aug 7, 2020
@skx skx deleted the template-override branch August 7, 2020 06:07
@DaleFarnsworth
Copy link

I still like this change. It's purely cosmetic though.

diff --git a/data/email.tmpl b/data/email.tmpl
index c404428..9cbf0ae 100644
--- a/data/email.tmpl
+++ b/data/email.tmpl
@@ -21,7 +21,9 @@
 
      This comment will be stripped from the generated email.
 
-  */}}Content-Type: multipart/mixed; boundary=21ee3da964c7bf70def62adb9ee1a061747003c026e363e47231258c48f1
+  */ -}}
+
+Content-Type: multipart/mixed; boundary=21ee3da964c7bf70def62adb9ee1a061747003c026e363e47231258c48f1
 From: {{.From}}
 To: {{.To}}
 Subject: [rss2email] {{.Subject}}

I won't mention it again though. :-)

@DaleFarnsworth
Copy link

Thank you for being so responsive to my requests/comments. rss2email has all the features I can think of. My feed emails look exactly the way I want them without any local patches. Thanks again!

@skx
Copy link
Owner Author

skx commented Aug 7, 2020

Sorry somehow I spaced out reading your feedback and missed that bit. Fixed now.

And no problem, it's been a fun few days. I'll take a new release shortly, and then leave the project to stall for a few more months!

@DaleFarnsworth
Copy link

Sounds like a plan!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Put feed.Title into the From: address?

2 participants