Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mrep: Set From header according to To/Cc header #173

Closed
wants to merge 2 commits into from
Closed

mrep: Set From header according to To/Cc header #173

wants to merge 2 commits into from

Conversation

nmeum
Copy link
Contributor

@nmeum nmeum commented Mar 30, 2020

I use different mail addresses for different purposes. When replying to
any email, mrep(1) currently uses the mail address configured as
Local-Mailbox for the From header. While generally desirable, I do not
want to reply to mails I received through my work address with my
private email address.

This commit introduces a change which uses the email address, the email
was sent to, (as specified by To/Cc) for the From header, as long as this
address was configured as an Alternate-Mailboxes. I just quickly hacked
this together as a proof of concept without much further considerations.
Seems to work though but not sure if it is generally desirable to enable
this behaviour by default, let me know what you think.

@leahneukirchen
Copy link
Owner

This is a good idea, but there are some cases that require more thought: if you are in Cc with address only, your from doesn't contain the real name e.g.

Probably an improvement over the status quo, however.

@nmeum
Copy link
Contributor Author

nmeum commented Mar 30, 2020

Good point, RFC 5322 addresses complicate this setup quite a bit. For example, my assumption was that Alternate-Mailboxes just contains the mail address but that's not necessarily the case, right? If so, it would probably make sense to modify grepmine in a way that it operates only on the mail address part and later map that address to the display name using a different function? Is there already any tool for extracting the display-name/mail-address part of an RFC 5322 address?

@leahneukirchen
Copy link
Owner

I think alternate-mailboxes should just contain the addresses.

maddr -a can print the address only, i guess adding maddr -d would be useful.

@Duncaen
Copy link
Collaborator

Duncaen commented Mar 30, 2020

magrep with -a would be a good instead of grep, but it only matches all headers or one header at a time.

$ magrep -ha "to:mail@duncano.de|administrator@duncano.de" 100
to: administrator@duncano.de
$ mhdr -h to 100
Duncan Overbruck <administrator@duncano.de>

Edit: nvm, this doesn't really solve the problem, but might be better than a plain grep if someone messes with comments in addresses etc.

@nmeum
Copy link
Contributor Author

nmeum commented Mar 30, 2020

I think alternate-mailboxes should just contain the addresses.

Good to know, but the Local-Mailbox should definitely contain a valid RFC 5322 address, or at least mine does. In general, it might makes sense to use this as is and improve it later on when tooling around RFC 5322 has improved? idk.

@leahneukirchen
Copy link
Owner

Perhaps we can use a different header in the configuration for this feature. Then map that by address to the given displayname.

@nmeum
Copy link
Contributor Author

nmeum commented Mar 30, 2020

Just to make sure I understood you correctly: You are proposing adding an additional header to mblaze-profile(5) which contains a display name which is then combined with the address extracted from the To/Cc header and used as the From header in the reply?

@leahneukirchen
Copy link
Owner

Exactly.

@leahneukirchen
Copy link
Owner

I added maddr -d.

@leahneukirchen
Copy link
Owner

not sure we need that actually, but it's an obvious feature.

@nmeum
Copy link
Contributor Author

nmeum commented Mar 30, 2020

I added maddr -d.

Pushed a change which utilities maddr -d to extract the description part form the local-mailbox and combines that with the address extracted from the To/Cc/Bcc header. I haven't handled the case where the local-mailbox configuration header doesn't contain a description. However, this should address your original comment. I personally find that preferable to adding a new configuration header, let me know what you think.

not sure we need that actually, but it's an obvious feature.

I personally find it very handy, but if you don't want it that's fine as well.

mcom Outdated Show resolved Hide resolved
@nmeum
Copy link
Contributor Author

nmeum commented Mar 30, 2020

The more compilated version which also handles a missing description in local-mailbox would be:

addr=$(maddr -a -h to:cc:bcc: "$1" | grepmine | head -n 1)
if [ -n "$addr" ]; then
	desc=$(maddr -d -h "local-mailbox" "$MBLAZE/profile")
	if [ -n "$desc" ]; then
		from=$(printf "%s <%s>" "$desc" "$addr")
	else
		from=$addr
	fi
else
	from=$(mhdr -h local-mailbox "$MBLAZE/profile")
fi

@leahneukirchen
Copy link
Owner

