Issue #25: mixed + alternative not supported

[simongreen-net]

No description provided.

[simongreen-net]

Fixes the issues with pull request #40, along with some more tests.

[pali]

Does not look like a good change as it completely remove method parts. This breaks backward API compatibility.

[pali]

E.g. you can fix it by adding re-implementing method parts:

sub parts {
  my $self = shift;
  return ($self->text_parts(@_), $self->attach_parts(@_));
}

[simongreen-net]

Thanks @pali for your comment. I have uploaded a new PR that re-adds the parts subroutine.

[pali]

I was thinking about this change and there is small terminology problem... In email is every part either attachment or inline. In Email::Stuffer documentation there is text body, html body and attachment.

In your patch you use text part for Email::Stuffer text body and html body and attach part for Email::Stuffer attachment.

Moreover, if somebody want to create email inline part, then it is possible via Email::Stuffer attachment attribute disposition => "inline".

Also email attachment can be plain text, but in your patch it would not be in text part.

Therefore I would suggest to not use text part in your patch, but rather invent better name (also for method text_part. Maybe body part as those covers only text_body and html_body?

[simongreen-net]

If an attachment (specifically something in attach_part) is of text/* and it's disposition is inline, then most e-mail clients (at least Thunderbird) will show the attachment inline. This is the correct behaviour because since the first text part might be a multipart/alternate, i.e. both text and html, where only one should be shown, while the text attachment is a text/plain or text/html.

Therefore I would suggest to not use text part in your patch, but rather invent better name (also for method text_part. Maybe body part as those covers only text_body and html_body?

I don't support this, since I think my patch already handles the situation where an attachment is of the type text/* as it should.

[pali]

If an attachment (specifically something in attach_part) is of text/* and it's disposition is inline, then most e-mail clients (at least Thunderbird) will show the attachment inline. This is the correct behaviour because since the first text part might be a multipart/alternate, i.e. both text and html, where only one should be shown, while the text attachment is a text/plain or text/html.

Yes it is correct behavior and does not contradict what I wrote.

Therefore I would suggest to not use text part in your patch, but rather invent better name (also for method text_part. Maybe body part as those covers only text_body and html_body?

I don't support this, since I think my patch already handles the situation where an attachment is of the type text/* as it should.

Note, your patch does not handle it. In sub attach you unconditionally do:
push @{$self->{attach_parts}}, Email::MIME->create(...)
even when passed part is marked as inline via disposition => "inline" user attribute.

But as I wrote Email::Stuffer already treat everything but text_body/html_body as attachment independently of disposition, even it is inline. Because of this fact and current Email::Stuffer terminology I suggested to use name body_parts instead of text_parts without changing code. As opposite of Email::Stuffer's attach is Email::Stuffer's body.