net.smtp: public mail attachment

[7underlines]

This should fix adding mail attachments if not inside v module.

See also #20640

[huly-for-github]

Connected to Huly®: V_0.6-21907

[JalonSolov]

Rather than making these fields public, I think I would rather have seen a method added - parameters could be the attachment itself, and a optional type (if you wanted to send it as something other than plain text/binary - which would be figured out in the method if no specific type was given).

Otherwise... what the heck does cid even mean in that struct? Is it something everyone should know, and know what to put in there?

A method could set it correctly, without everyone having to know.

[spytheman]

What would the API look like, if you want to send say 2 attachments?
With a plain struct with public fields, that is obvious - just pass an array of those, as the example in #20640 does.

[spytheman]

The cid is the optional Content-ID and https://datatracker.ietf.org/doc/html/rfc2392 refers to it as cid too, so imho it should not be too surprising for people that want to use it.

[JalonSolov]

One or more calls to attachment() method. It could take an array, if all the attachments were the same type, or you may want to do multiple calls if you had attachments with different types.

[spytheman]

One or more calls to attachment() method.

Can you please show the example from the issue, but written as you expect it to be clearer?

[spytheman]

The relevant part is this:

send_cfg := smtp.Mail{
		from: '${nicename}<${sender}>'
		to: receivers
		subject: subject
		body_type: smtp.BodyType.html
		body: body
		attachments: [
			smtp.Attachment {
				filename: 'readme.txt'
				bytes: '...Some file content...'.bytes()
			}
		]
	}

[JalonSolov]

Instead of what is has now, I'd prefer something like

mut send_cfg := smtp.Mail{
		from: '${nicename}<${sender}>'
		to: receivers
		subject: subject
		body_type: smtp.BodyType.html
		body: body
	}
send_cfg.attachment(name: 'readme.txt', contents: '...Some file content...'.bytes())

or perhaps

send_cfg.attachment(name: 'readme.txt', contents: '...Some file content...'.bytes(), type: 'application/json')

The type could be an enum instead, matching one of your listed types already defined in vlib, or whatever.

Why would I want to do this? Suppose you wanted to send that same contents, but as JSON instead of plain text? There's nothing in the current Attachment struct to support setting a type at all, right now (unless you want to figure out what cid to use, and they just look... fugly).

[spytheman]

The cid does not determine the type. It adds an id, that can be referenced in the body (if it contains say an html message with <img> tags).

The type of the attachment is currently hardcoded in the Attachment.to_string/1 method, and is Content-Type: application/octet-stream .

[spytheman]

I think that the current PR is fine, and a public method on smtp.Mail for adding attachments can be added, but is out of scope of this PR.

[spytheman]

@7underlines thank you. Good work.

[spytheman]

Why would I want to do this? Suppose you wanted to send that same contents, but as JSON instead of plain text? There's nothing in the current Attachment struct to support setting a type at all, right now (unless you want to figure out what cid to use, and they just look... fugly).

i.e. you want something that is not supported currently, and will not affect the need for those fields to be public.

[JalonSolov]

... except they wouldn't need to be public if the new method were added.

Oh, well.

[spytheman]

... except they wouldn't need to be public if the new method were added.

Oh, well.

They need to, to allow the more natural initialization, described in the issue.

[spytheman]

Note that previously that worked (in January last year), but V became stricter in time, and now checks the pub section modifier for the fields correctly -> it just makes possible again something, that was before.

[spytheman]

It could be argued, that we should have tests for those, that exercise that ability, so that it will not regress in the future, and we can also add the ability to customize the attachment type too (with a suitable default of Content-Type: application/octet-stream).