Received: from mail.cmpwn.com (mail.cmpwn.com [45.56.77.53])
	by mail.sr.ht (Postfix) with ESMTPS id E5189402BB
	for <~sircmpwn/aerc@lists.sr.ht>; Fri,  7 Jun 2019 14:06:36 +0000 (UTC)
Authentication-Results: mail.sr.ht;	dkim=pass (1024-bit key)
 header.d=cmpwn.com header.i=@cmpwn.com header.b=N10Pp1uT
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=cmpwn.com; s=cmpwn;
	t=1559916592; bh=5YRoKyM1+34GRM1NcRkxFRXZXlLOCRyy3S4gK0t8K6s=;
	h=In-Reply-To:To:From:Date:Subject;
	b=N10Pp1uTua/YxfQRSxre6WTEPTNSNpSdBJ6rX15sQ20EjTt7ywuUt9E+8zd338SWJ
	 PfQJ7yGrusjsSrEz29z6Qr06Y7fVKpYjaLkyygfUdaHeSyKG+8Sn8HJN0u2SP/e8c2
	 GtLNFu3ZhFDPbVprK8I16bMRrvo4iIIaTEzpS2xE=
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20190606095050.29563-1-r@gnzler.io>
To: =?utf-8?q?Robert_G=C3=BCnzler?= <r@gnzler.io>,
 <~sircmpwn/aerc@lists.sr.ht>
From: "Drew DeVault" <sir@cmpwn.com>
Date: Fri, 07 Jun 2019 10:06:35 -0400
Subject: Re: [PATCH v2] Add archive command
Message-Id: <BUNK7N51XEOH.6SMHCWUUQLY8@homura>
Content-Type: text/plain; charset=UTF-8

Thanks for the patch! I have just a few more comments.

On Thu Jun 6, 2019 at 11:50 AM Robert G=C3=BCnzler wrote:
> +	switch args[1] {
> +	case ARCHIVE_MONTH:
> +		archiveDir =3D path.Join(archiveDir,
> +			fmt.Sprintf("%d", msg.Envelope.Date.Year()),
> +			fmt.Sprintf("%02d", msg.Envelope.Date.Month()))
> +	case ARCHIVE_YEAR:
> +		archiveDir =3D path.Join(archiveDir, fmt.Sprintf("%v", msg.Envelope.Da=
te.Year()))
> +	case ARCHIVE_FLAT:
> +	}

Could use a /* this space deliberately left blank */ here on the last
case branch

> +	store.Move([]uint32{msg.Uid}, archiveDir, true, func(msg types.WorkerMe=
ssage) {

80 col limit

> +		switch msg :=3D msg.(type) {
> +		case *types.Done:
> +			aerc.PushStatus("Messages archived.", 10*time.Second)
> +			acct.Directories().UpdateList(nil)

Perhaps this UpdateList(nil) might be more suitable for the store to do
somehow? Like bubble up a callback from the store when it creates new
directories, then have the account widget update the list, idk

> diff --git a/commands/msg/move.go b/commands/msg/move.go
> index 1224efa..78a60ba 100644
> --- a/commands/msg/move.go
> +++ b/commands/msg/move.go
> -%<-
> -	store.Move([]uint32{msg.Uid}, args[1], func(msg types.WorkerMessage) {
> +	store.Move([]uint32{msg.Uid}, args[optind+1], createParents, func(msg t=
ypes.WorkerMessage) {

80 col limit

>  type AccountConfig struct {
> +	Archive         string

Let's go ahead and default this to "Archive" even if the user doesn't
specify it in their config. Speaking of config things, feel free to add
a default keybinding to the msglist & viewer contexts:

A =3D :archive flat<Enter>