~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
1

[PATCH] Add a 'folders-exclude' option

Details
Message ID
<20200628160559.1146-1-araspik@protonmail.com>
DKIM signature
pass
Download raw message
Patch: +46 -9
Added a 'folders-exclude' option that allows removing selected folders
from the directory list sidebar. My motivating example was that removing
a single folder from the list using Golang regexes seemed pretty hard,
so this is a better way to do it. The excluded folders list is included
in the man page.
---
 config/config.go      |  5 +++++
 doc/aerc-config.5.scd |  7 +++++++
 widgets/dirlist.go    | 43 ++++++++++++++++++++++++++++++++++---------
 3 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/config/config.go b/config/config.go
index 8ebd69d..ce59944 100644
--- a/config/config.go
+++ b/config/config.go
@@ -76,6 +76,7 @@ type AccountConfig struct {
	Source          string
	SourceCredCmd   string
	Folders         []string
	FoldersExclude	[]string
	Params          map[string]string
	Outgoing        string
	OutgoingCredCmd string
@@ -186,6 +187,10 @@ func loadAccountConfig(path string) ([]AccountConfig, error) {
				folders := strings.Split(val, ",")
				sort.Strings(folders)
				account.Folders = folders
			} else if key == "folders-exclude" {
				folders := strings.Split(val, ",")
				sort.Strings(folders)
				account.FoldersExclude = folders
			} else if key == "source-cred-cmd" {
				account.SourceCredCmd = val
			} else if key == "outgoing" {
diff --git a/doc/aerc-config.5.scd b/doc/aerc-config.5.scd
index 2bd3076..af64ad6 100644
--- a/doc/aerc-config.5.scd
+++ b/doc/aerc-config.5.scd
@@ -376,6 +376,13 @@ Note that many of these configuration options are written for you, such as

	Default: all folders

*folders-exclude*
	Specifies the comma separated list of folders to exclude from the sidebar.
	Names prefixed with ~ are interpreted as regular expressions.
	Note that this overrides anything from *folders*.

	Default: no folders

*folders-sort*
	Specifies a comma separated list of folders to be shown at the top of the
	list in the provided order. Remaining folders will be sorted alphabetically.
diff --git a/widgets/dirlist.go b/widgets/dirlist.go
index f12631b..83edfd7 100644
--- a/widgets/dirlist.go
+++ b/widgets/dirlist.go
@@ -393,21 +393,46 @@ func (dirlist *DirectoryList) sortDirsByFoldersSortConfig() {
// dirstore, based on the AccountConfig.Folders option
func (dirlist *DirectoryList) filterDirsByFoldersConfig() {
	dirlist.dirs = dirlist.store.List()
	// config option defaults to show all if unset
	configFolders := dirlist.acctConf.Folders
	configFoldersExclude := dirlist.acctConf.FoldersExclude
	// 'folders' unset and 'folders-exclude' unset: show all
	// 'folders' set and 'folders-exclude' unset: show 'folders'
	// 'folders' unset and 'folders-exclude' set: show all except for 'folders-exclude'
	// 'folders' set and 'folders-exclude' set: show 'folders' except for 'folders-exclude'

	// intermediate list of dirs
	var filtered []string

	// 'folders': if unset, select all, otherwise select only 'folders'
	if len(configFolders) == 0 {
		return
		filtered = dirlist.dirs
	} else {
		for _, folder := range dirlist.dirs {
			for _, cfgfolder := range configFolders {
				if folderMatches(folder, cfgfolder) {
					filtered = append(filtered, folder)
					break
				}
			}
		}
	}
	var filtered []string
	for _, folder := range dirlist.dirs {
		for _, cfgfolder := range configFolders {
			if folderMatches(folder, cfgfolder) {
				filtered = append(filtered, folder)
				break

	// another intermediate list of dirs
	var excluded []string

	// 'folders-exclude': if unset, select 'filtered', otherwise remove 'folders-exclude'
	if len(configFoldersExclude) != 0 {
		for _, folder := range filtered {
			for _, cfgnotfolder := range configFoldersExclude {
				if !folderMatches(folder, cfgnotfolder) {
					excluded = append(excluded, folder)
					break
				}
			}
		}
	}
	dirlist.dirs = filtered

	dirlist.dirs = excluded
}

func (dirlist *DirectoryList) SelectedMsgStore() (*lib.MessageStore, bool) {
-- 
2.27.0
Details
Message ID
<20200629051449.mdfzgu53e2kyt5vb@feather.localdomain>
In-Reply-To
<20200628160559.1146-1-araspik@protonmail.com> (view parent)
DKIM signature
pass
Download raw message
Time appropriate greetings,

On Sun, Jun 28, 2020 at 04:04:59PM +0000, ARaspiK wrote:
> Added a 'folders-exclude' option that allows removing selected folders
> from the directory list sidebar. My motivating example was that removing
> a single folder from the list using Golang regexes seemed pretty hard,
> so this is a better way to do it. The excluded folders list is included
> in the man page.

Sounds reasonable to me.
Some comments regarding the code:

> diff --git a/widgets/dirlist.go b/widgets/dirlist.go
> index f12631b..83edfd7 100644
> --- a/widgets/dirlist.go
> +++ b/widgets/dirlist.go
Here would be the function documentation, please do adapt it if you change the
way it works.
> @@ -393,21 +393,46 @@ func (dirlist *DirectoryList) sortDirsByFoldersSortConfig() {
>  // dirstore, based on the AccountConfig.Folders option
>  func (dirlist *DirectoryList) filterDirsByFoldersConfig() {
>  	dirlist.dirs = dirlist.store.List()
> -	// config option defaults to show all if unset
>  	configFolders := dirlist.acctConf.Folders
> +	configFoldersExclude := dirlist.acctConf.FoldersExclude
> +	// 'folders' unset and 'folders-exclude' unset: show all
> +	// 'folders' set and 'folders-exclude' unset: show 'folders'
> +	// 'folders' unset and 'folders-exclude' set: show all except for 'folders-exclude'
> +	// 'folders' set and 'folders-exclude' set: show 'folders' except for 'folders-exclude'
That's maybe a tad excessive?
I'm not sure that any comment is needed but if so it could be simplified to
"use folders as an inclusion and folders-exclude as an exclusion filter"
You really don't care if they are unset or set.

In fact you could somewhat simplify the code and help the reader if you do
something like:

```go
// filterDirsByFoldersConfig sets dirlist.dirs to the filtered subset of the
// dirstore, based on the AccountConfig.Folders* options
func (dirlist *DirectoryList) filterDirsByFoldersConfig() {

	// helper function used to filter based on various regex lists
	// include determines if filters are inclusion or exclusion patterns
	// empty filters means src will not be filtered at all
	filterDirsRegex := func(src, filters []string, include bool) []string {
		if len(filters) == 0 {
			return src
		}
		var dest []string
		for _, folder := range src {
			for _, f := range filters {
				matches := folderMatches(folder, f)
				if !include {
					matches = !matches
				}
				if matches {
					dest = append(dest, folder)
					break
				}
			}
		}
		return dest
	}

	dirlist.dirs = dirlist.store.List()

	configFolders := dirlist.acctConf.Folders
	dirlist.dirs = filterDirsRegex(dirlist.dirs, configFolders, true)

	configFoldersExclude := dirlist.acctConf.FoldersExclude
	dirlist.dirs = filterDirsRegex(dirlist.dirs, configFoldersExclude, false)
}
```

That's just my personal preference though.
If you really want you could even make it a method on dirlist directly.
I just thought that this method will only be used in this function so we might as
well "inline" it...

Feel free to pick whichever version you like better (yours or mine).
However please do fix the docstring as well as reduce the comments if you don't
mind.

Cheers,
Reto
Review patch Export thread (mbox)