Add optional multipart/mixed digest format.

[squeakbat]

The previous format (multipart/digest), was incompatible with many
popular e-mail readers (such as gmail), which made it essentially
unusable.

The new format is multipart/mixed, with each part consisting
of a single non-digest message (of type text/plain or text/html).
One possible incorrectness is that the parts still retain message
headers (subject, date, etc.) that are not really used by normal text
attachments.

This is an incompatible change. It looks different, and may break
existing digest-post-proc functions.

[squeakbat]

Would this be more palatable as an option and/or as a straight one-part message?

The problem with a default-off option is that digest will still be practically useless by default. The problem with one-part message format is it'll be even less compatible with existing postproc filters.

[amiryal]

I like the idea of a utility boost, but I hesitate to accept backwards-incompatible changes without a mandate. This leads me to think that this project needs to prepare for a major version update, where changes such as this and others like #216 can be accepted without hesitance.

Another way to go about it is to develop a digest-post-process hook that converts the given multipart/digest into a multipart/mixed (while somehow satisfying the code in _send_digest that extracts the date). To make it usable and discoverable, it may be a good idea to add a section in the manual about it (and if feeling generous, also briefly describe all the other hooks from the rss2email.post_process package in the same section). Interestingly, this idea can go both ways, i.e., should we choose to change the default to multipart/mixed, then including such a hook to convert back to the old multipart/digest would be handy, for those who still depend on it.

[squeakbat]

I posted such a digest-post-process in the discussion: #194 (comment). It preserves the nesting level with an additional multipart/mixed wrapping. Though it seems wrong, as a permanent solution, to force the user to resort to a postprocessor (even a well-documented one).

I'm willing to make the built-in feature optional and default off. (Mostly, I can't come up with a good name for that config option. "multipart-mixed-digest" sounds so lame.) This seems to be a good compromise. There is no incompatibility, because if a user sets that option, then they are aware that any digest-post-process filter they write has to handle the multipart/mixed format.

As a separate issue, is single-part digest better then multipart/mixed? It's cleaner, but lacks the structure that can help guide postproc filters.

[amiryal]

Yesterday I read a report about an interview with Linus Torvalds. The following quote reminded me of this discussion:

Torvalds is not just a developer, he is also a user, and he thinks that the most annoying problem that users experience is "doing a software upgrade and things stop working". He cannot change all of the other software projects out there, which generally have different policies, but he has been "very hard-nosed" with kernel policy to try to ensure that programs continue to run after a upgrade.

I completely agree with the sentiment, that your old setup should just work, following an upgrade.

There is no incompatibility, because if a user sets that option, then they are aware that any digest-post-process filter they write has to handle the multipart/mixed format.

True, it may be fine from the user’s perspective, but think about it from a software maintainability standpoint. I do not want if…else… statements to be sprinkled all over.

One way to work around the naming issue is to leave the option name as digest, but make it a string instead of boolean, and restrict it to the following options:

• multipart/digest
• multipart/mixed → the new implementation
• any value that previously evaluated to True → alias for multipart/digest
• any value that previously evaluated to False → the default, not a digest

As a separate issue, is single-part digest better then multipart/mixed? It's cleaner, but lacks the structure that can help guide postproc filters.

I don’t think any format is better than the other. So long as we keep a multipart default, users may postprocess further to their liking. A single-part digest is yet another example filter that we can provide and document.

[squeakbat]

I do agree with the sentiment. I wouldn't have suggested breaking compatibility except that I don't think anybody is really using digests.

I also like keeping the digest option and giving it new values. I don't think it keeps code paths simpler (however many kinds of output we support is how complicated it is), but it does heads off further proliferation of options.

It's not all that straightforward to flatten a multipart/mixed message after the fact, because each part is a full html document, but it can be done.

[squeakbat]

Oh yeah, anyway, I'll give it a go.

[squeakbat]

OK. After looking at the code and thinking about it, I think it much better to have a separate config option "digest-type" that defaults "multipart/digest". It's conceptually unclean to mix boolean and string values (even as a compatibility kludge) and the code really doesn't like to do it.

[squeakbat]

OK sorry I made the mistake of pulling from before making this change. So, if it's better I can trash this one, make a clean branch, and submit a new pull request with a clean, single commit.

[amiryal]

I made the mistake of pulling from before making this change. So, if it's better I can trash this one, make a clean branch, and submit a new pull request with a clean, single commit.

For a more complex change, that requires multiple commits to provide context for each stage of refactoring, I would have demanded a clean rebase. For this one, I would simply use the Squash and merge button, which is effectively going to create that single commit for you (us) automatically.

[squeakbat]

Thanks!

The commit message will be pretty ugly and hard to follow, with "change digest format..." followed by "revert ...", with merges from master in the middle. But I guess I'm okay with that.

There isn't a way for me to do the squash and edit the commit message, is there?

[squeakbat]

Oh yeah, I noticed I have a typo in a comment, two slashes: multipart//mixed.

[amiryal]

I fixed the typo, removed a pair of redundant parentheses, and added an entry to the CHANGELOG.

diff --git a/CHANGELOG b/CHANGELOG
index 2dbcf78..c245546 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,5 @@
 UNRELEASED
+    * New `digest-type` configuration adds optional more widely supported `multipart/mixed` format
     * New argument `--only-new` on the `add` command to ignore entries in feed
       when added, so only new entries will be sent.
     * Fix crash on html-mail entries with no URL
diff --git a/rss2email/feed.py b/rss2email/feed.py
index b4d62d4..df21d85 100644
--- a/rss2email/feed.py
+++ b/rss2email/feed.py
@@ -1005,12 +1005,12 @@ class Feed (object):
         return digest
 
     def _append_to_digest(self, digest, message):
-        if (self.digest_type == 'multipart/digest'):
+        if self.digest_type == 'multipart/digest':
             part = _MIMEMessage(message)
             part.add_header('Content-Disposition', 'attachment')
             digest.attach(part)
         else:
-            # multipart//mixed
+            # multipart/mixed
             digest.attach(message)
 
     def _send_digest(self, digest, sender):

[squeakbat]

Thank you!

Yeah, I tried to write that conditional in C there.

[amiryal]

Thanks, @squeakbat!

Amir (usually yxejamir in #rss2email on libera.chat)

[squeakbat]

Thank you all for the help!