fix: address CodeRabbit review findings
- bootstrap/engine.go: make buildSaveConfigFunc consistent with buildKeybindingsHandler — return (func, error) and check eagerly - config_migration.go: add nil guard for a.deps.Config before accessing Update.NotifyOnNewSettings - main.go: add comment explaining GTK single-thread model for OnConfigChange (no mutex needed — callback fires on GTK main thread via glib.IdleAdd) - app.go: guard FaviconService nil check before NewFaviconAdapter - content/navigation.go: log.Warn when onLaunchExternalURL is nil instead of silently dropping the external URL - input/shortcuts.go: fix docstring — workspace may be nil (not must not be) - adapter/favicon.go: skip EnsureSizedPNG when normalizedIconSize == 0 - dispatcher/keyboard.go: guard empty newPaneURL in ActionNewTab handler - domain/entity/config_types.go: consolidate 5 identical GetKeyBindings implementations into shared keyBindingsFromActions helper - coordinator/workspace.go: clamp resizeStepPercent to (0,0.5] and resizeMinPanePercent to (0,0.5) with sensible defaults in constructor - webkit/handlers/register.go: document why KeybindingsHandler is required while other handlers are optional - port/engine.go: add detailed comment explaining why EngineSettingsUpdate.Raw remains any (moving config schema to domain would be a large refactor) - port/favicon_service.go: doc comment on EnsureDiskCache explaining fire-and-forget / intentional error suppression pattern
- commit
- 1dcf50ce97344ff64a2e414f0d81cb51717d5863
- author
- brice · · 2026-03-17 12:59 UTC
- parent
- 3489869
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
13 files · +100 -67
M cmd/dumber/main.go +3 -0
| @@ -568,6 +568,9 @@ func buildUIDependencies( | |||
| 568 | 568 | }, | |
| 569 | 569 | OnConfigChange: func(callback func()) { | |
| 570 | 570 | config.GetManager().OnConfigChange(func(newCfg *config.Config) { | |
| 571 | + | // No synchronization needed: viper fires this callback on the | |
| 572 | + | // GTK main thread (via glib.IdleAdd), and all callers of cfg run | |
| 573 | + | // on the same GTK main thread, so there is no concurrent access. | |
| 571 | 574 | if cfg != nil && newCfg != nil { | |
| 572 | 575 | *cfg = *newCfg | |
| 573 | 576 | } | |
| @@ -568,6 +568,9 @@ func buildUIDependencies( | |||||
| 568 | }, | 568 | }, | ||
| 569 | OnConfigChange: func(callback func()) { | 569 | OnConfigChange: func(callback func()) { | ||
| 570 | config.GetManager().OnConfigChange(func(newCfg *config.Config) { | 570 | config.GetManager().OnConfigChange(func(newCfg *config.Config) { | ||
| 571 | + | // No synchronization needed: viper fires this callback on the | |||
| 572 | + | // GTK main thread (via glib.IdleAdd), and all callers of cfg run | |||
| 573 | + | // on the same GTK main thread, so there is no concurrent access. | |||
| 571 | if cfg != nil && newCfg != nil { | 574 | if cfg != nil && newCfg != nil { | ||
| 572 | *cfg = *newCfg | 575 | *cfg = *newCfg | ||
| 573 | } | 576 | } | ||
M internal/application/port/engine.go +13 -5
| @@ -150,13 +150,21 @@ type Engine interface { | |||
| 150 | 150 | } | |
| 151 | 151 | ||
| 152 | 152 | // EngineSettingsUpdate carries a runtime config change to the engine. | |
| 153 | - | // Raw holds the implementation-specific config object; the engine type-asserts | |
| 154 | - | // to its concrete type (e.g. *config.Config for the WebKit engine). | |
| 155 | - | // This is the only intentional use of `any` in the Engine port. | |
| 153 | + | // | |
| 154 | + | // The Raw field holds the implementation-specific config object; each engine | |
| 155 | + | // implementation type-asserts it to its concrete config type (e.g. the WebKit | |
| 156 | + | // engine asserts to *infrastructure/config.Config). | |
| 157 | + | // | |
| 158 | + | // This is the only intentional use of `any` in the Engine port. A fully typed | |
| 159 | + | // approach would require either moving the entire config schema into the domain | |
| 160 | + | // layer (a large refactor) or defining a SettingsProvider interface — which | |
| 161 | + | // would itself reduce to `any` because callers would embed engine-specific data | |
| 162 | + | // behind an empty interface. Until the config schema is promoted to domain, | |
| 163 | + | // the type assertion in the engine implementation is the least-invasive option. | |
| 156 | 164 | type EngineSettingsUpdate struct { | |
| 157 | 165 | // Raw holds the implementation-specific config. | |
| 158 | - | // WebKit engine expects *config.Config. | |
| 159 | - | Raw any | |
| 166 | + | // WebKit engine expects *infrastructure/config.Config. | |
| 167 | + | Raw any //nolint:iface // intentional: see type comment above | |
| 160 | 168 | } | |
| 161 | 169 | ||
| 162 | 170 | // HandlerDependencies holds use cases needed by WebUI message handlers. | |
| @@ -150,13 +150,21 @@ type Engine interface { | |||||
| 150 | } | 150 | } | ||
| 151 | 151 | ||||
| 152 | // EngineSettingsUpdate carries a runtime config change to the engine. | 152 | // EngineSettingsUpdate carries a runtime config change to the engine. | ||
| 153 | - | // Raw holds the implementation-specific config object; the engine type-asserts | 153 | + | // |
| 154 | - | // to its concrete type (e.g. *config.Config for the WebKit engine). | 154 | + | // The Raw field holds the implementation-specific config object; each engine |
| 155 | - | // This is the only intentional use of `any` in the Engine port. | 155 | + | // implementation type-asserts it to its concrete config type (e.g. the WebKit |
| 156 | + | // engine asserts to *infrastructure/config.Config). | |||
| 157 | + | // | |||
| 158 | + | // This is the only intentional use of `any` in the Engine port. A fully typed | |||
| 159 | + | // approach would require either moving the entire config schema into the domain | |||
| 160 | + | // layer (a large refactor) or defining a SettingsProvider interface — which | |||
| 161 | + | // would itself reduce to `any` because callers would embed engine-specific data | |||
| 162 | + | // behind an empty interface. Until the config schema is promoted to domain, | |||
| 163 | + | // the type assertion in the engine implementation is the least-invasive option. | |||
| 156 | type EngineSettingsUpdate struct { | 164 | type EngineSettingsUpdate struct { | ||
| 157 | // Raw holds the implementation-specific config. | 165 | // Raw holds the implementation-specific config. | ||
| 158 | - | // WebKit engine expects *config.Config. | 166 | + | // WebKit engine expects *infrastructure/config.Config. |
| 159 | - | Raw any | 167 | + | Raw any //nolint:iface // intentional: see type comment above |
| 160 | } | 168 | } | ||
| 161 | 169 | ||||
| 162 | // HandlerDependencies holds use cases needed by WebUI message handlers. | 170 | // HandlerDependencies holds use cases needed by WebUI message handlers. | ||
M internal/application/port/favicon_service.go +3 -0
| @@ -11,6 +11,9 @@ type FaviconService interface { | |||
| 11 | 11 | HasPNGSizedOnDisk(domain string, size int) bool | |
| 12 | 12 | EnsureSizedPNG(ctx context.Context, domain string, size int) error | |
| 13 | 13 | EnsureCacheDir() error | |
| 14 | + | // EnsureDiskCache ensures a favicon is written to the on-disk cache for the given | |
| 15 | + | // domain. It is intentionally fire-and-forget: any errors are logged internally by | |
| 16 | + | // the implementation and are not returned to the caller. | |
| 14 | 17 | EnsureDiskCache(ctx context.Context, domain string) | |
| 15 | 18 | Close() | |
| 16 | 19 | } | |
| @@ -11,6 +11,9 @@ type FaviconService interface { | |||||
| 11 | HasPNGSizedOnDisk(domain string, size int) bool | 11 | HasPNGSizedOnDisk(domain string, size int) bool | ||
| 12 | EnsureSizedPNG(ctx context.Context, domain string, size int) error | 12 | EnsureSizedPNG(ctx context.Context, domain string, size int) error | ||
| 13 | EnsureCacheDir() error | 13 | EnsureCacheDir() error | ||
| 14 | + | // EnsureDiskCache ensures a favicon is written to the on-disk cache for the given | |||
| 15 | + | // domain. It is intentionally fire-and-forget: any errors are logged internally by | |||
| 16 | + | // the implementation and are not returned to the caller. | |||
| 14 | EnsureDiskCache(ctx context.Context, domain string) | 17 | EnsureDiskCache(ctx context.Context, domain string) | ||
| 15 | Close() | 18 | Close() | ||
| 16 | } | 19 | } | ||
M internal/bootstrap/engine.go +9 -7
| @@ -48,7 +48,10 @@ func BuildEngine(input EngineInput) (port.Engine, error) { | |||
| 48 | 48 | } | |
| 49 | 49 | ||
| 50 | 50 | // Pre-build config save function for handler registration. | |
| 51 | - | saveConfigFunc := buildSaveConfigFunc() | |
| 51 | + | saveConfigFunc, err := buildSaveConfigFunc() | |
| 52 | + | if err != nil { | |
| 53 | + | return nil, fmt.Errorf("failed to build save config func: %w", err) | |
| 54 | + | } | |
| 52 | 55 | ||
| 53 | 56 | return webkit.NewEngine( | |
| 54 | 57 | input.Ctx, cfg, opts, wkCfg, | |
| @@ -93,14 +96,13 @@ func buildKeybindingsHandler() (*handlers.KeybindingsHandler, error) { | |||
| 93 | 96 | } | |
| 94 | 97 | ||
| 95 | 98 | // buildSaveConfigFunc constructs the config save function using the config manager. | |
| 96 | - | func buildSaveConfigFunc() func(context.Context, port.WebUIConfig) error { | |
| 99 | + | // It returns an error immediately if the config manager is not initialized, consistent | |
| 100 | + | // with buildKeybindingsHandler. | |
| 101 | + | func buildSaveConfigFunc() (func(context.Context, port.WebUIConfig) error, error) { | |
| 97 | 102 | mgr := config.GetManager() | |
| 98 | 103 | if mgr == nil { | |
| 99 | - | // Return a function that errors — config manager may not be ready yet. | |
| 100 | - | return func(_ context.Context, _ port.WebUIConfig) error { | |
| 101 | - | return fmt.Errorf("config manager not initialized") | |
| 102 | - | } | |
| 104 | + | return nil, fmt.Errorf("config manager not initialized") | |
| 103 | 105 | } | |
| 104 | 106 | uc := usecase.NewSaveWebUIConfigUseCase(config.NewWebUIConfigGateway(mgr)) | |
| 105 | - | return uc.Execute | |
| 107 | + | return uc.Execute, nil | |
| 106 | 108 | } | |
| @@ -48,7 +48,10 @@ func BuildEngine(input EngineInput) (port.Engine, error) { | |||||
| 48 | } | 48 | } | ||
| 49 | 49 | ||||
| 50 | // Pre-build config save function for handler registration. | 50 | // Pre-build config save function for handler registration. | ||
| 51 | - | saveConfigFunc := buildSaveConfigFunc() | 51 | + | saveConfigFunc, err := buildSaveConfigFunc() |
| 52 | + | if err != nil { | |||
| 53 | + | return nil, fmt.Errorf("failed to build save config func: %w", err) | |||
| 54 | + | } | |||
| 52 | 55 | ||||
| 53 | return webkit.NewEngine( | 56 | return webkit.NewEngine( | ||
| 54 | input.Ctx, cfg, opts, wkCfg, | 57 | input.Ctx, cfg, opts, wkCfg, | ||
| @@ -93,14 +96,13 @@ func buildKeybindingsHandler() (*handlers.KeybindingsHandler, error) { | |||||
| 93 | } | 96 | } | ||
| 94 | 97 | ||||
| 95 | // buildSaveConfigFunc constructs the config save function using the config manager. | 98 | // buildSaveConfigFunc constructs the config save function using the config manager. | ||
| 96 | - | func buildSaveConfigFunc() func(context.Context, port.WebUIConfig) error { | 99 | + | // It returns an error immediately if the config manager is not initialized, consistent |
| 100 | + | // with buildKeybindingsHandler. | |||
| 101 | + | func buildSaveConfigFunc() (func(context.Context, port.WebUIConfig) error, error) { | |||
| 97 | mgr := config.GetManager() | 102 | mgr := config.GetManager() | ||
| 98 | if mgr == nil { | 103 | if mgr == nil { | ||
| 99 | - | // Return a function that errors — config manager may not be ready yet. | 104 | + | return nil, fmt.Errorf("config manager not initialized") |
| 100 | - | return func(_ context.Context, _ port.WebUIConfig) error { | |||
| 101 | - | return fmt.Errorf("config manager not initialized") | |||
| 102 | - | } | |||
| 103 | } | 105 | } | ||
| 104 | uc := usecase.NewSaveWebUIConfigUseCase(config.NewWebUIConfigGateway(mgr)) | 106 | uc := usecase.NewSaveWebUIConfigUseCase(config.NewWebUIConfigGateway(mgr)) | ||
| 105 | - | return uc.Execute | 107 | + | return uc.Execute, nil |
| 106 | } | 108 | } | ||
M internal/domain/entity/config_types.go +13 -31
| @@ -39,10 +39,11 @@ type PaneModeConfig struct { | |||
| 39 | 39 | Actions map[string]ActionBinding `mapstructure:"actions" yaml:"actions" toml:"actions" json:"actions"` | |
| 40 | 40 | } | |
| 41 | 41 | ||
| 42 | - | // GetKeyBindings returns a map from key string to action name. | |
| 43 | - | func (p *PaneModeConfig) GetKeyBindings() map[string]string { | |
| 42 | + | // keyBindingsFromActions converts an action binding map to a key→action map. | |
| 43 | + | // This is the canonical implementation shared by all GetKeyBindings methods. | |
| 44 | + | func keyBindingsFromActions(actions map[string]ActionBinding) map[string]string { | |
| 44 | 45 | keyToAction := make(map[string]string) | |
| 45 | - | for action, binding := range p.Actions { | |
| 46 | + | for action, binding := range actions { | |
| 46 | 47 | for _, key := range binding.Keys { | |
| 47 | 48 | keyToAction[key] = action | |
| 48 | 49 | } | |
| @@ -50,6 +51,11 @@ func (p *PaneModeConfig) GetKeyBindings() map[string]string { | |||
| 50 | 51 | return keyToAction | |
| 51 | 52 | } | |
| 52 | 53 | ||
| 54 | + | // GetKeyBindings returns a map from key string to action name. | |
| 55 | + | func (p *PaneModeConfig) GetKeyBindings() map[string]string { | |
| 56 | + | return keyBindingsFromActions(p.Actions) | |
| 57 | + | } | |
| 58 | + | ||
| 53 | 59 | // TabModeConfig holds tab mode shortcut configuration. | |
| 54 | 60 | type TabModeConfig struct { | |
| 55 | 61 | ActivationShortcut string `mapstructure:"activation_shortcut" yaml:"activation_shortcut" toml:"activation_shortcut" json:"activation_shortcut"` //nolint:lll // struct tags must stay on one line | |
| @@ -59,13 +65,7 @@ type TabModeConfig struct { | |||
| 59 | 65 | ||
| 60 | 66 | // GetKeyBindings returns a map from key string to action name. | |
| 61 | 67 | func (t *TabModeConfig) GetKeyBindings() map[string]string { | |
| 62 | - | keyToAction := make(map[string]string) | |
| 63 | - | for action, binding := range t.Actions { | |
| 64 | - | for _, key := range binding.Keys { | |
| 65 | - | keyToAction[key] = action | |
| 66 | - | } | |
| 67 | - | } | |
| 68 | - | return keyToAction | |
| 68 | + | return keyBindingsFromActions(t.Actions) | |
| 69 | 69 | } | |
| 70 | 70 | ||
| 71 | 71 | // ResizeModeConfig holds resize mode shortcut configuration. | |
| @@ -79,13 +79,7 @@ type ResizeModeConfig struct { | |||
| 79 | 79 | ||
| 80 | 80 | // GetKeyBindings returns a map from key string to action name. | |
| 81 | 81 | func (r *ResizeModeConfig) GetKeyBindings() map[string]string { | |
| 82 | - | keyToAction := make(map[string]string) | |
| 83 | - | for action, binding := range r.Actions { | |
| 84 | - | for _, key := range binding.Keys { | |
| 85 | - | keyToAction[key] = action | |
| 86 | - | } | |
| 87 | - | } | |
| 88 | - | return keyToAction | |
| 82 | + | return keyBindingsFromActions(r.Actions) | |
| 89 | 83 | } | |
| 90 | 84 | ||
| 91 | 85 | // SessionModeConfig holds session mode shortcut configuration. | |
| @@ -97,13 +91,7 @@ type SessionModeConfig struct { | |||
| 97 | 91 | ||
| 98 | 92 | // GetKeyBindings returns a map from key string to action name. | |
| 99 | 93 | func (s *SessionModeConfig) GetKeyBindings() map[string]string { | |
| 100 | - | keyToAction := make(map[string]string) | |
| 101 | - | for action, binding := range s.Actions { | |
| 102 | - | for _, key := range binding.Keys { | |
| 103 | - | keyToAction[key] = action | |
| 104 | - | } | |
| 105 | - | } | |
| 106 | - | return keyToAction | |
| 94 | + | return keyBindingsFromActions(s.Actions) | |
| 107 | 95 | } | |
| 108 | 96 | ||
| 109 | 97 | // SessionConfig holds session persistence and restoration settings. | |
| @@ -126,13 +114,7 @@ type GlobalShortcutsConfig struct { | |||
| 126 | 114 | ||
| 127 | 115 | // GetKeyBindings returns a map from key string to action name. | |
| 128 | 116 | func (g *GlobalShortcutsConfig) GetKeyBindings() map[string]string { | |
| 129 | - | keyToAction := make(map[string]string) | |
| 130 | - | for action, binding := range g.Actions { | |
| 131 | - | for _, key := range binding.Keys { | |
| 132 | - | keyToAction[key] = action | |
| 133 | - | } | |
| 134 | - | } | |
| 135 | - | return keyToAction | |
| 117 | + | return keyBindingsFromActions(g.Actions) | |
| 136 | 118 | } | |
| 137 | 119 | ||
| 138 | 120 | // FloatingPaneProfile defines a named floating pane preset. | |
| @@ -39,10 +39,11 @@ type PaneModeConfig struct { | |||||
| 39 | Actions map[string]ActionBinding `mapstructure:"actions" yaml:"actions" toml:"actions" json:"actions"` | 39 | Actions map[string]ActionBinding `mapstructure:"actions" yaml:"actions" toml:"actions" json:"actions"` | ||
| 40 | } | 40 | } | ||
| 41 | 41 | ||||
| 42 | - | // GetKeyBindings returns a map from key string to action name. | 42 | + | // keyBindingsFromActions converts an action binding map to a key→action map. |
| 43 | - | func (p *PaneModeConfig) GetKeyBindings() map[string]string { | 43 | + | // This is the canonical implementation shared by all GetKeyBindings methods. |
| 44 | + | func keyBindingsFromActions(actions map[string]ActionBinding) map[string]string { | |||
| 44 | keyToAction := make(map[string]string) | 45 | keyToAction := make(map[string]string) | ||
| 45 | - | for action, binding := range p.Actions { | 46 | + | for action, binding := range actions { |
| 46 | for _, key := range binding.Keys { | 47 | for _, key := range binding.Keys { | ||
| 47 | keyToAction[key] = action | 48 | keyToAction[key] = action | ||
| 48 | } | 49 | } | ||
| @@ -50,6 +51,11 @@ func (p *PaneModeConfig) GetKeyBindings() map[string]string { | |||||
| 50 | return keyToAction | 51 | return keyToAction | ||
| 51 | } | 52 | } | ||
| 52 | 53 | ||||
| 54 | + | // GetKeyBindings returns a map from key string to action name. | |||
| 55 | + | func (p *PaneModeConfig) GetKeyBindings() map[string]string { | |||
| 56 | + | return keyBindingsFromActions(p.Actions) | |||
| 57 | + | } | |||
| 58 | + | ||||
| 53 | // TabModeConfig holds tab mode shortcut configuration. | 59 | // TabModeConfig holds tab mode shortcut configuration. | ||
| 54 | type TabModeConfig struct { | 60 | type TabModeConfig struct { | ||
| 55 | ActivationShortcut string `mapstructure:"activation_shortcut" yaml:"activation_shortcut" toml:"activation_shortcut" json:"activation_shortcut"` //nolint:lll // struct tags must stay on one line | 61 | ActivationShortcut string `mapstructure:"activation_shortcut" yaml:"activation_shortcut" toml:"activation_shortcut" json:"activation_shortcut"` //nolint:lll // struct tags must stay on one line | ||
| @@ -59,13 +65,7 @@ type TabModeConfig struct { | |||||
| 59 | 65 | ||||
| 60 | // GetKeyBindings returns a map from key string to action name. | 66 | // GetKeyBindings returns a map from key string to action name. | ||
| 61 | func (t *TabModeConfig) GetKeyBindings() map[string]string { | 67 | func (t *TabModeConfig) GetKeyBindings() map[string]string { | ||
| 62 | - | keyToAction := make(map[string]string) | 68 | + | return keyBindingsFromActions(t.Actions) |
| 63 | - | for action, binding := range t.Actions { | |||
| 64 | - | for _, key := range binding.Keys { | |||
| 65 | - | keyToAction[key] = action | |||
| 66 | - | } | |||
| 67 | - | } | |||
| 68 | - | return keyToAction | |||
| 69 | } | 69 | } | ||
| 70 | 70 | ||||
| 71 | // ResizeModeConfig holds resize mode shortcut configuration. | 71 | // ResizeModeConfig holds resize mode shortcut configuration. | ||
| @@ -79,13 +79,7 @@ type ResizeModeConfig struct { | |||||
| 79 | 79 | ||||
| 80 | // GetKeyBindings returns a map from key string to action name. | 80 | // GetKeyBindings returns a map from key string to action name. | ||
| 81 | func (r *ResizeModeConfig) GetKeyBindings() map[string]string { | 81 | func (r *ResizeModeConfig) GetKeyBindings() map[string]string { | ||
| 82 | - | keyToAction := make(map[string]string) | 82 | + | return keyBindingsFromActions(r.Actions) |
| 83 | - | for action, binding := range r.Actions { | |||
| 84 | - | for _, key := range binding.Keys { | |||
| 85 | - | keyToAction[key] = action | |||
| 86 | - | } | |||
| 87 | - | } | |||
| 88 | - | return keyToAction | |||
| 89 | } | 83 | } | ||
| 90 | 84 | ||||
| 91 | // SessionModeConfig holds session mode shortcut configuration. | 85 | // SessionModeConfig holds session mode shortcut configuration. | ||
| @@ -97,13 +91,7 @@ type SessionModeConfig struct { | |||||
| 97 | 91 | ||||
| 98 | // GetKeyBindings returns a map from key string to action name. | 92 | // GetKeyBindings returns a map from key string to action name. | ||
| 99 | func (s *SessionModeConfig) GetKeyBindings() map[string]string { | 93 | func (s *SessionModeConfig) GetKeyBindings() map[string]string { | ||
| 100 | - | keyToAction := make(map[string]string) | 94 | + | return keyBindingsFromActions(s.Actions) |
| 101 | - | for action, binding := range s.Actions { | |||
| 102 | - | for _, key := range binding.Keys { | |||
| 103 | - | keyToAction[key] = action | |||
| 104 | - | } | |||
| 105 | - | } | |||
| 106 | - | return keyToAction | |||
| 107 | } | 95 | } | ||
| 108 | 96 | ||||
| 109 | // SessionConfig holds session persistence and restoration settings. | 97 | // SessionConfig holds session persistence and restoration settings. | ||
| @@ -126,13 +114,7 @@ type GlobalShortcutsConfig struct { | |||||
| 126 | 114 | ||||
| 127 | // GetKeyBindings returns a map from key string to action name. | 115 | // GetKeyBindings returns a map from key string to action name. | ||
| 128 | func (g *GlobalShortcutsConfig) GetKeyBindings() map[string]string { | 116 | func (g *GlobalShortcutsConfig) GetKeyBindings() map[string]string { | ||
| 129 | - | keyToAction := make(map[string]string) | 117 | + | return keyBindingsFromActions(g.Actions) |
| 130 | - | for action, binding := range g.Actions { | |||
| 131 | - | for _, key := range binding.Keys { | |||
| 132 | - | keyToAction[key] = action | |||
| 133 | - | } | |||
| 134 | - | } | |||
| 135 | - | return keyToAction | |||
| 136 | } | 118 | } | ||
| 137 | 119 | ||||
| 138 | // FloatingPaneProfile defines a named floating pane preset. | 120 | // FloatingPaneProfile defines a named floating pane preset. | ||
M internal/infrastructure/webkit/handlers/register.go +9 -7
| @@ -18,13 +18,15 @@ type AccentKeyHandler = port.AccentKeyHandler | |||
| 18 | 18 | ||
| 19 | 19 | // Config holds all dependencies for message handlers. | |
| 20 | 20 | type Config struct { | |
| 21 | - | HistoryUC port.HomepageHistory | |
| 22 | - | FavoritesUC port.HomepageFavorites | |
| 23 | - | Clipboard port.Clipboard | |
| 24 | - | AutoCopyConfig port.AutoCopyConfig | |
| 25 | - | SaveConfig func(context.Context, port.WebUIConfig) error // Pre-built by bootstrap (usecase.SaveWebUIConfigUseCase.Execute) | |
| 26 | - | KeybindingsHandler *KeybindingsHandler // Pre-built by bootstrap | |
| 27 | - | OnClipboardCopied func(textLen int) // Called when auto-copy completes (for toast notification) | |
| 21 | + | HistoryUC port.HomepageHistory | |
| 22 | + | FavoritesUC port.HomepageFavorites | |
| 23 | + | Clipboard port.Clipboard | |
| 24 | + | AutoCopyConfig port.AutoCopyConfig | |
| 25 | + | SaveConfig func(context.Context, port.WebUIConfig) error // Pre-built by bootstrap (usecase.SaveWebUIConfigUseCase.Execute) | |
| 26 | + | // KeybindingsHandler is required (not optional like other handlers) because the WebUI | |
| 27 | + | // always needs keybinding read/write support regardless of configuration. | |
| 28 | + | KeybindingsHandler *KeybindingsHandler // Pre-built by bootstrap | |
| 29 | + | OnClipboardCopied func(textLen int) // Called when auto-copy completes (for toast notification) | |
| 28 | 30 | } | |
| 29 | 31 | ||
| 30 | 32 | // RegisterAll registers all message handlers with the router. | |
| @@ -18,13 +18,15 @@ type AccentKeyHandler = port.AccentKeyHandler | |||||
| 18 | 18 | ||||
| 19 | // Config holds all dependencies for message handlers. | 19 | // Config holds all dependencies for message handlers. | ||
| 20 | type Config struct { | 20 | type Config struct { | ||
| 21 | - | HistoryUC port.HomepageHistory | 21 | + | HistoryUC port.HomepageHistory |
| 22 | - | FavoritesUC port.HomepageFavorites | 22 | + | FavoritesUC port.HomepageFavorites |
| 23 | - | Clipboard port.Clipboard | 23 | + | Clipboard port.Clipboard |
| 24 | - | AutoCopyConfig port.AutoCopyConfig | 24 | + | AutoCopyConfig port.AutoCopyConfig |
| 25 | - | SaveConfig func(context.Context, port.WebUIConfig) error // Pre-built by bootstrap (usecase.SaveWebUIConfigUseCase.Execute) | 25 | + | SaveConfig func(context.Context, port.WebUIConfig) error // Pre-built by bootstrap (usecase.SaveWebUIConfigUseCase.Execute) |
| 26 | - | KeybindingsHandler *KeybindingsHandler // Pre-built by bootstrap | 26 | + | // KeybindingsHandler is required (not optional like other handlers) because the WebUI |
| 27 | - | OnClipboardCopied func(textLen int) // Called when auto-copy completes (for toast notification) | 27 | + | // always needs keybinding read/write support regardless of configuration. |
| 28 | + | KeybindingsHandler *KeybindingsHandler // Pre-built by bootstrap | |||
| 29 | + | OnClipboardCopied func(textLen int) // Called when auto-copy completes (for toast notification) | |||
| 28 | } | 30 | } | ||
| 29 | 31 | ||||
| 30 | // RegisterAll registers all message handlers with the router. | 32 | // RegisterAll registers all message handlers with the router. | ||
M internal/ui/adapter/favicon.go +13 -11
| @@ -246,17 +246,19 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||
| 246 | 246 | a.clearWarningDedup(savePNGWarnKey) | |
| 247 | 247 | log.Debug().Str("domain", domain).Str("path", pngPath).Msg("saved favicon PNG") | |
| 248 | 248 | ||
| 249 | - | // Create normalized sized copy for dmenu/fuzzel (async) | |
| 250 | - | go func() { | |
| 251 | - | if err := a.service.EnsureSizedPNG(ctx, domain, a.normalizedIconSize); err != nil { | |
| 252 | - | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(log *zerolog.Logger, warnErr error) { | |
| 253 | - | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") | |
| 254 | - | }) | |
| 255 | - | } else { | |
| 256 | - | a.clearWarningDedup(sizedPNGWarnKey) | |
| 257 | - | log.Debug().Str("domain", domain).Msg("created sized PNG") | |
| 258 | - | } | |
| 259 | - | }() | |
| 249 | + | // Create normalized sized copy for dmenu/fuzzel (async, only if size is set) | |
| 250 | + | if a.normalizedIconSize > 0 { | |
| 251 | + | go func() { | |
| 252 | + | if err := a.service.EnsureSizedPNG(ctx, domain, a.normalizedIconSize); err != nil { | |
| 253 | + | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(log *zerolog.Logger, warnErr error) { | |
| 254 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") | |
| 255 | + | }) | |
| 256 | + | } else { | |
| 257 | + | a.clearWarningDedup(sizedPNGWarnKey) | |
| 258 | + | log.Debug().Str("domain", domain).Msg("created sized PNG") | |
| 259 | + | } | |
| 260 | + | }() | |
| 261 | + | } | |
| 260 | 262 | } | |
| 261 | 263 | return false // don't repeat | |
| 262 | 264 | }) | |
| @@ -246,17 +246,19 @@ func (a *FaviconAdapter) saveFaviconToDisk(ctx context.Context, domain string, t | |||||
| 246 | a.clearWarningDedup(savePNGWarnKey) | 246 | a.clearWarningDedup(savePNGWarnKey) | ||
| 247 | log.Debug().Str("domain", domain).Str("path", pngPath).Msg("saved favicon PNG") | 247 | log.Debug().Str("domain", domain).Str("path", pngPath).Msg("saved favicon PNG") | ||
| 248 | 248 | ||||
| 249 | - | // Create normalized sized copy for dmenu/fuzzel (async) | 249 | + | // Create normalized sized copy for dmenu/fuzzel (async, only if size is set) |
| 250 | - | go func() { | 250 | + | if a.normalizedIconSize > 0 { |
| 251 | - | if err := a.service.EnsureSizedPNG(ctx, domain, a.normalizedIconSize); err != nil { | 251 | + | go func() { |
| 252 | - | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(log *zerolog.Logger, warnErr error) { | 252 | + | if err := a.service.EnsureSizedPNG(ctx, domain, a.normalizedIconSize); err != nil { |
| 253 | - | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") | 253 | + | a.logWarningDedup(ctx, sizedPNGWarnKey, err, func(log *zerolog.Logger, warnErr error) { |
| 254 | - | }) | 254 | + | log.Warn().Err(warnErr).Str("domain", domain).Msg("failed to create sized PNG") |
| 255 | - | } else { | 255 | + | }) |
| 256 | - | a.clearWarningDedup(sizedPNGWarnKey) | 256 | + | } else { |
| 257 | - | log.Debug().Str("domain", domain).Msg("created sized PNG") | 257 | + | a.clearWarningDedup(sizedPNGWarnKey) |
| 258 | - | } | 258 | + | log.Debug().Str("domain", domain).Msg("created sized PNG") |
| 259 | - | }() | 259 | + | } |
| 260 | + | }() | |||
| 261 | + | } | |||
| 260 | } | 262 | } | ||
| 261 | return false // don't repeat | 263 | return false // don't repeat | ||
| 262 | }) | 264 | }) | ||
M internal/ui/app.go +5 -2
| @@ -1521,8 +1521,11 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 1521 | 1521 | return a.activeWorkspace(), a.activeWorkspaceView() | |
| 1522 | 1522 | } | |
| 1523 | 1523 | ||
| 1524 | - | // Create FaviconAdapter with service and engine FaviconDatabase | |
| 1525 | - | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | |
| 1524 | + | // Create FaviconAdapter with service and engine FaviconDatabase. | |
| 1525 | + | // Skip if FaviconService is nil (e.g. in tests or when favicon support is disabled). | |
| 1526 | + | if a.deps.FaviconService != nil { | |
| 1527 | + | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | |
| 1528 | + | } | |
| 1526 | 1529 | ||
| 1527 | 1530 | // 1. Content Coordinator (no dependencies on other coordinators) | |
| 1528 | 1531 | a.contentCoord = content.NewCoordinator( | |
| @@ -1521,8 +1521,11 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 1521 | return a.activeWorkspace(), a.activeWorkspaceView() | 1521 | return a.activeWorkspace(), a.activeWorkspaceView() | ||
| 1522 | } | 1522 | } | ||
| 1523 | 1523 | ||||
| 1524 | - | // Create FaviconAdapter with service and engine FaviconDatabase | 1524 | + | // Create FaviconAdapter with service and engine FaviconDatabase. |
| 1525 | - | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | 1525 | + | // Skip if FaviconService is nil (e.g. in tests or when favicon support is disabled). |
| 1526 | + | if a.deps.FaviconService != nil { | |||
| 1527 | + | a.faviconAdapter = adapter.NewFaviconAdapter(a.deps.FaviconService, a.engine.FaviconDatabase(), a.deps.FaviconAdapterConfig) | |||
| 1528 | + | } | |||
| 1526 | 1529 | ||||
| 1527 | // 1. Content Coordinator (no dependencies on other coordinators) | 1530 | // 1. Content Coordinator (no dependencies on other coordinators) | ||
| 1528 | a.contentCoord = content.NewCoordinator( | 1531 | a.contentCoord = content.NewCoordinator( | ||
M internal/ui/config_migration.go +1 -1
| @@ -32,7 +32,7 @@ func (a *App) checkConfigMigration(ctx context.Context) { | |||
| 32 | 32 | Msg("config migration available") | |
| 33 | 33 | ||
| 34 | 34 | // Show toast notification if enabled | |
| 35 | - | if a.deps.Config.Update.NotifyOnNewSettings && a.appToaster != nil { | |
| 35 | + | if a.deps.Config != nil && a.deps.Config.Update.NotifyOnNewSettings && a.appToaster != nil { | |
| 36 | 36 | msg := fmt.Sprintf("Config has %d new settings. Run 'dumber config migrate'", len(result.MissingKeys)) | |
| 37 | 37 | a.appToaster.Show(ctx, msg, component.ToastInfo) | |
| 38 | 38 | } | |
| @@ -32,7 +32,7 @@ func (a *App) checkConfigMigration(ctx context.Context) { | |||||
| 32 | Msg("config migration available") | 32 | Msg("config migration available") | ||
| 33 | 33 | ||||
| 34 | // Show toast notification if enabled | 34 | // Show toast notification if enabled | ||
| 35 | - | if a.deps.Config.Update.NotifyOnNewSettings && a.appToaster != nil { | 35 | + | if a.deps.Config != nil && a.deps.Config.Update.NotifyOnNewSettings && a.appToaster != nil { |
| 36 | msg := fmt.Sprintf("Config has %d new settings. Run 'dumber config migrate'", len(result.MissingKeys)) | 36 | msg := fmt.Sprintf("Config has %d new settings. Run 'dumber config migrate'", len(result.MissingKeys)) | ||
| 37 | a.appToaster.Show(ctx, msg, component.ToastInfo) | 37 | a.appToaster.Show(ctx, msg, component.ToastInfo) | ||
| 38 | } | 38 | } | ||
M internal/ui/coordinator/content/navigation.go +2 -0
| @@ -326,6 +326,8 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||
| 326 | 326 | // Launch externally via injected callback | |
| 327 | 327 | if c.onLaunchExternalURL != nil { | |
| 328 | 328 | c.onLaunchExternalURL(uri) | |
| 329 | + | } else { | |
| 330 | + | log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external URL not launched: no handler registered") | |
| 329 | 331 | } | |
| 330 | 332 | ||
| 331 | 333 | // Stop loading to prevent WebKit from showing an error page | |
| @@ -326,6 +326,8 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID | |||||
| 326 | // Launch externally via injected callback | 326 | // Launch externally via injected callback | ||
| 327 | if c.onLaunchExternalURL != nil { | 327 | if c.onLaunchExternalURL != nil { | ||
| 328 | c.onLaunchExternalURL(uri) | 328 | c.onLaunchExternalURL(uri) | ||
| 329 | + | } else { | |||
| 330 | + | log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Msg("external URL not launched: no handler registered") | |||
| 329 | } | 331 | } | ||
| 330 | 332 | ||||
| 331 | // Stop loading to prevent WebKit from showing an error page | 333 | // Stop loading to prevent WebKit from showing an error page | ||
M internal/ui/coordinator/workspace.go +23 -2
| @@ -83,6 +83,27 @@ type incrementalCloseContext struct { | |||
| 83 | 83 | precheckReason string | |
| 84 | 84 | } | |
| 85 | 85 | ||
| 86 | + | const ( | |
| 87 | + | defaultResizeStepPercent = 0.05 | |
| 88 | + | defaultResizeMinPanePercent = 0.1 | |
| 89 | + | ) | |
| 90 | + | ||
| 91 | + | // clampResizeStep clamps resizeStepPercent to (0, 0.5], returning the default (0.05) if out of range. | |
| 92 | + | func clampResizeStep(v float64) float64 { | |
| 93 | + | if v <= 0 || v > 0.5 { | |
| 94 | + | return defaultResizeStepPercent | |
| 95 | + | } | |
| 96 | + | return v | |
| 97 | + | } | |
| 98 | + | ||
| 99 | + | // clampResizeMin clamps resizeMinPanePercent to (0, 0.5), returning the default (0.1) if out of range. | |
| 100 | + | func clampResizeMin(v float64) float64 { | |
| 101 | + | if v <= 0 || v >= 0.5 { | |
| 102 | + | return defaultResizeMinPanePercent | |
| 103 | + | } | |
| 104 | + | return v | |
| 105 | + | } | |
| 106 | + | ||
| 86 | 107 | // NewWorkspaceCoordinator creates a new WorkspaceCoordinator. | |
| 87 | 108 | func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig) *WorkspaceCoordinator { | |
| 88 | 109 | log := logging.FromContext(ctx) | |
| @@ -97,8 +118,8 @@ func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig | |||
| 97 | 118 | getActiveWS: cfg.GetActiveWS, | |
| 98 | 119 | generateID: cfg.GenerateID, | |
| 99 | 120 | newPaneURL: cfg.NewPaneURL, | |
| 100 | - | resizeStepPercent: cfg.ResizeStepPercent, | |
| 101 | - | resizeMinPanePercent: cfg.ResizeMinPanePercent, | |
| 121 | + | resizeStepPercent: clampResizeStep(cfg.ResizeStepPercent), | |
| 122 | + | resizeMinPanePercent: clampResizeMin(cfg.ResizeMinPanePercent), | |
| 102 | 123 | } | |
| 103 | 124 | } | |
| 104 | 125 | ||
| @@ -83,6 +83,27 @@ type incrementalCloseContext struct { | |||||
| 83 | precheckReason string | 83 | precheckReason string | ||
| 84 | } | 84 | } | ||
| 85 | 85 | ||||
| 86 | + | const ( | |||
| 87 | + | defaultResizeStepPercent = 0.05 | |||
| 88 | + | defaultResizeMinPanePercent = 0.1 | |||
| 89 | + | ) | |||
| 90 | + | ||||
| 91 | + | // clampResizeStep clamps resizeStepPercent to (0, 0.5], returning the default (0.05) if out of range. | |||
| 92 | + | func clampResizeStep(v float64) float64 { | |||
| 93 | + | if v <= 0 || v > 0.5 { | |||
| 94 | + | return defaultResizeStepPercent | |||
| 95 | + | } | |||
| 96 | + | return v | |||
| 97 | + | } | |||
| 98 | + | ||||
| 99 | + | // clampResizeMin clamps resizeMinPanePercent to (0, 0.5), returning the default (0.1) if out of range. | |||
| 100 | + | func clampResizeMin(v float64) float64 { | |||
| 101 | + | if v <= 0 || v >= 0.5 { | |||
| 102 | + | return defaultResizeMinPanePercent | |||
| 103 | + | } | |||
| 104 | + | return v | |||
| 105 | + | } | |||
| 106 | + | ||||
| 86 | // NewWorkspaceCoordinator creates a new WorkspaceCoordinator. | 107 | // NewWorkspaceCoordinator creates a new WorkspaceCoordinator. | ||
| 87 | func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig) *WorkspaceCoordinator { | 108 | func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig) *WorkspaceCoordinator { | ||
| 88 | log := logging.FromContext(ctx) | 109 | log := logging.FromContext(ctx) | ||
| @@ -97,8 +118,8 @@ func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig | |||||
| 97 | getActiveWS: cfg.GetActiveWS, | 118 | getActiveWS: cfg.GetActiveWS, | ||
| 98 | generateID: cfg.GenerateID, | 119 | generateID: cfg.GenerateID, | ||
| 99 | newPaneURL: cfg.NewPaneURL, | 120 | newPaneURL: cfg.NewPaneURL, | ||
| 100 | - | resizeStepPercent: cfg.ResizeStepPercent, | 121 | + | resizeStepPercent: clampResizeStep(cfg.ResizeStepPercent), |
| 101 | - | resizeMinPanePercent: cfg.ResizeMinPanePercent, | 122 | + | resizeMinPanePercent: clampResizeMin(cfg.ResizeMinPanePercent), |
| 102 | } | 123 | } | ||
| 103 | } | 124 | } | ||
| 104 | 125 | ||||
M internal/ui/dispatcher/keyboard.go +4 -0
| @@ -132,6 +132,10 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||
| 132 | 132 | d.actionHandlers = map[input.Action]func(ctx context.Context) error{ | |
| 133 | 133 | // Tab actions | |
| 134 | 134 | input.ActionNewTab: func(ctx context.Context) error { | |
| 135 | + | if d.newPaneURL == "" { | |
| 136 | + | logging.FromContext(ctx).Warn().Msg("ActionNewTab: newPaneURL is empty, cannot open new tab") | |
| 137 | + | return fmt.Errorf("newPaneURL is not configured") | |
| 138 | + | } | |
| 135 | 139 | _, err := d.tabCoord.Create(ctx, domainurl.Normalize(d.newPaneURL)) | |
| 136 | 140 | return err | |
| 137 | 141 | }, | |
| @@ -132,6 +132,10 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||||
| 132 | d.actionHandlers = map[input.Action]func(ctx context.Context) error{ | 132 | d.actionHandlers = map[input.Action]func(ctx context.Context) error{ | ||
| 133 | // Tab actions | 133 | // Tab actions | ||
| 134 | input.ActionNewTab: func(ctx context.Context) error { | 134 | input.ActionNewTab: func(ctx context.Context) error { | ||
| 135 | + | if d.newPaneURL == "" { | |||
| 136 | + | logging.FromContext(ctx).Warn().Msg("ActionNewTab: newPaneURL is empty, cannot open new tab") | |||
| 137 | + | return fmt.Errorf("newPaneURL is not configured") | |||
| 138 | + | } | |||
| 135 | _, err := d.tabCoord.Create(ctx, domainurl.Normalize(d.newPaneURL)) | 139 | _, err := d.tabCoord.Create(ctx, domainurl.Normalize(d.newPaneURL)) | ||
| 136 | return err | 140 | return err | ||
| 137 | }, | 141 | }, | ||
M internal/ui/input/shortcuts.go +2 -1
| @@ -219,7 +219,8 @@ type ShortcutSet struct { | |||
| 219 | 219 | } | |
| 220 | 220 | ||
| 221 | 221 | // NewShortcutSet creates a ShortcutSet from workspace and session configuration. | |
| 222 | - | // workspace must not be nil. session may be nil (session mode shortcuts will be empty). | |
| 222 | + | // workspace may be nil (workspace-specific shortcuts will be empty). | |
| 223 | + | // session may be nil (session mode shortcuts will be empty). | |
| 223 | 224 | func NewShortcutSet(ctx context.Context, workspace *entity.WorkspaceConfig, session *entity.SessionConfig) *ShortcutSet { | |
| 224 | 225 | log := logging.FromContext(ctx) | |
| 225 | 226 | set := &ShortcutSet{ | |
| @@ -219,7 +219,8 @@ type ShortcutSet struct { | |||||
| 219 | } | 219 | } | ||
| 220 | 220 | ||||
| 221 | // NewShortcutSet creates a ShortcutSet from workspace and session configuration. | 221 | // NewShortcutSet creates a ShortcutSet from workspace and session configuration. | ||
| 222 | - | // workspace must not be nil. session may be nil (session mode shortcuts will be empty). | 222 | + | // workspace may be nil (workspace-specific shortcuts will be empty). |
| 223 | + | // session may be nil (session mode shortcuts will be empty). | |||
| 223 | func NewShortcutSet(ctx context.Context, workspace *entity.WorkspaceConfig, session *entity.SessionConfig) *ShortcutSet { | 224 | func NewShortcutSet(ctx context.Context, workspace *entity.WorkspaceConfig, session *entity.SessionConfig) *ShortcutSet { | ||
| 224 | log := logging.FromContext(ctx) | 225 | log := logging.FromContext(ctx) | ||
| 225 | set := &ShortcutSet{ | 226 | set := &ShortcutSet{ | ||