~sircmpwn/aerc

msg/reply improvements v1 PROPOSED

Reto Brunner: 4
 base models.Address on the mail.Address type
 msg/reply: handle addresses as addresses
 Add account alias configuration and correctly set From field
 msg/reply: don't cc the sending address on reply all

 12 files changed, 121 insertions(+), 85 deletions(-)
> This simplifies the code considerably and makes it easier to follow
> ---
> commands/msg/reply.go | 48 +++++++++++++++++++++----------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/commands/msg/reply.go b/commands/msg/reply.go
> index 6fd6141..2ab12a7 100644
> --- a/commands/msg/reply.go
> +++ b/commands/msg/reply.go
> @@ -5,12 +5,12 @@ import (
> "errors"
> "fmt"
> "io"
> - gomail "net/mail"
> "strings"
> 
> "git.sr.ht/~sircmpwn/getopt"
> 
> "git.sr.ht/~sircmpwn/aerc/lib"
> + "git.sr.ht/~sircmpwn/aerc/lib/format"
> "git.sr.ht/~sircmpwn/aerc/models"
> "git.sr.ht/~sircmpwn/aerc/widgets"
> )
> @@ -60,7 +60,15 @@ func (reply) Execute(aerc *widgets.Aerc, args
> []string) error {
> return errors.New("No account selected")
> }
> conf := acct.AccountConfig()
> - us, _ := gomail.ParseAddress(conf.From)
> + var from *models.Address
> + fa, err := format.ParseAddressList(conf.From)
Why not use format.parseAddress? Can the from field ever be a list (if
so, you can ignore this comment)? You can skip the third else condition below then too.
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~sircmpwn/aerc/patches/12028/mbox | git am -3
Learn more about email & git
View this thread in the archives

[PATCH 1/4] base models.Address on the mail.Address type Export this patch

This allows us to hook into the std libs implementation of parsing related stuff.
For this, we need to get rid of the distinction between a mailbox and a host
to just a single "address" field.

However this is already the common case. All but one users immediately
concatenated the mbox/domain to a single address.

So this in effects makes it simpler for most cases and we simply do the
transformation in the special case.
---
 commands/msg/forward.go |  3 ++-
 lib/format/format.go    | 50 +++++++++++++++++++++++++++++++----------
 models/models.go        | 34 +++++++++-------------------
 widgets/msgviewer.go    | 15 +++++++------
 worker/imap/imap.go     |  3 +--
 worker/lib/parse.go     | 11 +--------
 worker/lib/sort.go      |  3 +--
 7 files changed, 61 insertions(+), 58 deletions(-)

diff --git a/commands/msg/forward.go b/commands/msg/forward.go
index 3ff0194..0c6b0e0 100644
--- a/commands/msg/forward.go
+++ b/commands/msg/forward.go
@@ -11,6 +11,7 @@ import (
	"strings"

	"git.sr.ht/~sircmpwn/aerc/lib"
	"git.sr.ht/~sircmpwn/aerc/lib/format"
	"git.sr.ht/~sircmpwn/aerc/models"
	"git.sr.ht/~sircmpwn/aerc/widgets"
	"git.sr.ht/~sircmpwn/aerc/worker/types"
@@ -77,7 +78,7 @@ func (forward) Execute(aerc *widgets.Aerc, args []string) error {

	addTab := func() (*widgets.Composer, error) {
		if template != "" {
			original.From = models.FormatAddresses(msg.Envelope.From)
			original.From = format.FormatAddresses(msg.Envelope.From)
			original.Date = msg.Envelope.Date
		}

diff --git a/lib/format/format.go b/lib/format/format.go
index cc46da8..8dc0f6c 100644
--- a/lib/format/format.go
+++ b/lib/format/format.go
@@ -2,13 +2,14 @@ package format

import (
	"errors"
	"fmt"
	"mime"
	gomail "net/mail"
	"strings"
	"time"
	"unicode"

	"git.sr.ht/~sircmpwn/aerc/models"
	"github.com/emersion/go-message"
)

func parseAddress(address string) *gomail.Address {
@@ -20,6 +21,30 @@ func parseAddress(address string) *gomail.Address {
	return addrs
}

func ParseAddressList(s string) ([]*models.Address, error) {
	parser := gomail.AddressParser{
		&mime.WordDecoder{message.CharsetReader},
	}
	list, err := parser.ParseList(s)
	if err != nil {
		return nil, err
	}

	addrs := make([]*models.Address, len(list))
	for i, a := range list {
		addrs[i] = (*models.Address)(a)
	}
	return addrs, nil
}

func FormatAddresses(l []*models.Address) string {
	formatted := make([]string, len(l))
	for i, a := range l {
		formatted[i] = a.Format()
	}
	return strings.Join(formatted, ", ")
}

func ParseMessageFormat(
	fromAddress string,
	format string, timestampformat string,
@@ -87,8 +112,7 @@ func ParseMessageFormat(
			}
			addr := msg.Envelope.From[0]
			retval = append(retval, 's')
			args = append(args,
				fmt.Sprintf("%s@%s", addr.Mailbox, addr.Host))
			args = append(args, addr.Address)
		case 'A':
			if msg.Envelope == nil {
				return "", nil,
@@ -106,8 +130,7 @@ func ParseMessageFormat(
				addr = msg.Envelope.ReplyTo[0]
			}
			retval = append(retval, 's')
			args = append(args,
				fmt.Sprintf("%s@%s", addr.Mailbox, addr.Host))
			args = append(args, addr.Address)
		case 'C':
			retval = append(retval, 'd')
			args = append(args, number)
@@ -158,7 +181,7 @@ func ParseMessageFormat(
			if addr.Name != "" {
				val = addr.Name
			} else {
				val = fmt.Sprintf("%s@%s", addr.Mailbox, addr.Host)
				val = addr.Address
			}
			retval = append(retval, 's')
			args = append(args, val)
@@ -188,7 +211,7 @@ func ParseMessageFormat(
			if addr.Name != "" {
				val = addr.Name
			} else {
				val = fmt.Sprintf("%s@%s", addr.Mailbox, addr.Host)
				val = addr.Address
			}
			retval = append(retval, 's')
			args = append(args, val)
@@ -197,7 +220,7 @@ func ParseMessageFormat(
				return "", nil,
					errors.New("no envelope available for this message")
			}
			addrs := models.FormatAddresses(msg.Envelope.To)
			addrs := FormatAddresses(msg.Envelope.To)
			retval = append(retval, 's')
			args = append(args, addrs)
		case 'R':
@@ -205,7 +228,7 @@ func ParseMessageFormat(
				return "", nil,
					errors.New("no envelope available for this message")
			}
			addrs := models.FormatAddresses(msg.Envelope.Cc)
			addrs := FormatAddresses(msg.Envelope.Cc)
			retval = append(retval, 's')
			args = append(args, addrs)
		case 's':
@@ -226,8 +249,7 @@ func ParseMessageFormat(
			}
			addr := msg.Envelope.To[0]
			retval = append(retval, 's')
			args = append(args,
				fmt.Sprintf("%s@%s", addr.Mailbox, addr.Host))
			args = append(args, addr.Address)
		case 'T':
			retval = append(retval, 's')
			args = append(args, accountName)
@@ -241,8 +263,12 @@ func ParseMessageFormat(
					errors.New("found no address for sender")
			}
			addr := msg.Envelope.From[0]
			mailbox := addr.Address // fallback if there's no @ sign
			if split := strings.SplitN(addr.Address, "@", 2); len(split) == 2 {
				mailbox = split[1]
			}
			retval = append(retval, 's')
			args = append(args, addr.Mailbox)
			args = append(args, mailbox)
		case 'v':
			if msg.Envelope == nil {
				return "", nil,
diff --git a/models/models.go b/models/models.go
index d61b774..b7d7e05 100644
--- a/models/models.go
+++ b/models/models.go
@@ -1,9 +1,9 @@
package models

import (
	"bytes"
	"fmt"
	"io"
	gomail "net/mail"
	"regexp"
	"strings"
	"time"
@@ -134,38 +134,24 @@ type Envelope struct {
	MessageId string
}

type Address struct {
	Name    string
	Mailbox string
	Host    string
}
type Address gomail.Address

var atom *regexp.Regexp = regexp.MustCompile("^[a-z0-9!#$%7'*+-/=?^_`{}|~ ]+$")

func (a Address) Format() string {
// String formats the address. If the address's name
// contains non-ASCII characters it will be quoted but not encoded.
// Meant for display purposes to the humans, not for sending over the wire.
func (a *Address) Format() string {
	if a.Name != "" {
		if atom.MatchString(a.Name) {
			return fmt.Sprintf("%s <%s@%s>", a.Name, a.Mailbox, a.Host)
			return fmt.Sprintf("%s <%s>", a.Name, a.Address)
		} else {
			return fmt.Sprintf("\"%s\" <%s@%s>",
				strings.ReplaceAll(a.Name, "\"", "'"),
				a.Mailbox, a.Host)
			return fmt.Sprintf("\"%s\" <%s>",
				strings.ReplaceAll(a.Name, "\"", "'"), a.Address)
		}
	} else {
		return fmt.Sprintf("<%s@%s>", a.Mailbox, a.Host)
	}
}

// FormatAddresses formats a list of addresses, separating each by a comma
func FormatAddresses(addrs []*Address) string {
	val := bytes.Buffer{}
	for i, addr := range addrs {
		val.WriteString(addr.Format())
		if i != len(addrs)-1 {
			val.WriteString(", ")
		}
		return fmt.Sprintf("<%s>", a.Address)
	}
	return val.String()
}

// OriginalMail is helper struct used for reply/forward
diff --git a/widgets/msgviewer.go b/widgets/msgviewer.go
index 6a91741..587959b 100644
--- a/widgets/msgviewer.go
+++ b/widgets/msgviewer.go
@@ -17,6 +17,7 @@ import (

	"git.sr.ht/~sircmpwn/aerc/config"
	"git.sr.ht/~sircmpwn/aerc/lib"
	"git.sr.ht/~sircmpwn/aerc/lib/format"
	"git.sr.ht/~sircmpwn/aerc/lib/ui"
	"git.sr.ht/~sircmpwn/aerc/models"
)
@@ -130,13 +131,13 @@ func NewMessageViewer(acct *AccountView,
func fmtHeader(msg *models.MessageInfo, header string, timefmt string) string {
	switch header {
	case "From":
		return models.FormatAddresses(msg.Envelope.From)
		return format.FormatAddresses(msg.Envelope.From)
	case "To":
		return models.FormatAddresses(msg.Envelope.To)
		return format.FormatAddresses(msg.Envelope.To)
	case "Cc":
		return models.FormatAddresses(msg.Envelope.Cc)
		return format.FormatAddresses(msg.Envelope.Cc)
	case "Bcc":
		return models.FormatAddresses(msg.Envelope.Bcc)
		return format.FormatAddresses(msg.Envelope.Bcc)
	case "Date":
		return msg.Envelope.Date.Local().Format(timefmt)
	case "Subject":
@@ -496,11 +497,11 @@ func NewPartViewer(acct *AccountView, conf *config.AercConfig,
			case "subject":
				header = info.Envelope.Subject
			case "from":
				header = models.FormatAddresses(info.Envelope.From)
				header = format.FormatAddresses(info.Envelope.From)
			case "to":
				header = models.FormatAddresses(info.Envelope.To)
				header = format.FormatAddresses(info.Envelope.To)
			case "cc":
				header = models.FormatAddresses(info.Envelope.Cc)
				header = format.FormatAddresses(info.Envelope.Cc)
			}
			if f.Regex.Match([]byte(header)) {
				filter = exec.Command("sh", "-c", f.Command)
diff --git a/worker/imap/imap.go b/worker/imap/imap.go
index 7afab02..aa1854d 100644
--- a/worker/imap/imap.go
+++ b/worker/imap/imap.go
@@ -64,8 +64,7 @@ func translateAddresses(addrs []*imap.Address) []*models.Address {
	for _, addr := range addrs {
		converted = append(converted, &models.Address{
			Name:    addr.PersonalName,
			Mailbox: addr.MailboxName,
			Host:    addr.HostName,
			Address: addr.Address(),
		})
	}
	return converted
diff --git a/worker/lib/parse.go b/worker/lib/parse.go
index 58327c9..b003d96 100644
--- a/worker/lib/parse.go
+++ b/worker/lib/parse.go
@@ -205,18 +205,9 @@ func parseAddressList(h *mail.Header, key string) ([]*models.Address, error) {
		return nil, err
	}
	for _, addr := range addrs {
		parts := strings.Split(addr.Address, "@")
		var mbox, host string
		if len(parts) > 1 {
			mbox = strings.Join(parts[0:len(parts)-1], "@")
			host = parts[len(parts)-1]
		} else {
			mbox = addr.Address
		}
		converted = append(converted, &models.Address{
			Name:    addr.Name,
			Mailbox: mbox,
			Host:    host,
			Address: addr.Address,
		})
	}
	return converted, nil
diff --git a/worker/lib/sort.go b/worker/lib/sort.go
index ac8ed07..9d1f50a 100644
--- a/worker/lib/sort.go
+++ b/worker/lib/sort.go
@@ -1,7 +1,6 @@
package lib

import (
	"fmt"
	"sort"
	"strings"

@@ -83,7 +82,7 @@ func sortAddresses(messageInfos []*models.MessageInfo, criterion *types.SortCrit
				if addr.Name != "" {
					return addr.Name
				} else {
					return fmt.Sprintf("%s@%s", addr.Mailbox, addr.Host)
					return addr.Address
				}
			}
			return getName(firstI) < getName(firstJ)
-- 
2.28.0

[PATCH 2/4] msg/reply: handle addresses as addresses Export this patch

This simplifies the code considerably and makes it easier to follow
---
 commands/msg/reply.go | 48 +++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/commands/msg/reply.go b/commands/msg/reply.go
index 6fd6141..2ab12a7 100644
--- a/commands/msg/reply.go
+++ b/commands/msg/reply.go
@@ -5,12 +5,12 @@ import (
	"errors"
	"fmt"
	"io"
	gomail "net/mail"
	"strings"

	"git.sr.ht/~sircmpwn/getopt"

	"git.sr.ht/~sircmpwn/aerc/lib"
	"git.sr.ht/~sircmpwn/aerc/lib/format"
	"git.sr.ht/~sircmpwn/aerc/models"
	"git.sr.ht/~sircmpwn/aerc/widgets"
)
@@ -60,7 +60,15 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
		return errors.New("No account selected")
	}
	conf := acct.AccountConfig()
	us, _ := gomail.ParseAddress(conf.From)
	var from *models.Address
	fa, err := format.ParseAddressList(conf.From)
	if err != nil {
		return err
	} else if len(fa) == 0 {
		return fmt.Errorf("No from address specified")
	} else {
		from = fa[0]
	}
	store := widget.Store()
	if store == nil {
		return errors.New("Cannot perform action. Messages still loading")
@@ -72,27 +80,18 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
	acct.Logger().Println("Replying to email " + msg.Envelope.MessageId)

	var (
		to     []string
		cc     []string
		toList []*models.Address
		to []*models.Address
		cc []*models.Address
	)
	if args[0] == "reply" {
		if len(msg.Envelope.ReplyTo) != 0 {
			toList = msg.Envelope.ReplyTo
			to = msg.Envelope.ReplyTo
		} else {
			toList = msg.Envelope.From
		}
		for _, addr := range toList {
			if addr.Name != "" {
				to = append(to, fmt.Sprintf("%s <%s@%s>",
					addr.Name, addr.Mailbox, addr.Host))
			} else {
				to = append(to, fmt.Sprintf("<%s@%s>", addr.Mailbox, addr.Host))
			}
			to = msg.Envelope.From
		}
		isMainRecipient := func(a *models.Address) bool {
			for _, ta := range toList {
				if ta.Mailbox == a.Mailbox && ta.Host == a.Host {
			for _, ta := range to {
				if ta.Address == a.Address {
					return true
				}
			}
@@ -104,15 +103,16 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
				if isMainRecipient(addr) {
					continue
				}
				cc = append(cc, addr.Format())
				cc = append(cc, addr)
			}
			envTos := make([]*models.Address, 0, len(msg.Envelope.To))
			for _, addr := range msg.Envelope.To {
				address := fmt.Sprintf("%s@%s", addr.Mailbox, addr.Host)
				if strings.EqualFold(address, us.Address) {
				if addr.Address == from.Address {
					continue
				}
				to = append(to, addr.Format())
				envTos = append(envTos, addr)
			}
			to = append(to, envTos...)
		}
	}

@@ -124,8 +124,8 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
	}

	defaults := map[string]string{
		"To":          strings.Join(to, ", "),
		"Cc":          strings.Join(cc, ", "),
		"To":          format.FormatAddresses(to),
		"Cc":          format.FormatAddresses(cc),
		"Subject":     subject,
		"In-Reply-To": msg.Envelope.MessageId,
	}
@@ -133,7 +133,7 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {

	addTab := func() error {
		if template != "" {
			original.From = models.FormatAddresses(msg.Envelope.From)
			original.From = format.FormatAddresses(msg.Envelope.From)
			original.Date = msg.Envelope.Date
		}

-- 
2.28.0
> This simplifies the code considerably and makes it easier to follow
> ---
> commands/msg/reply.go | 48 +++++++++++++++++++++----------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/commands/msg/reply.go b/commands/msg/reply.go
> index 6fd6141..2ab12a7 100644
> --- a/commands/msg/reply.go
> +++ b/commands/msg/reply.go
> @@ -5,12 +5,12 @@ import (
> "errors"
> "fmt"
> "io"
> - gomail "net/mail"
> "strings"
> 
> "git.sr.ht/~sircmpwn/getopt"
> 
> "git.sr.ht/~sircmpwn/aerc/lib"
> + "git.sr.ht/~sircmpwn/aerc/lib/format"
> "git.sr.ht/~sircmpwn/aerc/models"
> "git.sr.ht/~sircmpwn/aerc/widgets"
> )
> @@ -60,7 +60,15 @@ func (reply) Execute(aerc *widgets.Aerc, args
> []string) error {
> return errors.New("No account selected")
> }
> conf := acct.AccountConfig()
> - us, _ := gomail.ParseAddress(conf.From)
> + var from *models.Address
> + fa, err := format.ParseAddressList(conf.From)
Why not use format.parseAddress? Can the from field ever be a list (if
so, you can ignore this comment)? You can skip the third else condition below then too.

[PATCH 3/4] Add account alias configuration and correctly set From field Export this patch

From: y0ast <joost@joo.st>

We infer the correct From using the To: and Cc: field of the email that
we reply to.
---
 commands/msg/reply.go | 25 ++++++++++++++++++++++++-
 config/config.go      |  3 +++
 doc/aerc-config.5.scd |  7 +++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/commands/msg/reply.go b/commands/msg/reply.go
index 2ab12a7..c2cd6a7 100644
--- a/commands/msg/reply.go
+++ b/commands/msg/reply.go
@@ -69,6 +69,10 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
	} else {
		from = fa[0]
	}
	alias_of_us, err := format.ParseAddressList(conf.Aliases)
	if err != nil {
		return err
	}
	store := widget.Store()
	if store == nil {
		return errors.New("Cannot perform action. Messages still loading")
@@ -77,7 +81,6 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
	if err != nil {
		return err
	}
	acct.Logger().Println("Replying to email " + msg.Envelope.MessageId)

	var (
		to []*models.Address
@@ -89,6 +92,25 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
		} else {
			to = msg.Envelope.From
		}

		// figure out the sending from address if we have aliases
		if len(alias_of_us) != 0 {
			allRecipients := append(msg.Envelope.To, msg.Envelope.Cc...)
			for _, addr := range allRecipients {
				if addr.Address == from.Address {
					from = addr
					break
				}
				for _, alias := range alias_of_us {
					if addr.Address == alias.Address {
						from = addr
						break
					}
				}
			}

		}

		isMainRecipient := func(a *models.Address) bool {
			for _, ta := range to {
				if ta.Address == a.Address {
@@ -126,6 +148,7 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
	defaults := map[string]string{
		"To":          format.FormatAddresses(to),
		"Cc":          format.FormatAddresses(cc),
		"From":        from.Format(),
		"Subject":     subject,
		"In-Reply-To": msg.Envelope.MessageId,
	}
diff --git a/config/config.go b/config/config.go
index 12096ba..3ae26c1 100644
--- a/config/config.go
+++ b/config/config.go
@@ -75,6 +75,7 @@ type AccountConfig struct {
	Default         string
	Postpone        string
	From            string
	Aliases         string
	Name            string
	Source          string
	SourceCredCmd   string
@@ -202,6 +203,8 @@ func loadAccountConfig(path string) ([]AccountConfig, error) {
				account.OutgoingCredCmd = val
			} else if key == "from" {
				account.From = val
			} else if key == "aliases" {
				account.Aliases = val
			} else if key == "copy-to" {
				account.CopyTo = val
			} else if key == "archive" {
diff --git a/doc/aerc-config.5.scd b/doc/aerc-config.5.scd
index fcd70ec..64a0998 100644
--- a/doc/aerc-config.5.scd
+++ b/doc/aerc-config.5.scd
@@ -409,6 +409,13 @@ Note that many of these configuration options are written for you, such as

	Default: none

*aliases*
	All aliases of the current account. These will be used to fill in the From:
	field. Make sure that your email server accepts this value, or for example
	use *aerc-sendmail*(5) in combination with msmtp and --read-envelope-from.

	Default: none

*outgoing*
	Specifies the transport for sending outgoing emails on this account.  It
	should be a connection string, and the specific meaning of each component
-- 
2.28.0

[PATCH 4/4] msg/reply: don't cc the sending address on reply all Export this patch

---
 commands/msg/reply.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/commands/msg/reply.go b/commands/msg/reply.go
index c2cd6a7..7bd193f 100644
--- a/commands/msg/reply.go
+++ b/commands/msg/reply.go
@@ -121,8 +121,8 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
		}
		if replyAll {
			for _, addr := range msg.Envelope.Cc {
				//dedupe stuff already in the to: header, no need to repeat
				if isMainRecipient(addr) {
				//dedupe stuff from the to/from headers
				if isMainRecipient(addr) || addr.Address == from.Address {
					continue
				}
				cc = append(cc, addr)
-- 
2.28.0