From 9d0297e9d913a92b2d7ae02692e83f0f4093a766 Mon Sep 17 00:00:00 2001 From: Robin Jarry Date: Sun, 11 Dec 2022 23:57:30 +0100 Subject: config: rework contextual sections implementation The current contextual binds and ui config API is awkward and cumbersome to use. Rework it to make it more elegant. Store the contextual sections as private fields of the UIConfig and KeyBindings structures. Add cache to avoid recomputation of the composed UIConfig and KeyBindings objects every time a contextual item is requested. Replace the cache from DirectoryList with that. Signed-off-by: Robin Jarry Acked-by: Tim Culverhouse --- config/binds.go | 90 +++++++++++++++++++++++++++++------------- config/config.go | 46 ++++++++++------------ config/ui.go | 109 ++++++++++++++++++++++++++++++++++----------------- widgets/account.go | 4 +- widgets/aerc.go | 19 +++++---- widgets/compose.go | 5 +-- widgets/dirlist.go | 18 +-------- widgets/msglist.go | 14 +------ widgets/msgviewer.go | 7 +--- 9 files changed, 173 insertions(+), 139 deletions(-) diff --git a/config/binds.go b/config/binds.go index 6ddeb1e1..abaff78c 100644 --- a/config/binds.go +++ b/config/binds.go @@ -27,11 +27,17 @@ type BindingConfig struct { Terminal *KeyBindings } +type bindsContextType int + +const ( + bindsContextFolder bindsContextType = iota + bindsContextAccount +) + type BindingConfigContext struct { - ContextType ContextType + ContextType bindsContextType Regex *regexp.Regexp Bindings *KeyBindings - BindContext string } type KeyStroke struct { @@ -47,11 +53,20 @@ type Binding struct { type KeyBindings struct { Bindings []*Binding - // If false, disable global keybindings in this context Globals bool // Which key opens the ex line (default is :) ExKey KeyStroke + + // private + contextualBinds []*BindingConfigContext + contextualCounts map[bindsContextType]int + contextualCache map[bindsContextKey]*KeyBindings +} + +type bindsContextKey struct { + ctxType bindsContextType + value string } const ( @@ -115,13 +130,6 @@ func (config *AercConfig) parseBinds(root string) error { } } - config.Bindings.Global.Globals = false - for _, contextBind := range config.ContextualBinds { - if contextBind.BindContext == "default" { - contextBind.Bindings.Globals = false - } - } - log.Debugf("binds.conf: %#v", config.Bindings) return nil } @@ -168,6 +176,12 @@ func (config *AercConfig) LoadBinds(binds *ini.File, baseName string, baseGroup *baseGroup = MergeBindings(binds, *baseGroup) } + b := *baseGroup + + if baseName == "default" { + b.Globals = false + } + for _, sectionName := range binds.SectionStrings() { if !strings.Contains(sectionName, baseName+":") || strings.Contains(sectionName, baseName+"::") { @@ -183,10 +197,12 @@ func (config *AercConfig) LoadBinds(binds *ini.File, baseName string, baseGroup if err != nil { return err } + if baseName == "default" { + binds.Globals = false + } contextualBind := BindingConfigContext{ - Bindings: binds, - BindContext: baseName, + Bindings: binds, } var index int @@ -215,15 +231,16 @@ func (config *AercConfig) LoadBinds(binds *ini.File, baseName string, baseGroup log.Warnf("binds.conf: unexistent account: %s", acctName) continue } - contextualBind.ContextType = BIND_CONTEXT_ACCOUNT + contextualBind.ContextType = bindsContextAccount case "folder": // No validation needed. If the folder doesn't exist, the binds // never get used - contextualBind.ContextType = BIND_CONTEXT_FOLDER + contextualBind.ContextType = bindsContextFolder default: return fmt.Errorf("Unknown Context Bind Section: %s", sectionName) } - config.ContextualBinds = append(config.ContextualBinds, contextualBind) + b.contextualBinds = append(b.contextualBinds, &contextualBind) + b.contextualCounts[contextualBind.ContextType]++ } return nil @@ -245,8 +262,10 @@ func defaultBindsConfig() BindingConfig { func NewKeyBindings() *KeyBindings { return &KeyBindings{ - ExKey: KeyStroke{tcell.ModNone, tcell.KeyRune, ':'}, - Globals: true, + ExKey: KeyStroke{tcell.ModNone, tcell.KeyRune, ':'}, + Globals: true, + contextualCache: make(map[bindsContextKey]*KeyBindings), + contextualCounts: make(map[bindsContextType]int), } } @@ -260,26 +279,41 @@ func MergeBindings(bindings ...*KeyBindings) *KeyBindings { return merged } -func (config AercConfig) MergeContextualBinds(baseBinds *KeyBindings, - contextType ContextType, reg string, bindCtx string, +func (base *KeyBindings) contextual( + contextType bindsContextType, reg string, ) *KeyBindings { - bindings := baseBinds - for _, contextualBind := range config.ContextualBinds { + if base.contextualCounts[contextType] == 0 { + // shortcut if no contextual binds for that type + return base + } + + key := bindsContextKey{ctxType: contextType, value: reg} + c, found := base.contextualCache[key] + if found { + return c + } + + c = base + for _, contextualBind := range base.contextualBinds { if contextualBind.ContextType != contextType { continue } - if !contextualBind.Regex.Match([]byte(reg)) { continue } + c = MergeBindings(contextualBind.Bindings, c) + } + base.contextualCache[key] = c - if contextualBind.BindContext != bindCtx { - continue - } + return c +} - bindings = MergeBindings(contextualBind.Bindings, bindings) - } - return bindings +func (bindings *KeyBindings) ForAccount(account string) *KeyBindings { + return bindings.contextual(bindsContextAccount, account) +} + +func (bindings *KeyBindings) ForFolder(folder string) *KeyBindings { + return bindings.contextual(bindsContextFolder, folder) } func (bindings *KeyBindings) Add(binding *Binding) { diff --git a/config/config.go b/config/config.go index b5cc0d60..90951985 100644 --- a/config/config.go +++ b/config/config.go @@ -15,20 +15,18 @@ import ( ) type AercConfig struct { - Bindings BindingConfig - ContextualBinds []BindingConfigContext - Compose ComposeConfig - Converters map[string]string - Accounts []AccountConfig `ini:"-"` - Filters []FilterConfig `ini:"-"` - Viewer ViewerConfig `ini:"-"` - Statusline StatuslineConfig `ini:"-"` - Triggers TriggersConfig `ini:"-"` - Ui UIConfig - ContextualUis []UIConfigContext - General GeneralConfig - Templates TemplateConfig - Openers map[string][]string + Bindings BindingConfig + Compose ComposeConfig + Converters map[string]string + Accounts []AccountConfig `ini:"-"` + Filters []FilterConfig `ini:"-"` + Viewer ViewerConfig `ini:"-"` + Statusline StatuslineConfig `ini:"-"` + Triggers TriggersConfig `ini:"-"` + Ui UIConfig + General GeneralConfig + Templates TemplateConfig + Openers map[string][]string } // Input: TimestampFormat @@ -133,17 +131,15 @@ func LoadConfigFromFile(root *string, accts []string) (*AercConfig, error) { } file.NameMapper = mapName config := &AercConfig{ - Bindings: defaultBindsConfig(), - ContextualBinds: []BindingConfigContext{}, - General: defaultGeneralConfig(), - Ui: defaultUiConfig(), - ContextualUis: []UIConfigContext{}, - Viewer: defaultViewerConfig(), - Statusline: defaultStatuslineConfig(), - Compose: defaultComposeConfig(), - Converters: make(map[string]string), - Templates: defaultTemplatesConfig(), - Openers: make(map[string][]string), + Bindings: defaultBindsConfig(), + General: defaultGeneralConfig(), + Ui: defaultUiConfig(), + Viewer: defaultViewerConfig(), + Statusline: defaultStatuslineConfig(), + Compose: defaultComposeConfig(), + Converters: make(map[string]string), + Templates: defaultTemplatesConfig(), + Openers: make(map[string][]string), } if err := config.parseGeneral(file); err != nil { diff --git a/config/ui.go b/config/ui.go index 9af47085..36b2a9a2 100644 --- a/config/ui.go +++ b/config/ui.go @@ -64,24 +64,32 @@ type UIConfig struct { ReverseOrder bool `ini:"reverse-msglist-order"` ReverseThreadOrder bool `ini:"reverse-thread-order"` SortThreadSiblings bool `ini:"sort-thread-siblings"` + + // private + contextualUis []*UiConfigContext + contextualCounts map[uiContextType]int + contextualCache map[uiContextKey]*UIConfig } -type ContextType int +type uiContextType int const ( - UI_CONTEXT_FOLDER ContextType = iota - UI_CONTEXT_ACCOUNT - UI_CONTEXT_SUBJECT - BIND_CONTEXT_ACCOUNT - BIND_CONTEXT_FOLDER + uiContextFolder uiContextType = iota + uiContextAccount + uiContextSubject ) -type UIConfigContext struct { - ContextType ContextType +type UiConfigContext struct { + ContextType uiContextType Regex *regexp.Regexp UiConfig UIConfig } +type uiContextKey struct { + ctxType uiContextType + value string +} + func defaultUiConfig() UIConfig { return UIConfig{ AutoMarkRead: true, @@ -122,6 +130,9 @@ func defaultUiConfig() UIConfig { // border defaults BorderCharVertical: ' ', BorderCharHorizontal: ' ', + // private + contextualCache: make(map[uiContextKey]*UIConfig), + contextualCounts: make(map[uiContextType]int), } } @@ -145,7 +156,7 @@ func (config *AercConfig) parseUi(file *ini.File) error { if err := uiSubConfig.parse(uiSection); err != nil { return err } - contextualUi := UIConfigContext{ + contextualUi := UiConfigContext{ UiConfig: uiSubConfig, } @@ -171,15 +182,16 @@ func (config *AercConfig) parseUi(file *ini.File) error { switch sectionName[3:index] { case "account": - contextualUi.ContextType = UI_CONTEXT_ACCOUNT + contextualUi.ContextType = uiContextAccount case "folder": - contextualUi.ContextType = UI_CONTEXT_FOLDER + contextualUi.ContextType = uiContextFolder case "subject": - contextualUi.ContextType = UI_CONTEXT_SUBJECT + contextualUi.ContextType = uiContextSubject default: return fmt.Errorf("Unknown Contextual Ui Section: %s", sectionName) } - config.ContextualUis = append(config.ContextualUis, contextualUi) + config.Ui.contextualUis = append(config.Ui.contextualUis, &contextualUi) + config.Ui.contextualCounts[contextualUi.ContextType]++ } // append default paths to styleset-dirs @@ -193,20 +205,20 @@ func (config *AercConfig) parseUi(file *ini.File) error { return err } - for idx, contextualUi := range config.ContextualUis { + for _, contextualUi := range config.Ui.contextualUis { if contextualUi.UiConfig.StyleSetName == "" && len(contextualUi.UiConfig.StyleSetDirs) == 0 { continue // no need to do anything if nothing is overridden } // fill in the missing part from the base if contextualUi.UiConfig.StyleSetName == "" { - config.ContextualUis[idx].UiConfig.StyleSetName = config.Ui.StyleSetName + contextualUi.UiConfig.StyleSetName = config.Ui.StyleSetName } else if len(contextualUi.UiConfig.StyleSetDirs) == 0 { - config.ContextualUis[idx].UiConfig.StyleSetDirs = config.Ui.StyleSetDirs + contextualUi.UiConfig.StyleSetDirs = config.Ui.StyleSetDirs } // since at least one of them has changed, load the styleset - if err := config.ContextualUis[idx].UiConfig.loadStyleSet( - config.ContextualUis[idx].UiConfig.StyleSetDirs); err != nil { + if err := contextualUi.UiConfig.loadStyleSet( + contextualUi.UiConfig.StyleSetDirs); err != nil { return err } } @@ -280,29 +292,30 @@ func (ui *UIConfig) loadStyleSet(styleSetDirs []string) error { return nil } -func (config *AercConfig) mergeContextualUi(baseUi UIConfig, - contextType ContextType, s string, -) UIConfig { - for _, contextualUi := range config.ContextualUis { +func (base *UIConfig) mergeContextual( + contextType uiContextType, s string, +) *UIConfig { + for _, contextualUi := range base.contextualUis { if contextualUi.ContextType != contextType { continue } - if !contextualUi.Regex.Match([]byte(s)) { continue } - - err := mergo.Merge(&baseUi, contextualUi.UiConfig, mergo.WithOverride) + // Try to make this as lightweight as possible and avoid copying + // the base UIConfig object unless necessary. + ui := *base + err := mergo.Merge(&ui, contextualUi.UiConfig, mergo.WithOverride) if err != nil { log.Warnf("merge ui failed: %v", err) } + ui.contextualCache = make(map[uiContextKey]*UIConfig) if contextualUi.UiConfig.StyleSetName != "" { - baseUi.style = contextualUi.UiConfig.style + ui.style = contextualUi.UiConfig.style } - return baseUi + return &ui } - - return baseUi + return base } func (uiConfig *UIConfig) GetStyle(so StyleObject) tcell.Style { @@ -325,16 +338,38 @@ func (uiConfig *UIConfig) GetComposedStyleSelected( return uiConfig.style.ComposeSelected(base, styles) } -func (config *AercConfig) GetUiConfig(params map[ContextType]string) *UIConfig { - baseUi := config.Ui - - for k, v := range params { - baseUi = config.mergeContextualUi(baseUi, k, v) +func (base *UIConfig) contextual( + ctxType uiContextType, value string, useCache bool, +) *UIConfig { + if base.contextualCounts[ctxType] == 0 { + // shortcut if no contextual ui for that type + return base + } + if !useCache { + return base.mergeContextual(ctxType, value) + } + key := uiContextKey{ctxType: ctxType, value: value} + c, found := base.contextualCache[key] + if !found { + c = base.mergeContextual(ctxType, value) + base.contextualCache[key] = c } + return c +} + +func (base *UIConfig) ForAccount(account string) *UIConfig { + return base.contextual(uiContextAccount, account, true) +} - return &baseUi +func (base *UIConfig) ForFolder(folder string) *UIConfig { + return base.contextual(uiContextFolder, folder, true) } -func (config *AercConfig) GetContextualUIConfigs() []UIConfigContext { - return config.ContextualUis +func (base *UIConfig) ForSubject(subject string) *UIConfig { + // TODO: this [ui:subject] contextual config should be dropped and + // replaced by another solution. Possibly something in the stylesets. + // Do not use a cache for contextual subject config as this + // could consume all available memory given enough time and + // enough messages. + return base.contextual(uiContextSubject, subject, false) } diff --git a/widgets/account.go b/widgets/account.go index c8c58b1e..838dd624 100644 --- a/widgets/account.go +++ b/widgets/account.go @@ -58,9 +58,7 @@ func (acct *AccountView) UiConfig() *config.UIConfig { func NewAccountView(aerc *Aerc, conf *config.AercConfig, acct *config.AccountConfig, host TabHost, deferLoop chan struct{}, ) (*AccountView, error) { - acctUiConf := conf.GetUiConfig(map[config.ContextType]string{ - config.UI_CONTEXT_ACCOUNT: acct.Name, - }) + acctUiConf := conf.Ui.ForAccount(acct.Name) view := &AccountView{ acct: acct, diff --git a/widgets/aerc.go b/widgets/aerc.go index ca82ee28..97e3c663 100644 --- a/widgets/aerc.go +++ b/widgets/aerc.go @@ -229,25 +229,30 @@ func (aerc *Aerc) getBindings() *config.KeyBindings { } switch view := aerc.SelectedTabContent().(type) { case *AccountView: - binds := aerc.conf.MergeContextualBinds(aerc.conf.Bindings.MessageList, config.BIND_CONTEXT_ACCOUNT, selectedAccountName, "messages") - return aerc.conf.MergeContextualBinds(binds, config.BIND_CONTEXT_FOLDER, view.SelectedDirectory(), "messages") + binds := aerc.conf.Bindings.MessageList.ForAccount(selectedAccountName) + return binds.ForFolder(view.SelectedDirectory()) case *AccountWizard: return aerc.conf.Bindings.AccountWizard case *Composer: switch view.Bindings() { case "compose::editor": - return aerc.conf.MergeContextualBinds(aerc.conf.Bindings.ComposeEditor, config.BIND_CONTEXT_ACCOUNT, selectedAccountName, "compose::editor") + return aerc.conf.Bindings.ComposeEditor.ForAccount( + selectedAccountName) case "compose::review": - return aerc.conf.MergeContextualBinds(aerc.conf.Bindings.ComposeReview, config.BIND_CONTEXT_ACCOUNT, selectedAccountName, "compose::review") + return aerc.conf.Bindings.ComposeReview.ForAccount( + selectedAccountName) default: - return aerc.conf.MergeContextualBinds(aerc.conf.Bindings.Compose, config.BIND_CONTEXT_ACCOUNT, selectedAccountName, "compose") + return aerc.conf.Bindings.Compose.ForAccount( + selectedAccountName) } case *MessageViewer: switch view.Bindings() { case "view::passthrough": - return aerc.conf.MergeContextualBinds(aerc.conf.Bindings.MessageViewPassthrough, config.BIND_CONTEXT_ACCOUNT, selectedAccountName, "view::passthrough") + return aerc.conf.Bindings.MessageViewPassthrough.ForAccount( + selectedAccountName) default: - return aerc.conf.MergeContextualBinds(aerc.conf.Bindings.MessageView, config.BIND_CONTEXT_ACCOUNT, selectedAccountName, "view") + return aerc.conf.Bindings.MessageView.ForAccount( + selectedAccountName) } case *Terminal: return aerc.conf.Bindings.Terminal diff --git a/widgets/compose.go b/widgets/compose.go index 37e09ccb..4cf7da16 100644 --- a/widgets/compose.go +++ b/widgets/compose.go @@ -1261,11 +1261,8 @@ type reviewMessage struct { } func newReviewMessage(composer *Composer, err error) *reviewMessage { - bindings := composer.config.MergeContextualBinds( - composer.config.Bindings.ComposeReview, - config.BIND_CONTEXT_ACCOUNT, + bindings := composer.config.Bindings.ComposeReview.ForAccount( composer.acctConfig.Name, - "compose::review", ) reviewCommands := [][]string{ diff --git a/widgets/dirlist.go b/widgets/dirlist.go index e4f867eb..0b41c024 100644 --- a/widgets/dirlist.go +++ b/widgets/dirlist.go @@ -8,7 +8,6 @@ import ( "regexp" "sort" "strings" - "sync" "time" "github.com/gdamore/tcell/v2" @@ -48,7 +47,6 @@ type DirectoryLister interface { } type DirectoryList struct { - sync.Mutex Scrollable aercConf *config.AercConfig acctConf *config.AccountConfig @@ -60,7 +58,6 @@ type DirectoryList struct { worker *types.Worker skipSelect context.Context skipSelectCancel context.CancelFunc - uiConf map[string]*config.UIConfig } func NewDirectoryList(conf *config.AercConfig, acctConf *config.AccountConfig, @@ -68,8 +65,6 @@ func NewDirectoryList(conf *config.AercConfig, acctConf *config.AccountConfig, ) DirectoryLister { ctx, cancel := context.WithCancel(context.Background()) - uiConfMap := make(map[string]*config.UIConfig) - dirlist := &DirectoryList{ aercConf: conf, acctConf: acctConf, @@ -77,7 +72,6 @@ func NewDirectoryList(conf *config.AercConfig, acctConf *config.AccountConfig, worker: worker, skipSelect: ctx, skipSelectCancel: cancel, - uiConf: uiConfMap, } uiConf := dirlist.UiConfig("") dirlist.spinner = NewSpinner(uiConf) @@ -91,20 +85,10 @@ func NewDirectoryList(conf *config.AercConfig, acctConf *config.AccountConfig, } func (dirlist *DirectoryList) UiConfig(dir string) *config.UIConfig { - dirlist.Lock() - defer dirlist.Unlock() if dir == "" { dir = dirlist.Selected() } - if ui, ok := dirlist.uiConf[dir]; ok { - return ui - } - ui := dirlist.aercConf.GetUiConfig(map[config.ContextType]string{ - config.UI_CONTEXT_ACCOUNT: dirlist.acctConf.Name, - config.UI_CONTEXT_FOLDER: dir, - }) - dirlist.uiConf[dir] = ui - return ui + return dirlist.aercConf.Ui.ForAccount(dirlist.acctConf.Name).ForFolder(dir) } func (dirlist *DirectoryList) List() []string { diff --git a/widgets/msglist.go b/widgets/msglist.go index 09ee705e..85b8f168 100644 --- a/widgets/msglist.go +++ b/widgets/msglist.go @@ -202,19 +202,9 @@ func (ml *MessageList) drawRow(textWidth int, ctx *ui.Context, uid uint32, row i // TODO deprecate subject contextual UIs? Only related setting is styleset, // should implement a better per-message styling method // Check if we have any applicable ContextualUIConfigs - confs := ml.aerc.conf.GetContextualUIConfigs() uiConfig := acct.Directories().UiConfig(store.DirInfo.Name) - for _, c := range confs { - if c.ContextType == config.UI_CONTEXT_SUBJECT && msg.Envelope != nil { - if c.Regex.Match([]byte(msg.Envelope.Subject)) { - confParams := map[config.ContextType]string{ - config.UI_CONTEXT_ACCOUNT: acct.AccountConfig().Name, - config.UI_CONTEXT_FOLDER: acct.Directories().Selected(), - config.UI_CONTEXT_SUBJECT: msg.Envelope.Subject, - } - uiConfig = ml.conf.GetUiConfig(confParams) - } - } + if msg.Envelope != nil { + uiConfig = uiConfig.ForSubject(msg.Envelope.Subject) } msg_styles := []config.StyleObject{} diff --git a/widgets/msgviewer.go b/widgets/msgviewer.go index 6c929578..875ff873 100644 --- a/widgets/msgviewer.go +++ b/widgets/msgviewer.go @@ -738,12 +738,7 @@ var noFilterConfiguredCommands = [][]string{ } func newNoFilterConfigured(pv *PartViewer) *ui.Grid { - bindings := pv.conf.MergeContextualBinds( - pv.conf.Bindings.MessageView, - config.BIND_CONTEXT_ACCOUNT, - pv.acctConfig.Name, - "view", - ) + bindings := pv.conf.Bindings.MessageView.ForAccount(pv.acctConfig.Name) var actions []string -- cgit