Well, I have some private addresses that are listed as Alternative-Mailboxes to get the > sign, but that I don't use for replying. Also, people could use From: addresses that are not in Alternative-Mailboxes (e.g. a shared helpdesk email).

@nmeum
Copy link
Contributor Author

nmeum commented Mar 30, 2020

Well, I have some private addresses that are listed as Alternative-Mailboxes to get the sign, but that I don't use for replying.

It would certainly be possible to add a new auto-reply-addresses profile header setting (or something along the lines).

Also, people could use From: addresses that are not in Alternative-Mailboxes (e.g. a shared helpdesk email).

Why would that be a problem? Unless I am missing something here the code should fallback to local-mailbox then.

If you generally disagree with the feature I will just apply my patch locally. If you like the idea in general but don't like the implementation I can try to adjust it. Either way is fine with me.

@leahneukirchen
Copy link
Owner

I'm thinking of a slightly more general feature, something like

replyfrom() {
        addrs="$(maddr -a -h reply-from "$MBLAZE/profile")"
        grep -Fv -e "addrs"
}
addr=$(maddr -a -h to:cc:bcc: "$1" | replyfrom | head -n 1)
if [ -n "$addr" ]; then
	from=$(maddr -h reply-from "$MBLAZE/profile" | grep -xi -e "<$addr>" | head - n1)
fi
if [ -z "$from" ]; then
	from=$(mhdr -h local-mailbox "$MBLAZE/profile")
fi

@leahneukirchen
Copy link
Owner

We could default addrs to alternative-mailboxes if reply-from is empty (having full names there should also be ok).

@nmeum
Copy link
Contributor Author

nmeum commented Mar 30, 2020

I'm thinking of a slightly more general feature, […]

Sure, that would work for me 👍

@nmeum
Copy link
Contributor Author

nmeum commented Apr 20, 2020

Implemented (i.e. copied) your implementation. Also added some documentation to mblaze-profile(5). The downside of this implementation is that you end of with a lot of duplicate address in mblaze-profile(5) at least for my simple use-case.

@leahneukirchen
Copy link
Owner

We could default to Alternative-Mailboxes when Reply-From is unset.

Sorry, I lost track of this PR in my todo list.

mcom Outdated Show resolved Hide resolved
If any of the address contained in the aforementioned headers matches an
address configured in the Reply-From header in mblaze-profile(5).

Without this change, mrep(1) uses the mail address configured as
Local-Mailbox for the From header. While generally desirable, I use
different mail addresses for different proposes. As such, I do not
want to reply to mails I received through my work address with my
private email address.
@nmeum
Copy link
Contributor Author

nmeum commented Apr 20, 2020

We could default to Alternative-Mailboxes when Reply-From is unset.

Sure but imho that would complicate the code a bit (more case distinctions) as the fallback is needed both here:

mblaze/mcom

Lines 19 to 20 in 4e4dcdf

addrs="$(maddr -a -h reply-from: "$MBLAZE/profile")"
grep -F -e "$addrs"

and here:

mblaze/mcom

Line 367 in 4e4dcdf

from=$(maddr -h reply-from "$MBLAZE/profile" | grep -Fi "<$addr>" | head -n1)

Sorry, I lost track of this PR in my todo list.

So did I.

@nmeum
Copy link
Contributor Author

nmeum commented Apr 20, 2020

Added some experimental fallback code.

@xelxebar
Copy link

xelxebar commented May 20, 2020

Have my eye on this feature. The list of emails I have to reply as is very long and constantly growing. Would it make sense to allow wildcards or regex specifications in reply-from and/or alternative-mailboxes specifications?

Unfortunately * and / can technically appear in the local and domain parts, so we can't use an obvious wildcard syntax like *@example.com nor a regex syntax like /.*@example.com/. Unwinding the address specification in rfc5322 is a bit of a windy path, but it looks like we could get away with using \ of the specials (section 3.2.3). AFAICT the backslash can't appear anywhere in an address specification so a regex like \.*@example.com\ should be easily distinguishable and unambiguous.

Thoughts?

@leahneukirchen
Copy link
Owner

Do we really need something more complicated than @example.com?

@leahneukirchen
Copy link
Owner

I mangled this into 282de65, please test.

@xelxebar
Copy link

xelxebar commented Jun 4, 2020

Do we really need something more complicated than @example.com?

Beautiful.

I mangled this into 282de65, please test.

Thank you. Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants