~sircmpwn/aerc

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
15 4

[PATCH v2 0/4] msg/reply improvements

Details
Message ID
<20200820195611.321440-1-reto@labrat.space>
DKIM signature
pass
Download raw message
While reading joost's PR for the aliases I got fed up with the reply code...
It treated addresses as strings which got messy after a while.

This patch series tries to clean this up.
As an added bonus, the bug mentioned by joost regarding the duplication of the
from address in the CC header was addressed as well.

@joost: Considering that I heavily modified your patch, but still kept your name
as the author, mind giving a review?
If you prefer I can change the author to be me as well.

Opinions of anyone else are of course appreciated as well.


v1 --> v2:
  * move changes to the proper commit
  * fix bug in the loop break statement
  * change to format.ParseAddress for the from address parsing instead
    of the convoluted if chain.
    While at it the method also returns a models.Address and gets exported.

Thanks for the review Joost, much appreciated.


Reto Brunner (3):
  base models.Address on the mail.Address type
  msg/reply: handle addresses as addresses
  msg/reply: don't cc the sending address on reply all

y0ast (1):
  Add account alias configuration and correctly set From field

 commands/msg/forward.go |  3 +-
 commands/msg/reply.go   | 69 ++++++++++++++++++++++++++---------------
 config/config.go        |  3 ++
 doc/aerc-config.5.scd   |  7 +++++
 lib/format/format.go    | 60 +++++++++++++++++++++++++----------
 models/models.go        | 34 ++++++--------------
 widgets/msgviewer.go    | 15 ++++-----
 worker/imap/imap.go     |  3 +-
 worker/lib/parse.go     | 11 +------
 worker/lib/sort.go      |  3 +-
 10 files changed, 121 insertions(+), 87 deletions(-)

-- 
2.28.0

[PATCH v2 1/4] base models.Address on the mail.Address type

Details
Message ID
<20200820195611.321440-2-reto@labrat.space>
In-Reply-To
<20200820195611.321440-1-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
Patch: +67 -62
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    | 60 ++++++++++++++++++++++++++++++-----------
 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, 67 insertions(+), 62 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..06b4d3f 100644
--- a/lib/format/format.go
+++ b/lib/format/format.go
@@ -2,22 +2,46 @@ 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 {
func ParseAddress(address string) (*models.Address, error) {
	addrs, err := gomail.ParseAddress(address)
	if err != nil {
		return nil
		return nil, err
	}
	return (*models.Address)(addrs), nil
}

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
	}

	return addrs
	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(
@@ -29,7 +53,10 @@ func ParseMessageFormat(
	retval := make([]byte, 0, len(format))
	var args []interface{}

	accountFromAddress := parseAddress(fromAddress)
	accountFromAddress, err := ParseAddress(fromAddress)
	if err != nil {
		return "", nil, err
	}

	var c rune
	for i, ni := 0, 0; i < len(format); {
@@ -87,8 +114,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 +132,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 +183,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 +213,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 +222,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 +230,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 +251,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 +265,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 v2 2/4] msg/reply: handle addresses as addresses

Details
Message ID
<20200820195611.321440-3-reto@labrat.space>
In-Reply-To
<20200820195611.321440-1-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
Patch: +20 -24
This simplifies the code considerably and makes it easier to follow
---
 commands/msg/reply.go | 44 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/commands/msg/reply.go b/commands/msg/reply.go
index 6fd6141..7181e1e 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,10 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
		return errors.New("No account selected")
	}
	conf := acct.AccountConfig()
	us, _ := gomail.ParseAddress(conf.From)
	from, err := format.ParseAddress(conf.From)
	if err != nil {
		return err
	}
	store := widget.Store()
	if store == nil {
		return errors.New("Cannot perform action. Messages still loading")
@@ -72,27 +75,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 +98,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 +119,9 @@ 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),
		"From":        from.Format(),
		"Subject":     subject,
		"In-Reply-To": msg.Envelope.MessageId,
	}
@@ -133,7 +129,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

[PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<20200820195611.321440-4-reto@labrat.space>
In-Reply-To
<20200820195611.321440-1-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
Patch: +34 -1
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 7181e1e..8cca2b1 100644
--- a/commands/msg/reply.go
+++ b/commands/msg/reply.go
@@ -64,6 +64,10 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
	if err != nil {
		return err
	}
	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")
@@ -72,7 +76,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
@@ -84,6 +87,26 @@ 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...)
		outer:
			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 outer
					}
				}
			}

		}

		isMainRecipient := func(a *models.Address) bool {
			for _, ta := range to {
				if ta.Address == a.Address {
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 v2 4/4] msg/reply: don't cc the sending address on reply all

Details
Message ID
<20200820195611.321440-5-reto@labrat.space>
In-Reply-To
<20200820195611.321440-1-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
Patch: +2 -2
---
 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 8cca2b1..d4b3be6 100644
--- a/commands/msg/reply.go
+++ b/commands/msg/reply.go
@@ -117,8 +117,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

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<C52S0TTHP1BA.1IBKXF1VYPIHS@arch>
In-Reply-To
<20200820195611.321440-4-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
On Thu Aug 20, 2020 at 8:56 PM BST, Reto Brunner wrote:
> 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 7181e1e..8cca2b1 100644
> --- a/commands/msg/reply.go
> +++ b/commands/msg/reply.go
> @@ -64,6 +64,10 @@ func (reply) Execute(aerc *widgets.Aerc, args []string) error {
>  	if err != nil {
>  		return err
>  	}
> +	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")
> @@ -72,7 +76,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
> @@ -84,6 +87,26 @@ 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...)
> +		outer:
> +			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 outer
> +					}
> +				}
> +			}
> +
> +		}
> +
Hmm I think it's nicer to make a simple function here, like the one
below, but for example "isUsorAlias". Then you can just check the return
value of that and do a normal break if you hit True.

Other than this small thing  the patches look good! Everything works as
expected in my testing.
>  		isMainRecipient := func(a *models.Address) bool {
>  			for _, ta := range to {
>  				if ta.Address == a.Address {
> 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

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<635BDF42-ACE1-48EC-8A36-3431AE38BBFE@labrat.space>
In-Reply-To
<C52S0TTHP1BA.1IBKXF1VYPIHS@arch> (view parent)
DKIM signature
pass
Download raw message
On 21 August 2020 17:23:34 CEST, joost@joo.st wrote:

>Hmm I think it's nicer to make a simple function here, like the one
>below, but for example "isUsorAlias". Then you can just check the
>return
>value of that and do a normal break if you hit True.
>
>Other than this small thing  the patches look good! Everything works as
>expected in my testing.

Hm, I'm not really fond of this as it would be an inline function which is always a bit funny to read.

"Because you asked."
"Why?"

instead of

"Why?"
"Because you asked."

If  the control flow logic is non trivial, sure, or if the function is used more than once. Then I personally like inline functions.

Here it's a simple nested loop with a single comparison, and granted I fucked up the break the first time round, but "break outer" seems to be common enough.

How "hard" is your opinion here?

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<C52SRWLSVJ99.3T3B94KIXUM07@arch>
In-Reply-To
<635BDF42-ACE1-48EC-8A36-3431AE38BBFE@labrat.space> (view parent)
DKIM signature
pass
Download raw message
On Fri Aug 21, 2020 at 4:57 PM BST, Reto wrote:
> How "hard" is your opinion here?

Fair enough! Not hard at all, happy to follow your preference.
Details
Message ID
<20200822083528.fh2xutddugoxty4r@feather.localdomain>
In-Reply-To
<20200820195611.321440-1-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
On Thu, Aug 20, 2020 at 09:56:07PM +0200, Reto Brunner wrote:
> Reto Brunner (3):
>   base models.Address on the mail.Address type
>   msg/reply: handle addresses as addresses
>   msg/reply: don't cc the sending address on reply all
> 
> y0ast (1):
>   Add account alias configuration and correctly set From field

Merged to master.
Thanks a lot for the initial PR and the review Joost.

Cheers,
Reto

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Ning Shi
Details
Message ID
<C54V62PK2CNS.3UKKYPRJZ1UHS@nings-mbp.lan>
In-Reply-To
<20200820195611.321440-4-reto@labrat.space> (view parent)
DKIM signature
pass
Download raw message
> diff --git a/commands/msg/reply.go b/commands/msg/reply.go
> index 7181e1e..8cca2b1 100644
> --- a/commands/msg/reply.go
> +++ b/commands/msg/reply.go
> @@ -64,6 +64,10 @@ func (reply) Execute(aerc *widgets.Aerc, args
> []string) error {
>         if err != nil {
>                 return err
>         }
> +       alias_of_us, err := format.ParseAddressList(conf.Aliases)
> +       if err != nil {
> +               return err
> +       }

This seems to require "aliases" to be set in the configuration which is
not obvious in the man page. Should it either be optional or made
explicit that it's required in the man page?

--
Ning

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<A240B68F-1AC8-4B21-8737-ED0A432C3253@labrat.space>
In-Reply-To
<C54V62PK2CNS.3UKKYPRJZ1UHS@nings-mbp.lan> (view parent)
DKIM signature
pass
Download raw message
On 24 August 2020 04:16:48 CEST, Ning Shi <ningshi2@gmail.com> wrote:

>This seems to require "aliases" to be set in the configuration which is
>not obvious in the man page. Should it either be optional or made
>explicit that it's required in the man page?

Did you even test this?
That's exactly what the patch does.

It sets a default of ""

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Ning Shi
Details
Message ID
<C558RL3W33S8.2BJ1JJLLC5OQR@nings-mbp.lan>
In-Reply-To
<A240B68F-1AC8-4B21-8737-ED0A432C3253@labrat.space> (view parent)
DKIM signature
pass
Download raw message
On Mon Aug 24, 2020 at 12:19 AM EDT, Reto wrote:
> Did you even test this?

I did. If I don't set aliases in the config, I get an error saying no
address when I try to reply.

> That's exactly what the patch does.
>
> It sets a default of ""

The man page says that the default is none.

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<A1F2B202-4C6D-41AC-8158-986BDD248C55@labrat.space>
In-Reply-To
<C558RL3W33S8.2BJ1JJLLC5OQR@nings-mbp.lan> (view parent)
DKIM signature
pass
Download raw message
On 24 August 2020 14:56:08 CEST, Ning Shi <ningshi2@gmail.com> wrote:
>On Mon Aug 24, 2020 at 12:19 AM EDT, Reto wrote:
>The man page says that the default is none.

Conceptually that's true. However the config type is a struct that defines the option as a string, there is no "none" / nil for strings in go. Hence ""

Are you trying to reply to a message you sent yourself?
Aerc also removes the sending address from the recipient list.
(doesn't really make sense to self reply on a large distribution list, you normally get a copy into the sent folder by other means)

Can you give me a reproducer? Exact steps / config file (naturally without passwords...)?

I don't have an alias option set and it works perfectly fine for me.

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<20200824194458.q6amil2nwsk6jijs@feather.localdomain>
In-Reply-To
<C558RL3W33S8.2BJ1JJLLC5OQR@nings-mbp.lan> (view parent)
DKIM signature
pass
Download raw message
On Mon, Aug 24, 2020 at 08:56:08AM -0400, Ning Shi wrote:
> On Mon Aug 24, 2020 at 12:19 AM EDT, Reto wrote:
> > Did you even test this?
> 
> I did. If I don't set aliases in the config, I get an error saying no
> address when I try to reply.

Let me guess, the affected account doesn't have a `from` option set?

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Details
Message ID
<20200824211803.wytzienahianixax@feather.localdomain>
In-Reply-To
<20200824194458.q6amil2nwsk6jijs@feather.localdomain> (view parent)
DKIM signature
pass
Download raw message
On Mon, Aug 24, 2020 at 09:44:58PM +0200, Reto wrote:
> On Mon, Aug 24, 2020 at 08:56:08AM -0400, Ning Shi wrote:
> > On Mon Aug 24, 2020 at 12:19 AM EDT, Reto wrote:
> > > Did you even test this?
> >
> > I did. If I don't set aliases in the config, I get an error saying no
> > address when I try to reply.
>
> Let me guess, the affected account doesn't have a `from` option set?

Nevermind... you are on go 1.14 I presume?

Please apply and give feedback http://ix.io/2v8B

There's a bug in the go stdlib fixed by

commit 30e3bf2051e1659ba7ea1d14849f79deb82a5606
Author: Timmy Douglas <timmyd983@gmail.com>
Date:   Sat Feb 1 22:14:30 2020 +0000

net/mail: skip empty entries in parseAddressList

...in 1.15

Cheers,
Reto

Re: [PATCH v2 3/4] Add account alias configuration and correctly set From field

Ning Shi
Details
Message ID
<C55PVHSBGCFS.1K16KYR1LF7ZN@nings-mbp.lan>
In-Reply-To
<20200824211803.wytzienahianixax@feather.localdomain> (view parent)
DKIM signature
pass
Download raw message
On Mon Aug 24, 2020 at 5:18 PM EDT, Reto wrote:
> Nevermind... you are on go 1.14 I presume?

Sorry for the late response. Yes, I was using 1.14. The issue is
resolved after upgrading to 1.15. Thank you for pointing it out.

I didn't get a chance to test the patch before upgrading. The code
change does make sense.

--
Ning
Review patch Export thread (mbox)