Merge pull request #94 from bnema/fix/issues-82-87-83
fix: add auto-copy on selection, link overlay auto-hide, multi-word search
19 files · +642 -85
M CHANGELOG.md +6 -1
| @@ -5,15 +5,20 @@ All notable changes to this project will be documented in this file. | |||
| 5 | 5 | ## [Unreleased] | |
| 6 | 6 | ||
| 7 | 7 | ### Added | |
| 8 | + | - **Auto-copy on selection**: Text selected in web pages is automatically copied to the clipboard (like zellij/tmux). Shows brief "Copied to clipboard" toast notification. Configurable via `clipboard.auto_copy_on_selection` (default: true). Fixes #83. | |
| 9 | + | - **Stacked pane close buttons**: Close buttons in stacked pane title bars allow closing individual panes directly. Includes proper signal cleanup on removal. | |
| 8 | 10 | ||
| 9 | 11 | ### Changed | |
| 10 | 12 | - **Flatpak runtime**: Bumped to GNOME 49 runtime. | |
| 13 | + | - **Focus method refactored**: `Focus()` now delegates to `ApplyFocusChange()` to ensure stack index is updated when focusing stacked panes. | |
| 11 | 14 | ||
| 12 | 15 | ### Fixed | |
| 13 | 16 | - **Flatpak locale crash**: Attempted fix for crash on systems with certain locale configurations by adding proper locale data to the Flatpak bundle. | |
| 14 | 17 | - **Stacked pane vertical splits**: Vertical splits from stacked panes now correctly split around the entire stack instead of just the active pane. | |
| 15 | - | - **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. | |
| 18 | + | - **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. Fixes #89. | |
| 16 | 19 | - **Geometric focus navigation**: Fixed focus jumping to wrong pane when navigating left/right in complex layouts. Navigation now prioritizes panes at the same vertical level (for left/right) or horizontal level (for up/down). | |
| 20 | + | - **Link status overlay auto-hide**: Link URL overlay now automatically hides after 10 seconds of inactivity instead of staying visible indefinitely. Timer resets when hovering new links. Fixes #82. | |
| 21 | + | - **Omnibox multi-word search**: History search now matches all words in query (AND logic) and searches both URL and title fields. Prefix matching enabled (e.g., "git iss" matches "github.com/issues"). FTS5 special characters are now sanitized to prevent syntax errors. Results interleave URL and title matches for better balance. Fixes #87. | |
| 17 | 22 | ||
| 18 | 23 | ## [0.23.2] - 2025-12-31 | |
| 19 | 24 | ||
| @@ -5,15 +5,20 @@ All notable changes to this project will be documented in this file. | |||||
| 5 | ## [Unreleased] | 5 | ## [Unreleased] | ||
| 6 | 6 | ||||
| 7 | ### Added | 7 | ### Added | ||
| 8 | + | - **Auto-copy on selection**: Text selected in web pages is automatically copied to the clipboard (like zellij/tmux). Shows brief "Copied to clipboard" toast notification. Configurable via `clipboard.auto_copy_on_selection` (default: true). Fixes #83. | |||
| 9 | + | - **Stacked pane close buttons**: Close buttons in stacked pane title bars allow closing individual panes directly. Includes proper signal cleanup on removal. | |||
| 8 | 10 | ||||
| 9 | ### Changed | 11 | ### Changed | ||
| 10 | - **Flatpak runtime**: Bumped to GNOME 49 runtime. | 12 | - **Flatpak runtime**: Bumped to GNOME 49 runtime. | ||
| 13 | + | - **Focus method refactored**: `Focus()` now delegates to `ApplyFocusChange()` to ensure stack index is updated when focusing stacked panes. | |||
| 11 | 14 | ||||
| 12 | ### Fixed | 15 | ### Fixed | ||
| 13 | - **Flatpak locale crash**: Attempted fix for crash on systems with certain locale configurations by adding proper locale data to the Flatpak bundle. | 16 | - **Flatpak locale crash**: Attempted fix for crash on systems with certain locale configurations by adding proper locale data to the Flatpak bundle. | ||
| 14 | - **Stacked pane vertical splits**: Vertical splits from stacked panes now correctly split around the entire stack instead of just the active pane. | 17 | - **Stacked pane vertical splits**: Vertical splits from stacked panes now correctly split around the entire stack instead of just the active pane. | ||
| 15 | - | - **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. | 18 | + | - **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. Fixes #89. |
| 16 | - **Geometric focus navigation**: Fixed focus jumping to wrong pane when navigating left/right in complex layouts. Navigation now prioritizes panes at the same vertical level (for left/right) or horizontal level (for up/down). | 19 | - **Geometric focus navigation**: Fixed focus jumping to wrong pane when navigating left/right in complex layouts. Navigation now prioritizes panes at the same vertical level (for left/right) or horizontal level (for up/down). | ||
| 20 | + | - **Link status overlay auto-hide**: Link URL overlay now automatically hides after 10 seconds of inactivity instead of staying visible indefinitely. Timer resets when hovering new links. Fixes #82. | |||
| 21 | + | - **Omnibox multi-word search**: History search now matches all words in query (AND logic) and searches both URL and title fields. Prefix matching enabled (e.g., "git iss" matches "github.com/issues"). FTS5 special characters are now sanitized to prevent syntax errors. Results interleave URL and title matches for better balance. Fixes #87. | |||
| 17 | 22 | ||||
| 18 | ## [0.23.2] - 2025-12-31 | 23 | ## [0.23.2] - 2025-12-31 | ||
| 19 | 24 | ||||
M internal/application/usecase/manage_panes.go +3 -21
| @@ -540,31 +540,13 @@ func (uc *ManagePanesUseCase) Close(ctx context.Context, ws *entity.Workspace, p | |||
| 540 | 540 | } | |
| 541 | 541 | ||
| 542 | 542 | // Focus sets the active pane in the workspace. | |
| 543 | + | // Delegates to ApplyFocusChange to ensure stack index is updated when focusing a stacked pane. | |
| 543 | 544 | func (uc *ManagePanesUseCase) Focus(ctx context.Context, ws *entity.Workspace, paneID entity.PaneID) error { | |
| 544 | 545 | log := logging.FromContext(ctx) | |
| 545 | - | if uc == nil { | |
| 546 | - | return fmt.Errorf("manage panes use case is nil") | |
| 547 | - | } | |
| 548 | 546 | log.Debug().Str("pane_id", string(paneID)).Msg("focusing pane") | |
| 549 | 547 | ||
| 550 | - | if ws == nil { | |
| 551 | - | return fmt.Errorf("workspace is required") | |
| 552 | - | } | |
| 553 | - | ||
| 554 | - | node := ws.FindPane(paneID) | |
| 555 | - | if node == nil { | |
| 556 | - | return fmt.Errorf("pane not found: %s", paneID) | |
| 557 | - | } | |
| 558 | - | ||
| 559 | - | oldActive := ws.ActivePaneID | |
| 560 | - | ws.ActivePaneID = paneID | |
| 561 | - | ||
| 562 | - | log.Info(). | |
| 563 | - | Str("from", string(oldActive)). | |
| 564 | - | Str("to", string(paneID)). | |
| 565 | - | Msg("focus changed") | |
| 566 | - | ||
| 567 | - | return nil | |
| 548 | + | _, err := uc.ApplyFocusChange(ctx, ws, paneID) | |
| 549 | + | return err | |
| 568 | 550 | } | |
| 569 | 551 | ||
| 570 | 552 | // NavigateFocus moves focus to an adjacent pane. | |
| @@ -540,31 +540,13 @@ func (uc *ManagePanesUseCase) Close(ctx context.Context, ws *entity.Workspace, p | |||||
| 540 | } | 540 | } | ||
| 541 | 541 | ||||
| 542 | // Focus sets the active pane in the workspace. | 542 | // Focus sets the active pane in the workspace. | ||
| 543 | + | // Delegates to ApplyFocusChange to ensure stack index is updated when focusing a stacked pane. | |||
| 543 | func (uc *ManagePanesUseCase) Focus(ctx context.Context, ws *entity.Workspace, paneID entity.PaneID) error { | 544 | func (uc *ManagePanesUseCase) Focus(ctx context.Context, ws *entity.Workspace, paneID entity.PaneID) error { | ||
| 544 | log := logging.FromContext(ctx) | 545 | log := logging.FromContext(ctx) | ||
| 545 | - | if uc == nil { | |||
| 546 | - | return fmt.Errorf("manage panes use case is nil") | |||
| 547 | - | } | |||
| 548 | log.Debug().Str("pane_id", string(paneID)).Msg("focusing pane") | 546 | log.Debug().Str("pane_id", string(paneID)).Msg("focusing pane") | ||
| 549 | 547 | ||||
| 550 | - | if ws == nil { | 548 | + | _, err := uc.ApplyFocusChange(ctx, ws, paneID) |
| 551 | - | return fmt.Errorf("workspace is required") | 549 | + | return err |
| 552 | - | } | |||
| 553 | - | ||||
| 554 | - | node := ws.FindPane(paneID) | |||
| 555 | - | if node == nil { | |||
| 556 | - | return fmt.Errorf("pane not found: %s", paneID) | |||
| 557 | - | } | |||
| 558 | - | ||||
| 559 | - | oldActive := ws.ActivePaneID | |||
| 560 | - | ws.ActivePaneID = paneID | |||
| 561 | - | ||||
| 562 | - | log.Info(). | |||
| 563 | - | Str("from", string(oldActive)). | |||
| 564 | - | Str("to", string(paneID)). | |||
| 565 | - | Msg("focus changed") | |||
| 566 | - | ||||
| 567 | - | return nil | |||
| 568 | } | 550 | } | ||
| 569 | 551 | ||||
| 570 | // NavigateFocus moves focus to an adjacent pane. | 552 | // NavigateFocus moves focus to an adjacent pane. | ||
M internal/bootstrap/webkit_stack.go +5 -0
| @@ -120,6 +120,11 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 120 | 120 | settings := webkit.NewSettingsManager(ctx, cfg) | |
| 121 | 121 | injector := webkit.NewContentInjector(colorResolver) | |
| 122 | 122 | ||
| 123 | + | // Set up auto-copy on selection config getter | |
| 124 | + | injector.SetAutoCopyConfigGetter(func() bool { | |
| 125 | + | return config.Get().Clipboard.AutoCopyOnSelection | |
| 126 | + | }) | |
| 127 | + | ||
| 123 | 128 | prepareThemeUC := usecase.NewPrepareWebUIThemeUseCase(injector) | |
| 124 | 129 | themeCSSText := themeManager.GetWebUIThemeCSS() | |
| 125 | 130 | if err := prepareThemeUC.Execute(ctx, usecase.PrepareWebUIThemeInput{CSSVars: themeCSSText}); err != nil { | |
| @@ -120,6 +120,11 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 120 | settings := webkit.NewSettingsManager(ctx, cfg) | 120 | settings := webkit.NewSettingsManager(ctx, cfg) | ||
| 121 | injector := webkit.NewContentInjector(colorResolver) | 121 | injector := webkit.NewContentInjector(colorResolver) | ||
| 122 | 122 | ||||
| 123 | + | // Set up auto-copy on selection config getter | |||
| 124 | + | injector.SetAutoCopyConfigGetter(func() bool { | |||
| 125 | + | return config.Get().Clipboard.AutoCopyOnSelection | |||
| 126 | + | }) | |||
| 127 | + | ||||
| 123 | prepareThemeUC := usecase.NewPrepareWebUIThemeUseCase(injector) | 128 | prepareThemeUC := usecase.NewPrepareWebUIThemeUseCase(injector) | ||
| 124 | themeCSSText := themeManager.GetWebUIThemeCSS() | 129 | themeCSSText := themeManager.GetWebUIThemeCSS() | ||
| 125 | if err := prepareThemeUC.Execute(ctx, usecase.PrepareWebUIThemeInput{CSSVars: themeCSSText}); err != nil { | 130 | if err := prepareThemeUC.Execute(ctx, usecase.PrepareWebUIThemeInput{CSSVars: themeCSSText}); err != nil { | ||
M internal/infrastructure/config/defaults.go +3 -0
| @@ -253,6 +253,9 @@ func DefaultConfig() *Config { | |||
| 253 | 253 | Enabled: true, // Ad blocking enabled by default | |
| 254 | 254 | AutoUpdate: true, // Auto-update filters from GitHub releases | |
| 255 | 255 | }, | |
| 256 | + | Clipboard: ClipboardConfig{ | |
| 257 | + | AutoCopyOnSelection: true, // Enabled by default (zellij-style) | |
| 258 | + | }, | |
| 256 | 259 | Omnibox: OmniboxConfig{ | |
| 257 | 260 | InitialBehavior: defaultOmniboxInitialBehavior, | |
| 258 | 261 | AutoOpenOnNewPane: defaultOmniboxAutoOpenOnNewPane, | |
| @@ -253,6 +253,9 @@ func DefaultConfig() *Config { | |||||
| 253 | Enabled: true, // Ad blocking enabled by default | 253 | Enabled: true, // Ad blocking enabled by default | ||
| 254 | AutoUpdate: true, // Auto-update filters from GitHub releases | 254 | AutoUpdate: true, // Auto-update filters from GitHub releases | ||
| 255 | }, | 255 | }, | ||
| 256 | + | Clipboard: ClipboardConfig{ | |||
| 257 | + | AutoCopyOnSelection: true, // Enabled by default (zellij-style) | |||
| 258 | + | }, | |||
| 256 | Omnibox: OmniboxConfig{ | 259 | Omnibox: OmniboxConfig{ | ||
| 257 | InitialBehavior: defaultOmniboxInitialBehavior, | 260 | InitialBehavior: defaultOmniboxInitialBehavior, | ||
| 258 | AutoOpenOnNewPane: defaultOmniboxAutoOpenOnNewPane, | 261 | AutoOpenOnNewPane: defaultOmniboxAutoOpenOnNewPane, | ||
M internal/infrastructure/config/loader.go +5 -0
| @@ -328,6 +328,7 @@ func (m *Manager) setDefaults() { | |||
| 328 | 328 | m.setRenderingDefaults(defaults) | |
| 329 | 329 | m.setWorkspaceDefaults(defaults) | |
| 330 | 330 | m.setContentFilteringDefaults(defaults) | |
| 331 | + | m.setClipboardDefaults(defaults) | |
| 331 | 332 | m.setOmniboxDefaults(defaults) | |
| 332 | 333 | m.setMediaDefaults(defaults) | |
| 333 | 334 | m.setRuntimeDefaults(defaults) | |
| @@ -443,6 +444,10 @@ func (m *Manager) setContentFilteringDefaults(defaults *Config) { | |||
| 443 | 444 | m.viper.SetDefault("content_filtering.auto_update", defaults.ContentFiltering.AutoUpdate) | |
| 444 | 445 | } | |
| 445 | 446 | ||
| 447 | + | func (m *Manager) setClipboardDefaults(defaults *Config) { | |
| 448 | + | m.viper.SetDefault("clipboard.auto_copy_on_selection", defaults.Clipboard.AutoCopyOnSelection) | |
| 449 | + | } | |
| 450 | + | ||
| 446 | 451 | func (m *Manager) setOmniboxDefaults(defaults *Config) { | |
| 447 | 452 | m.viper.SetDefault("omnibox.initial_behavior", defaults.Omnibox.InitialBehavior) | |
| 448 | 453 | m.viper.SetDefault("omnibox.auto_open_on_new_pane", defaults.Omnibox.AutoOpenOnNewPane) | |
| @@ -328,6 +328,7 @@ func (m *Manager) setDefaults() { | |||||
| 328 | m.setRenderingDefaults(defaults) | 328 | m.setRenderingDefaults(defaults) | ||
| 329 | m.setWorkspaceDefaults(defaults) | 329 | m.setWorkspaceDefaults(defaults) | ||
| 330 | m.setContentFilteringDefaults(defaults) | 330 | m.setContentFilteringDefaults(defaults) | ||
| 331 | + | m.setClipboardDefaults(defaults) | |||
| 331 | m.setOmniboxDefaults(defaults) | 332 | m.setOmniboxDefaults(defaults) | ||
| 332 | m.setMediaDefaults(defaults) | 333 | m.setMediaDefaults(defaults) | ||
| 333 | m.setRuntimeDefaults(defaults) | 334 | m.setRuntimeDefaults(defaults) | ||
| @@ -443,6 +444,10 @@ func (m *Manager) setContentFilteringDefaults(defaults *Config) { | |||||
| 443 | m.viper.SetDefault("content_filtering.auto_update", defaults.ContentFiltering.AutoUpdate) | 444 | m.viper.SetDefault("content_filtering.auto_update", defaults.ContentFiltering.AutoUpdate) | ||
| 444 | } | 445 | } | ||
| 445 | 446 | ||||
| 447 | + | func (m *Manager) setClipboardDefaults(defaults *Config) { | |||
| 448 | + | m.viper.SetDefault("clipboard.auto_copy_on_selection", defaults.Clipboard.AutoCopyOnSelection) | |||
| 449 | + | } | |||
| 450 | + | ||||
| 446 | func (m *Manager) setOmniboxDefaults(defaults *Config) { | 451 | func (m *Manager) setOmniboxDefaults(defaults *Config) { | ||
| 447 | m.viper.SetDefault("omnibox.initial_behavior", defaults.Omnibox.InitialBehavior) | 452 | m.viper.SetDefault("omnibox.initial_behavior", defaults.Omnibox.InitialBehavior) | ||
| 448 | m.viper.SetDefault("omnibox.auto_open_on_new_pane", defaults.Omnibox.AutoOpenOnNewPane) | 453 | m.viper.SetDefault("omnibox.auto_open_on_new_pane", defaults.Omnibox.AutoOpenOnNewPane) | ||
M internal/infrastructure/config/schema.go +11 -0
| @@ -21,6 +21,8 @@ type Config struct { | |||
| 21 | 21 | Session SessionConfig `mapstructure:"session" yaml:"session" toml:"session"` | |
| 22 | 22 | // ContentFiltering controls ad blocking and content filtering | |
| 23 | 23 | ContentFiltering ContentFilteringConfig `mapstructure:"content_filtering" yaml:"content_filtering" toml:"content_filtering"` | |
| 24 | + | // Clipboard controls clipboard-related behavior | |
| 25 | + | Clipboard ClipboardConfig `mapstructure:"clipboard" yaml:"clipboard" toml:"clipboard"` | |
| 24 | 26 | // Omnibox controls the omnibox behavior (initial history display) | |
| 25 | 27 | Omnibox OmniboxConfig `mapstructure:"omnibox" yaml:"omnibox" toml:"omnibox"` | |
| 26 | 28 | // Rendering controls WebKit, GTK and compositor behavior. | |
| @@ -337,6 +339,15 @@ type ContentFilteringConfig struct { | |||
| 337 | 339 | // Note: Whitelist is managed via database (content_whitelist table) | |
| 338 | 340 | } | |
| 339 | 341 | ||
| 342 | + | // ClipboardConfig holds clipboard-related behavior preferences | |
| 343 | + | type ClipboardConfig struct { | |
| 344 | + | // AutoCopyOnSelection automatically copies selected text to clipboard (zellij-style). | |
| 345 | + | // When enabled, selecting text in a webview immediately copies it to clipboard. | |
| 346 | + | // Does not apply to text selection in input fields or textareas. | |
| 347 | + | // Default: true | |
| 348 | + | AutoCopyOnSelection bool `mapstructure:"auto_copy_on_selection" yaml:"auto_copy_on_selection" toml:"auto_copy_on_selection" json:"autoCopyOnSelection"` //nolint:lll // struct tags must stay on one line | |
| 349 | + | } | |
| 350 | + | ||
| 340 | 351 | // OmniboxConfig holds omnibox behavior preferences | |
| 341 | 352 | type OmniboxConfig struct { | |
| 342 | 353 | // InitialBehavior controls what to show when omnibox opens with empty input | |
| @@ -21,6 +21,8 @@ type Config struct { | |||||
| 21 | Session SessionConfig `mapstructure:"session" yaml:"session" toml:"session"` | 21 | Session SessionConfig `mapstructure:"session" yaml:"session" toml:"session"` | ||
| 22 | // ContentFiltering controls ad blocking and content filtering | 22 | // ContentFiltering controls ad blocking and content filtering | ||
| 23 | ContentFiltering ContentFilteringConfig `mapstructure:"content_filtering" yaml:"content_filtering" toml:"content_filtering"` | 23 | ContentFiltering ContentFilteringConfig `mapstructure:"content_filtering" yaml:"content_filtering" toml:"content_filtering"` | ||
| 24 | + | // Clipboard controls clipboard-related behavior | |||
| 25 | + | Clipboard ClipboardConfig `mapstructure:"clipboard" yaml:"clipboard" toml:"clipboard"` | |||
| 24 | // Omnibox controls the omnibox behavior (initial history display) | 26 | // Omnibox controls the omnibox behavior (initial history display) | ||
| 25 | Omnibox OmniboxConfig `mapstructure:"omnibox" yaml:"omnibox" toml:"omnibox"` | 27 | Omnibox OmniboxConfig `mapstructure:"omnibox" yaml:"omnibox" toml:"omnibox"` | ||
| 26 | // Rendering controls WebKit, GTK and compositor behavior. | 28 | // Rendering controls WebKit, GTK and compositor behavior. | ||
| @@ -337,6 +339,15 @@ type ContentFilteringConfig struct { | |||||
| 337 | // Note: Whitelist is managed via database (content_whitelist table) | 339 | // Note: Whitelist is managed via database (content_whitelist table) | ||
| 338 | } | 340 | } | ||
| 339 | 341 | ||||
| 342 | + | // ClipboardConfig holds clipboard-related behavior preferences | |||
| 343 | + | type ClipboardConfig struct { | |||
| 344 | + | // AutoCopyOnSelection automatically copies selected text to clipboard (zellij-style). | |||
| 345 | + | // When enabled, selecting text in a webview immediately copies it to clipboard. | |||
| 346 | + | // Does not apply to text selection in input fields or textareas. | |||
| 347 | + | // Default: true | |||
| 348 | + | AutoCopyOnSelection bool `mapstructure:"auto_copy_on_selection" yaml:"auto_copy_on_selection" toml:"auto_copy_on_selection" json:"autoCopyOnSelection"` //nolint:lll // struct tags must stay on one line | |||
| 349 | + | } | |||
| 350 | + | ||||
| 340 | // OmniboxConfig holds omnibox behavior preferences | 351 | // OmniboxConfig holds omnibox behavior preferences | ||
| 341 | type OmniboxConfig struct { | 352 | type OmniboxConfig struct { | ||
| 342 | // InitialBehavior controls what to show when omnibox opens with empty input | 353 | // InitialBehavior controls what to show when omnibox opens with empty input | ||
M internal/infrastructure/persistence/sqlite/history_repo.go +77 -15
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "database/sql" | |
| 6 | 6 | "errors" | |
| 7 | 7 | "fmt" | |
| 8 | + | "strings" | |
| 8 | 9 | "time" | |
| 9 | 10 | ||
| 10 | 11 | "github.com/bnema/dumber/internal/domain/entity" | |
| @@ -47,31 +48,92 @@ func (r *historyRepo) FindByURL(ctx context.Context, url string) (*entity.Histor | |||
| 47 | 48 | func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]entity.HistoryMatch, error) { | |
| 48 | 49 | log := logging.FromContext(ctx) | |
| 49 | 50 | ||
| 50 | - | // Use FTS5 full-text search for better accuracy | |
| 51 | - | // FTS5 query syntax: use * for prefix matching | |
| 52 | - | ftsQuery := query + "*" | |
| 51 | + | // Split query into words and add prefix matching to each | |
| 52 | + | // This enables multi-word searches like "github issues" -> "github* issues*" | |
| 53 | + | words := strings.Fields(query) | |
| 54 | + | if len(words) == 0 { | |
| 55 | + | return []entity.HistoryMatch{}, nil | |
| 56 | + | } | |
| 53 | 57 | ||
| 54 | - | rows, err := r.queries.SearchHistoryFTS(ctx, sqlc.SearchHistoryFTSParams{ | |
| 55 | - | Url: ftsQuery, | |
| 58 | + | // Build FTS5 query: "word1* word2*" (implicit AND between terms) | |
| 59 | + | // Sanitize each word to remove FTS5 special characters | |
| 60 | + | parts := make([]string, len(words)) | |
| 61 | + | for i, word := range words { | |
| 62 | + | parts[i] = sanitizeFTS5Word(word) + "*" | |
| 63 | + | } | |
| 64 | + | ftsQuery := strings.Join(parts, " ") | |
| 65 | + | ||
| 66 | + | // Search both URL and title columns, then merge results | |
| 67 | + | urlRows, urlErr := r.queries.SearchHistoryFTSUrl(ctx, sqlc.SearchHistoryFTSUrlParams{ | |
| 68 | + | Query: ftsQuery, | |
| 56 | 69 | Limit: int64(limit), | |
| 57 | 70 | }) | |
| 58 | - | if err != nil { | |
| 59 | - | log.Debug().Err(err).Str("query", query).Msg("FTS search failed, no results") | |
| 60 | - | // FTS5 returns error on no match or invalid query, return empty results | |
| 61 | - | return []entity.HistoryMatch{}, nil | |
| 71 | + | if urlErr != nil { | |
| 72 | + | log.Debug().Err(urlErr).Str("query", query).Msg("FTS URL search failed") | |
| 73 | + | urlRows = []sqlc.History{} | |
| 62 | 74 | } | |
| 63 | 75 | ||
| 64 | - | matches := make([]entity.HistoryMatch, len(rows)) | |
| 65 | - | for i := range rows { | |
| 66 | - | row := rows[i] | |
| 67 | - | matches[i] = entity.HistoryMatch{ | |
| 68 | - | Entry: historyFromRow(row), | |
| 69 | - | Score: 1.0, // FTS5 already ranked by bm25 | |
| 76 | + | titleRows, titleErr := r.queries.SearchHistoryFTSTitle(ctx, sqlc.SearchHistoryFTSTitleParams{ | |
| 77 | + | Query: ftsQuery, | |
| 78 | + | Limit: int64(limit), | |
| 79 | + | }) | |
| 80 | + | if titleErr != nil { | |
| 81 | + | log.Debug().Err(titleErr).Str("query", query).Msg("FTS title search failed") | |
| 82 | + | titleRows = []sqlc.History{} | |
| 83 | + | } | |
| 84 | + | ||
| 85 | + | // Merge results by interleaving URL and title matches for balanced results | |
| 86 | + | seen := make(map[int64]bool) | |
| 87 | + | var matches []entity.HistoryMatch | |
| 88 | + | ||
| 89 | + | i, j := 0, 0 | |
| 90 | + | for len(matches) < limit && (i < len(urlRows) || j < len(titleRows)) { | |
| 91 | + | // Take next URL match if available | |
| 92 | + | if i < len(urlRows) { | |
| 93 | + | row := urlRows[i] | |
| 94 | + | i++ | |
| 95 | + | if !seen[row.ID] { | |
| 96 | + | seen[row.ID] = true | |
| 97 | + | matches = append(matches, entity.HistoryMatch{ | |
| 98 | + | Entry: historyFromRow(row), | |
| 99 | + | Score: 1.0, | |
| 100 | + | }) | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | ||
| 104 | + | // Take next title match if available | |
| 105 | + | if j < len(titleRows) && len(matches) < limit { | |
| 106 | + | row := titleRows[j] | |
| 107 | + | j++ | |
| 108 | + | if !seen[row.ID] { | |
| 109 | + | seen[row.ID] = true | |
| 110 | + | matches = append(matches, entity.HistoryMatch{ | |
| 111 | + | Entry: historyFromRow(row), | |
| 112 | + | Score: 1.0, | |
| 113 | + | }) | |
| 114 | + | } | |
| 70 | 115 | } | |
| 71 | 116 | } | |
| 117 | + | ||
| 72 | 118 | return matches, nil | |
| 73 | 119 | } | |
| 74 | 120 | ||
| 121 | + | // sanitizeFTS5Word removes FTS5 special characters from a search word. | |
| 122 | + | // FTS5 special chars: " * ( ) : ^ - AND OR NOT NEAR | |
| 123 | + | func sanitizeFTS5Word(word string) string { | |
| 124 | + | // Remove characters that have special meaning in FTS5 | |
| 125 | + | var result strings.Builder | |
| 126 | + | for _, r := range word { | |
| 127 | + | switch r { | |
| 128 | + | case '"', '*', '(', ')', ':', '^', '-': | |
| 129 | + | // Skip FTS5 special characters | |
| 130 | + | default: | |
| 131 | + | result.WriteRune(r) | |
| 132 | + | } | |
| 133 | + | } | |
| 134 | + | return result.String() | |
| 135 | + | } | |
| 136 | + | ||
| 75 | 137 | func (r *historyRepo) GetRecent(ctx context.Context, limit, offset int) ([]*entity.HistoryEntry, error) { | |
| 76 | 138 | rows, err := r.queries.GetRecentHistory(ctx, sqlc.GetRecentHistoryParams{ | |
| 77 | 139 | Limit: int64(limit), | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "database/sql" | 5 | "database/sql" | ||
| 6 | "errors" | 6 | "errors" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | + | "strings" | |||
| 8 | "time" | 9 | "time" | ||
| 9 | 10 | ||||
| 10 | "github.com/bnema/dumber/internal/domain/entity" | 11 | "github.com/bnema/dumber/internal/domain/entity" | ||
| @@ -47,31 +48,92 @@ func (r *historyRepo) FindByURL(ctx context.Context, url string) (*entity.Histor | |||||
| 47 | func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]entity.HistoryMatch, error) { | 48 | func (r *historyRepo) Search(ctx context.Context, query string, limit int) ([]entity.HistoryMatch, error) { | ||
| 48 | log := logging.FromContext(ctx) | 49 | log := logging.FromContext(ctx) | ||
| 49 | 50 | ||||
| 50 | - | // Use FTS5 full-text search for better accuracy | 51 | + | // Split query into words and add prefix matching to each |
| 51 | - | // FTS5 query syntax: use * for prefix matching | 52 | + | // This enables multi-word searches like "github issues" -> "github* issues*" |
| 52 | - | ftsQuery := query + "*" | 53 | + | words := strings.Fields(query) |
| 54 | + | if len(words) == 0 { | |||
| 55 | + | return []entity.HistoryMatch{}, nil | |||
| 56 | + | } | |||
| 53 | 57 | ||||
| 54 | - | rows, err := r.queries.SearchHistoryFTS(ctx, sqlc.SearchHistoryFTSParams{ | 58 | + | // Build FTS5 query: "word1* word2*" (implicit AND between terms) |
| 55 | - | Url: ftsQuery, | 59 | + | // Sanitize each word to remove FTS5 special characters |
| 60 | + | parts := make([]string, len(words)) | |||
| 61 | + | for i, word := range words { | |||
| 62 | + | parts[i] = sanitizeFTS5Word(word) + "*" | |||
| 63 | + | } | |||
| 64 | + | ftsQuery := strings.Join(parts, " ") | |||
| 65 | + | ||||
| 66 | + | // Search both URL and title columns, then merge results | |||
| 67 | + | urlRows, urlErr := r.queries.SearchHistoryFTSUrl(ctx, sqlc.SearchHistoryFTSUrlParams{ | |||
| 68 | + | Query: ftsQuery, | |||
| 56 | Limit: int64(limit), | 69 | Limit: int64(limit), | ||
| 57 | }) | 70 | }) | ||
| 58 | - | if err != nil { | 71 | + | if urlErr != nil { |
| 59 | - | log.Debug().Err(err).Str("query", query).Msg("FTS search failed, no results") | 72 | + | log.Debug().Err(urlErr).Str("query", query).Msg("FTS URL search failed") |
| 60 | - | // FTS5 returns error on no match or invalid query, return empty results | 73 | + | urlRows = []sqlc.History{} |
| 61 | - | return []entity.HistoryMatch{}, nil | |||
| 62 | } | 74 | } | ||
| 63 | 75 | ||||
| 64 | - | matches := make([]entity.HistoryMatch, len(rows)) | 76 | + | titleRows, titleErr := r.queries.SearchHistoryFTSTitle(ctx, sqlc.SearchHistoryFTSTitleParams{ |
| 65 | - | for i := range rows { | 77 | + | Query: ftsQuery, |
| 66 | - | row := rows[i] | 78 | + | Limit: int64(limit), |
| 67 | - | matches[i] = entity.HistoryMatch{ | 79 | + | }) |
| 68 | - | Entry: historyFromRow(row), | 80 | + | if titleErr != nil { |
| 69 | - | Score: 1.0, // FTS5 already ranked by bm25 | 81 | + | log.Debug().Err(titleErr).Str("query", query).Msg("FTS title search failed") |
| 82 | + | titleRows = []sqlc.History{} | |||
| 83 | + | } | |||
| 84 | + | ||||
| 85 | + | // Merge results by interleaving URL and title matches for balanced results | |||
| 86 | + | seen := make(map[int64]bool) | |||
| 87 | + | var matches []entity.HistoryMatch | |||
| 88 | + | ||||
| 89 | + | i, j := 0, 0 | |||
| 90 | + | for len(matches) < limit && (i < len(urlRows) || j < len(titleRows)) { | |||
| 91 | + | // Take next URL match if available | |||
| 92 | + | if i < len(urlRows) { | |||
| 93 | + | row := urlRows[i] | |||
| 94 | + | i++ | |||
| 95 | + | if !seen[row.ID] { | |||
| 96 | + | seen[row.ID] = true | |||
| 97 | + | matches = append(matches, entity.HistoryMatch{ | |||
| 98 | + | Entry: historyFromRow(row), | |||
| 99 | + | Score: 1.0, | |||
| 100 | + | }) | |||
| 101 | + | } | |||
| 102 | + | } | |||
| 103 | + | ||||
| 104 | + | // Take next title match if available | |||
| 105 | + | if j < len(titleRows) && len(matches) < limit { | |||
| 106 | + | row := titleRows[j] | |||
| 107 | + | j++ | |||
| 108 | + | if !seen[row.ID] { | |||
| 109 | + | seen[row.ID] = true | |||
| 110 | + | matches = append(matches, entity.HistoryMatch{ | |||
| 111 | + | Entry: historyFromRow(row), | |||
| 112 | + | Score: 1.0, | |||
| 113 | + | }) | |||
| 114 | + | } | |||
| 70 | } | 115 | } | ||
| 71 | } | 116 | } | ||
| 117 | + | ||||
| 72 | return matches, nil | 118 | return matches, nil | ||
| 73 | } | 119 | } | ||
| 74 | 120 | ||||
| 121 | + | // sanitizeFTS5Word removes FTS5 special characters from a search word. | |||
| 122 | + | // FTS5 special chars: " * ( ) : ^ - AND OR NOT NEAR | |||
| 123 | + | func sanitizeFTS5Word(word string) string { | |||
| 124 | + | // Remove characters that have special meaning in FTS5 | |||
| 125 | + | var result strings.Builder | |||
| 126 | + | for _, r := range word { | |||
| 127 | + | switch r { | |||
| 128 | + | case '"', '*', '(', ')', ':', '^', '-': | |||
| 129 | + | // Skip FTS5 special characters | |||
| 130 | + | default: | |||
| 131 | + | result.WriteRune(r) | |||
| 132 | + | } | |||
| 133 | + | } | |||
| 134 | + | return result.String() | |||
| 135 | + | } | |||
| 136 | + | ||||
| 75 | func (r *historyRepo) GetRecent(ctx context.Context, limit, offset int) ([]*entity.HistoryEntry, error) { | 137 | func (r *historyRepo) GetRecent(ctx context.Context, limit, offset int) ([]*entity.HistoryEntry, error) { | ||
| 76 | rows, err := r.queries.GetRecentHistory(ctx, sqlc.GetRecentHistoryParams{ | 138 | rows, err := r.queries.GetRecentHistory(ctx, sqlc.GetRecentHistoryParams{ | ||
| 77 | Limit: int64(limit), | 139 | Limit: int64(limit), | ||
M internal/infrastructure/persistence/sqlite/history_repo_test.go +161 -0
| @@ -323,3 +323,164 @@ func TestHistoryRepository_GetAllMostVisited(t *testing.T) { | |||
| 323 | 323 | assert.Equal(t, "https://github.com", results[0].URL) | |
| 324 | 324 | assert.Equal(t, int64(4), results[0].VisitCount) | |
| 325 | 325 | } | |
| 326 | + | ||
| 327 | + | func TestHistoryRepository_Search_SingleWord(t *testing.T) { | |
| 328 | + | ctx := historyTestCtx() | |
| 329 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 330 | + | ||
| 331 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 332 | + | require.NoError(t, err) | |
| 333 | + | t.Cleanup(func() { _ = db.Close() }) | |
| 334 | + | ||
| 335 | + | repo := sqlite.NewHistoryRepository(db) | |
| 336 | + | ||
| 337 | + | // Save entries | |
| 338 | + | entries := []*entity.HistoryEntry{ | |
| 339 | + | {URL: "https://github.com/user/repo", Title: "My GitHub Repository"}, | |
| 340 | + | {URL: "https://gitlab.com/project", Title: "GitLab Project"}, | |
| 341 | + | {URL: "https://example.com", Title: "Example Site"}, | |
| 342 | + | } | |
| 343 | + | for _, e := range entries { | |
| 344 | + | require.NoError(t, repo.Save(ctx, e)) | |
| 345 | + | } | |
| 346 | + | ||
| 347 | + | // Search for "github" - should find the github entry | |
| 348 | + | results, err := repo.Search(ctx, "github", 10) | |
| 349 | + | require.NoError(t, err) | |
| 350 | + | require.Len(t, results, 1) | |
| 351 | + | assert.Equal(t, "https://github.com/user/repo", results[0].Entry.URL) | |
| 352 | + | } | |
| 353 | + | ||
| 354 | + | func TestHistoryRepository_Search_MultiWord(t *testing.T) { | |
| 355 | + | ctx := historyTestCtx() | |
| 356 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 357 | + | ||
| 358 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 359 | + | require.NoError(t, err) | |
| 360 | + | t.Cleanup(func() { _ = db.Close() }) | |
| 361 | + | ||
| 362 | + | repo := sqlite.NewHistoryRepository(db) | |
| 363 | + | ||
| 364 | + | // Save entries | |
| 365 | + | entries := []*entity.HistoryEntry{ | |
| 366 | + | {URL: "https://github.com/issues", Title: "GitHub Issues"}, | |
| 367 | + | {URL: "https://github.com/pulls", Title: "GitHub Pull Requests"}, | |
| 368 | + | {URL: "https://jira.com/issues", Title: "Jira Issues"}, | |
| 369 | + | } | |
| 370 | + | for _, e := range entries { | |
| 371 | + | require.NoError(t, repo.Save(ctx, e)) | |
| 372 | + | } | |
| 373 | + | ||
| 374 | + | // Search for "github issues" - should find only the github issues entry | |
| 375 | + | results, err := repo.Search(ctx, "github issues", 10) | |
| 376 | + | require.NoError(t, err) | |
| 377 | + | require.Len(t, results, 1) | |
| 378 | + | assert.Equal(t, "https://github.com/issues", results[0].Entry.URL) | |
| 379 | + | } | |
| 380 | + | ||
| 381 | + | func TestHistoryRepository_Search_MatchesTitle(t *testing.T) { | |
| 382 | + | ctx := historyTestCtx() | |
| 383 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 384 | + | ||
| 385 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 386 | + | require.NoError(t, err) | |
| 387 | + | t.Cleanup(func() { _ = db.Close() }) | |
| 388 | + | ||
| 389 | + | repo := sqlite.NewHistoryRepository(db) | |
| 390 | + | ||
| 391 | + | // Save entry with specific title that doesn't appear in URL | |
| 392 | + | entry := &entity.HistoryEntry{ | |
| 393 | + | URL: "https://docs.example.com/guide", | |
| 394 | + | Title: "Comprehensive Documentation Guide", | |
| 395 | + | } | |
| 396 | + | require.NoError(t, repo.Save(ctx, entry)) | |
| 397 | + | ||
| 398 | + | // Search for "Documentation" - should find by title | |
| 399 | + | results, err := repo.Search(ctx, "Documentation", 10) | |
| 400 | + | require.NoError(t, err) | |
| 401 | + | require.Len(t, results, 1) | |
| 402 | + | assert.Equal(t, "https://docs.example.com/guide", results[0].Entry.URL) | |
| 403 | + | } | |
| 404 | + | ||
| 405 | + | func TestHistoryRepository_Search_EmptyQuery(t *testing.T) { | |
| 406 | + | ctx := historyTestCtx() | |
| 407 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 408 | + | ||
| 409 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 410 | + | require.NoError(t, err) | |
| 411 | + | t.Cleanup(func() { _ = db.Close() }) | |
| 412 | + | ||
| 413 | + | repo := sqlite.NewHistoryRepository(db) | |
| 414 | + | ||
| 415 | + | // Save an entry | |
| 416 | + | require.NoError(t, repo.Save(ctx, &entity.HistoryEntry{ | |
| 417 | + | URL: "https://example.com", | |
| 418 | + | Title: "Example", | |
| 419 | + | })) | |
| 420 | + | ||
| 421 | + | // Empty query should return empty results, no error | |
| 422 | + | results, err := repo.Search(ctx, "", 10) | |
| 423 | + | require.NoError(t, err) | |
| 424 | + | assert.Empty(t, results) | |
| 425 | + | ||
| 426 | + | // Whitespace-only query should also return empty results | |
| 427 | + | results, err = repo.Search(ctx, " ", 10) | |
| 428 | + | require.NoError(t, err) | |
| 429 | + | assert.Empty(t, results) | |
| 430 | + | } | |
| 431 | + | ||
| 432 | + | func TestHistoryRepository_Search_NoResults(t *testing.T) { | |
| 433 | + | ctx := historyTestCtx() | |
| 434 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 435 | + | ||
| 436 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 437 | + | require.NoError(t, err) | |
| 438 | + | t.Cleanup(func() { _ = db.Close() }) | |
| 439 | + | ||
| 440 | + | repo := sqlite.NewHistoryRepository(db) | |
| 441 | + | ||
| 442 | + | // Save an entry | |
| 443 | + | require.NoError(t, repo.Save(ctx, &entity.HistoryEntry{ | |
| 444 | + | URL: "https://example.com", | |
| 445 | + | Title: "Example", | |
| 446 | + | })) | |
| 447 | + | ||
| 448 | + | // Search for something that doesn't exist | |
| 449 | + | results, err := repo.Search(ctx, "nonexistent", 10) | |
| 450 | + | require.NoError(t, err) | |
| 451 | + | assert.Empty(t, results) | |
| 452 | + | } | |
| 453 | + | ||
| 454 | + | func TestHistoryRepository_Search_PrefixMatching(t *testing.T) { | |
| 455 | + | ctx := historyTestCtx() | |
| 456 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 457 | + | ||
| 458 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 459 | + | require.NoError(t, err) | |
| 460 | + | t.Cleanup(func() { _ = db.Close() }) | |
| 461 | + | ||
| 462 | + | repo := sqlite.NewHistoryRepository(db) | |
| 463 | + | ||
| 464 | + | // Save entries | |
| 465 | + | entries := []*entity.HistoryEntry{ | |
| 466 | + | {URL: "https://github.com", Title: "GitHub"}, | |
| 467 | + | {URL: "https://gitlab.com", Title: "GitLab"}, | |
| 468 | + | {URL: "https://example.com", Title: "Example"}, | |
| 469 | + | } | |
| 470 | + | for _, e := range entries { | |
| 471 | + | require.NoError(t, repo.Save(ctx, e)) | |
| 472 | + | } | |
| 473 | + | ||
| 474 | + | // Search for "git" - should find both github and gitlab (prefix match) | |
| 475 | + | results, err := repo.Search(ctx, "git", 10) | |
| 476 | + | require.NoError(t, err) | |
| 477 | + | require.Len(t, results, 2) | |
| 478 | + | ||
| 479 | + | // Verify both git* entries are found | |
| 480 | + | urls := make(map[string]bool) | |
| 481 | + | for _, r := range results { | |
| 482 | + | urls[r.Entry.URL] = true | |
| 483 | + | } | |
| 484 | + | assert.True(t, urls["https://github.com"]) | |
| 485 | + | assert.True(t, urls["https://gitlab.com"]) | |
| 486 | + | } | |
| @@ -323,3 +323,164 @@ func TestHistoryRepository_GetAllMostVisited(t *testing.T) { | |||||
| 323 | assert.Equal(t, "https://github.com", results[0].URL) | 323 | assert.Equal(t, "https://github.com", results[0].URL) | ||
| 324 | assert.Equal(t, int64(4), results[0].VisitCount) | 324 | assert.Equal(t, int64(4), results[0].VisitCount) | ||
| 325 | } | 325 | } | ||
| 326 | + | ||||
| 327 | + | func TestHistoryRepository_Search_SingleWord(t *testing.T) { | |||
| 328 | + | ctx := historyTestCtx() | |||
| 329 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |||
| 330 | + | ||||
| 331 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |||
| 332 | + | require.NoError(t, err) | |||
| 333 | + | t.Cleanup(func() { _ = db.Close() }) | |||
| 334 | + | ||||
| 335 | + | repo := sqlite.NewHistoryRepository(db) | |||
| 336 | + | ||||
| 337 | + | // Save entries | |||
| 338 | + | entries := []*entity.HistoryEntry{ | |||
| 339 | + | {URL: "https://github.com/user/repo", Title: "My GitHub Repository"}, | |||
| 340 | + | {URL: "https://gitlab.com/project", Title: "GitLab Project"}, | |||
| 341 | + | {URL: "https://example.com", Title: "Example Site"}, | |||
| 342 | + | } | |||
| 343 | + | for _, e := range entries { | |||
| 344 | + | require.NoError(t, repo.Save(ctx, e)) | |||
| 345 | + | } | |||
| 346 | + | ||||
| 347 | + | // Search for "github" - should find the github entry | |||
| 348 | + | results, err := repo.Search(ctx, "github", 10) | |||
| 349 | + | require.NoError(t, err) | |||
| 350 | + | require.Len(t, results, 1) | |||
| 351 | + | assert.Equal(t, "https://github.com/user/repo", results[0].Entry.URL) | |||
| 352 | + | } | |||
| 353 | + | ||||
| 354 | + | func TestHistoryRepository_Search_MultiWord(t *testing.T) { | |||
| 355 | + | ctx := historyTestCtx() | |||
| 356 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |||
| 357 | + | ||||
| 358 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |||
| 359 | + | require.NoError(t, err) | |||
| 360 | + | t.Cleanup(func() { _ = db.Close() }) | |||
| 361 | + | ||||
| 362 | + | repo := sqlite.NewHistoryRepository(db) | |||
| 363 | + | ||||
| 364 | + | // Save entries | |||
| 365 | + | entries := []*entity.HistoryEntry{ | |||
| 366 | + | {URL: "https://github.com/issues", Title: "GitHub Issues"}, | |||
| 367 | + | {URL: "https://github.com/pulls", Title: "GitHub Pull Requests"}, | |||
| 368 | + | {URL: "https://jira.com/issues", Title: "Jira Issues"}, | |||
| 369 | + | } | |||
| 370 | + | for _, e := range entries { | |||
| 371 | + | require.NoError(t, repo.Save(ctx, e)) | |||
| 372 | + | } | |||
| 373 | + | ||||
| 374 | + | // Search for "github issues" - should find only the github issues entry | |||
| 375 | + | results, err := repo.Search(ctx, "github issues", 10) | |||
| 376 | + | require.NoError(t, err) | |||
| 377 | + | require.Len(t, results, 1) | |||
| 378 | + | assert.Equal(t, "https://github.com/issues", results[0].Entry.URL) | |||
| 379 | + | } | |||
| 380 | + | ||||
| 381 | + | func TestHistoryRepository_Search_MatchesTitle(t *testing.T) { | |||
| 382 | + | ctx := historyTestCtx() | |||
| 383 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |||
| 384 | + | ||||
| 385 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |||
| 386 | + | require.NoError(t, err) | |||
| 387 | + | t.Cleanup(func() { _ = db.Close() }) | |||
| 388 | + | ||||
| 389 | + | repo := sqlite.NewHistoryRepository(db) | |||
| 390 | + | ||||
| 391 | + | // Save entry with specific title that doesn't appear in URL | |||
| 392 | + | entry := &entity.HistoryEntry{ | |||
| 393 | + | URL: "https://docs.example.com/guide", | |||
| 394 | + | Title: "Comprehensive Documentation Guide", | |||
| 395 | + | } | |||
| 396 | + | require.NoError(t, repo.Save(ctx, entry)) | |||
| 397 | + | ||||
| 398 | + | // Search for "Documentation" - should find by title | |||
| 399 | + | results, err := repo.Search(ctx, "Documentation", 10) | |||
| 400 | + | require.NoError(t, err) | |||
| 401 | + | require.Len(t, results, 1) | |||
| 402 | + | assert.Equal(t, "https://docs.example.com/guide", results[0].Entry.URL) | |||
| 403 | + | } | |||
| 404 | + | ||||
| 405 | + | func TestHistoryRepository_Search_EmptyQuery(t *testing.T) { | |||
| 406 | + | ctx := historyTestCtx() | |||
| 407 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |||
| 408 | + | ||||
| 409 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |||
| 410 | + | require.NoError(t, err) | |||
| 411 | + | t.Cleanup(func() { _ = db.Close() }) | |||
| 412 | + | ||||
| 413 | + | repo := sqlite.NewHistoryRepository(db) | |||
| 414 | + | ||||
| 415 | + | // Save an entry | |||
| 416 | + | require.NoError(t, repo.Save(ctx, &entity.HistoryEntry{ | |||
| 417 | + | URL: "https://example.com", | |||
| 418 | + | Title: "Example", | |||
| 419 | + | })) | |||
| 420 | + | ||||
| 421 | + | // Empty query should return empty results, no error | |||
| 422 | + | results, err := repo.Search(ctx, "", 10) | |||
| 423 | + | require.NoError(t, err) | |||
| 424 | + | assert.Empty(t, results) | |||
| 425 | + | ||||
| 426 | + | // Whitespace-only query should also return empty results | |||
| 427 | + | results, err = repo.Search(ctx, " ", 10) | |||
| 428 | + | require.NoError(t, err) | |||
| 429 | + | assert.Empty(t, results) | |||
| 430 | + | } | |||
| 431 | + | ||||
| 432 | + | func TestHistoryRepository_Search_NoResults(t *testing.T) { | |||
| 433 | + | ctx := historyTestCtx() | |||
| 434 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |||
| 435 | + | ||||
| 436 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |||
| 437 | + | require.NoError(t, err) | |||
| 438 | + | t.Cleanup(func() { _ = db.Close() }) | |||
| 439 | + | ||||
| 440 | + | repo := sqlite.NewHistoryRepository(db) | |||
| 441 | + | ||||
| 442 | + | // Save an entry | |||
| 443 | + | require.NoError(t, repo.Save(ctx, &entity.HistoryEntry{ | |||
| 444 | + | URL: "https://example.com", | |||
| 445 | + | Title: "Example", | |||
| 446 | + | })) | |||
| 447 | + | ||||
| 448 | + | // Search for something that doesn't exist | |||
| 449 | + | results, err := repo.Search(ctx, "nonexistent", 10) | |||
| 450 | + | require.NoError(t, err) | |||
| 451 | + | assert.Empty(t, results) | |||
| 452 | + | } | |||
| 453 | + | ||||
| 454 | + | func TestHistoryRepository_Search_PrefixMatching(t *testing.T) { | |||
| 455 | + | ctx := historyTestCtx() | |||
| 456 | + | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |||
| 457 | + | ||||
| 458 | + | db, err := sqlite.NewConnection(ctx, dbPath) | |||
| 459 | + | require.NoError(t, err) | |||
| 460 | + | t.Cleanup(func() { _ = db.Close() }) | |||
| 461 | + | ||||
| 462 | + | repo := sqlite.NewHistoryRepository(db) | |||
| 463 | + | ||||
| 464 | + | // Save entries | |||
| 465 | + | entries := []*entity.HistoryEntry{ | |||
| 466 | + | {URL: "https://github.com", Title: "GitHub"}, | |||
| 467 | + | {URL: "https://gitlab.com", Title: "GitLab"}, | |||
| 468 | + | {URL: "https://example.com", Title: "Example"}, | |||
| 469 | + | } | |||
| 470 | + | for _, e := range entries { | |||
| 471 | + | require.NoError(t, repo.Save(ctx, e)) | |||
| 472 | + | } | |||
| 473 | + | ||||
| 474 | + | // Search for "git" - should find both github and gitlab (prefix match) | |||
| 475 | + | results, err := repo.Search(ctx, "git", 10) | |||
| 476 | + | require.NoError(t, err) | |||
| 477 | + | require.Len(t, results, 2) | |||
| 478 | + | ||||
| 479 | + | // Verify both git* entries are found | |||
| 480 | + | urls := make(map[string]bool) | |||
| 481 | + | for _, r := range results { | |||
| 482 | + | urls[r.Entry.URL] = true | |||
| 483 | + | } | |||
| 484 | + | assert.True(t, urls["https://github.com"]) | |||
| 485 | + | assert.True(t, urls["https://gitlab.com"]) | |||
| 486 | + | } | |||
M internal/infrastructure/persistence/sqlite/queries/history.sql +12 -4
| @@ -49,13 +49,21 @@ SELECT date(last_visited) as day, COUNT(*) as entries, SUM(visit_count) as visit | |||
| 49 | 49 | -- name: DeleteHistoryByDomain :exec | |
| 50 | 50 | DELETE FROM history WHERE url LIKE '%://' || ? || '/%' OR url LIKE '%://' || ? || '?%' OR url LIKE '%://' || ? || '#%' OR url LIKE '%://' || ?; | |
| 51 | 51 | ||
| 52 | - | -- name: SearchHistoryFTS :many | |
| 52 | + | -- name: SearchHistoryFTSUrl :many | |
| 53 | 53 | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | |
| 54 | 54 | FROM history_fts fts | |
| 55 | 55 | JOIN history h ON fts.rowid = h.id | |
| 56 | - | WHERE fts.url MATCH ? | |
| 57 | - | ORDER BY bm25(history_fts) | |
| 58 | - | LIMIT ?; | |
| 56 | + | WHERE fts.url MATCH @query | |
| 57 | + | ORDER BY h.visit_count DESC, h.last_visited DESC | |
| 58 | + | LIMIT @limit; | |
| 59 | + | ||
| 60 | + | -- name: SearchHistoryFTSTitle :many | |
| 61 | + | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | |
| 62 | + | FROM history_fts fts | |
| 63 | + | JOIN history h ON fts.rowid = h.id | |
| 64 | + | WHERE fts.title MATCH @query | |
| 65 | + | ORDER BY h.visit_count DESC, h.last_visited DESC | |
| 66 | + | LIMIT @limit; | |
| 59 | 67 | ||
| 60 | 68 | -- name: GetRecentHistorySince :many | |
| 61 | 69 | SELECT * FROM history | |
| @@ -49,13 +49,21 @@ SELECT date(last_visited) as day, COUNT(*) as entries, SUM(visit_count) as visit | |||||
| 49 | -- name: DeleteHistoryByDomain :exec | 49 | -- name: DeleteHistoryByDomain :exec | ||
| 50 | DELETE FROM history WHERE url LIKE '%://' || ? || '/%' OR url LIKE '%://' || ? || '?%' OR url LIKE '%://' || ? || '#%' OR url LIKE '%://' || ?; | 50 | DELETE FROM history WHERE url LIKE '%://' || ? || '/%' OR url LIKE '%://' || ? || '?%' OR url LIKE '%://' || ? || '#%' OR url LIKE '%://' || ?; | ||
| 51 | 51 | ||||
| 52 | - | -- name: SearchHistoryFTS :many | 52 | + | -- name: SearchHistoryFTSUrl :many |
| 53 | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | 53 | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | ||
| 54 | FROM history_fts fts | 54 | FROM history_fts fts | ||
| 55 | JOIN history h ON fts.rowid = h.id | 55 | JOIN history h ON fts.rowid = h.id | ||
| 56 | - | WHERE fts.url MATCH ? | 56 | + | WHERE fts.url MATCH @query |
| 57 | - | ORDER BY bm25(history_fts) | 57 | + | ORDER BY h.visit_count DESC, h.last_visited DESC |
| 58 | - | LIMIT ?; | 58 | + | LIMIT @limit; |
| 59 | + | ||||
| 60 | + | -- name: SearchHistoryFTSTitle :many | |||
| 61 | + | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | |||
| 62 | + | FROM history_fts fts | |||
| 63 | + | JOIN history h ON fts.rowid = h.id | |||
| 64 | + | WHERE fts.title MATCH @query | |||
| 65 | + | ORDER BY h.visit_count DESC, h.last_visited DESC | |||
| 66 | + | LIMIT @limit; | |||
| 59 | 67 | ||||
| 60 | -- name: GetRecentHistorySince :many | 68 | -- name: GetRecentHistorySince :many | ||
| 61 | SELECT * FROM history | 69 | SELECT * FROM history | ||
M internal/infrastructure/persistence/sqlite/sqlc/history.sql.go +53 -8
| @@ -439,22 +439,67 @@ func (q *Queries) SearchHistory(ctx context.Context, arg SearchHistoryParams) ([ | |||
| 439 | 439 | return items, nil | |
| 440 | 440 | } | |
| 441 | 441 | ||
| 442 | - | const SearchHistoryFTS = `-- name: SearchHistoryFTS :many | |
| 442 | + | const SearchHistoryFTSTitle = `-- name: SearchHistoryFTSTitle :many | |
| 443 | 443 | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | |
| 444 | 444 | FROM history_fts fts | |
| 445 | 445 | JOIN history h ON fts.rowid = h.id | |
| 446 | - | WHERE fts.url MATCH ? | |
| 447 | - | ORDER BY bm25(history_fts) | |
| 448 | - | LIMIT ? | |
| 446 | + | WHERE fts.title MATCH ?1 | |
| 447 | + | ORDER BY h.visit_count DESC, h.last_visited DESC | |
| 448 | + | LIMIT ?2 | |
| 449 | + | ` | |
| 450 | + | ||
| 451 | + | type SearchHistoryFTSTitleParams struct { | |
| 452 | + | Query string `json:"query"` | |
| 453 | + | Limit int64 `json:"limit"` | |
| 454 | + | } | |
| 455 | + | ||
| 456 | + | func (q *Queries) SearchHistoryFTSTitle(ctx context.Context, arg SearchHistoryFTSTitleParams) ([]History, error) { | |
| 457 | + | rows, err := q.db.QueryContext(ctx, SearchHistoryFTSTitle, arg.Query, arg.Limit) | |
| 458 | + | if err != nil { | |
| 459 | + | return nil, err | |
| 460 | + | } | |
| 461 | + | defer rows.Close() | |
| 462 | + | items := []History{} | |
| 463 | + | for rows.Next() { | |
| 464 | + | var i History | |
| 465 | + | if err := rows.Scan( | |
| 466 | + | &i.ID, | |
| 467 | + | &i.Url, | |
| 468 | + | &i.Title, | |
| 469 | + | &i.FaviconUrl, | |
| 470 | + | &i.VisitCount, | |
| 471 | + | &i.LastVisited, | |
| 472 | + | &i.CreatedAt, | |
| 473 | + | ); err != nil { | |
| 474 | + | return nil, err | |
| 475 | + | } | |
| 476 | + | items = append(items, i) | |
| 477 | + | } | |
| 478 | + | if err := rows.Close(); err != nil { | |
| 479 | + | return nil, err | |
| 480 | + | } | |
| 481 | + | if err := rows.Err(); err != nil { | |
| 482 | + | return nil, err | |
| 483 | + | } | |
| 484 | + | return items, nil | |
| 485 | + | } | |
| 486 | + | ||
| 487 | + | const SearchHistoryFTSUrl = `-- name: SearchHistoryFTSUrl :many | |
| 488 | + | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | |
| 489 | + | FROM history_fts fts | |
| 490 | + | JOIN history h ON fts.rowid = h.id | |
| 491 | + | WHERE fts.url MATCH ?1 | |
| 492 | + | ORDER BY h.visit_count DESC, h.last_visited DESC | |
| 493 | + | LIMIT ?2 | |
| 449 | 494 | ` | |
| 450 | 495 | ||
| 451 | - | type SearchHistoryFTSParams struct { | |
| 452 | - | Url string `json:"url"` | |
| 496 | + | type SearchHistoryFTSUrlParams struct { | |
| 497 | + | Query string `json:"query"` | |
| 453 | 498 | Limit int64 `json:"limit"` | |
| 454 | 499 | } | |
| 455 | 500 | ||
| 456 | - | func (q *Queries) SearchHistoryFTS(ctx context.Context, arg SearchHistoryFTSParams) ([]History, error) { | |
| 457 | - | rows, err := q.db.QueryContext(ctx, SearchHistoryFTS, arg.Url, arg.Limit) | |
| 501 | + | func (q *Queries) SearchHistoryFTSUrl(ctx context.Context, arg SearchHistoryFTSUrlParams) ([]History, error) { | |
| 502 | + | rows, err := q.db.QueryContext(ctx, SearchHistoryFTSUrl, arg.Query, arg.Limit) | |
| 458 | 503 | if err != nil { | |
| 459 | 504 | return nil, err | |
| 460 | 505 | } | |
| @@ -439,22 +439,67 @@ func (q *Queries) SearchHistory(ctx context.Context, arg SearchHistoryParams) ([ | |||||
| 439 | return items, nil | 439 | return items, nil | ||
| 440 | } | 440 | } | ||
| 441 | 441 | ||||
| 442 | - | const SearchHistoryFTS = `-- name: SearchHistoryFTS :many | 442 | + | const SearchHistoryFTSTitle = `-- name: SearchHistoryFTSTitle :many |
| 443 | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | 443 | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | ||
| 444 | FROM history_fts fts | 444 | FROM history_fts fts | ||
| 445 | JOIN history h ON fts.rowid = h.id | 445 | JOIN history h ON fts.rowid = h.id | ||
| 446 | - | WHERE fts.url MATCH ? | 446 | + | WHERE fts.title MATCH ?1 |
| 447 | - | ORDER BY bm25(history_fts) | 447 | + | ORDER BY h.visit_count DESC, h.last_visited DESC |
| 448 | - | LIMIT ? | 448 | + | LIMIT ?2 |
| 449 | + | ` | |||
| 450 | + | ||||
| 451 | + | type SearchHistoryFTSTitleParams struct { | |||
| 452 | + | Query string `json:"query"` | |||
| 453 | + | Limit int64 `json:"limit"` | |||
| 454 | + | } | |||
| 455 | + | ||||
| 456 | + | func (q *Queries) SearchHistoryFTSTitle(ctx context.Context, arg SearchHistoryFTSTitleParams) ([]History, error) { | |||
| 457 | + | rows, err := q.db.QueryContext(ctx, SearchHistoryFTSTitle, arg.Query, arg.Limit) | |||
| 458 | + | if err != nil { | |||
| 459 | + | return nil, err | |||
| 460 | + | } | |||
| 461 | + | defer rows.Close() | |||
| 462 | + | items := []History{} | |||
| 463 | + | for rows.Next() { | |||
| 464 | + | var i History | |||
| 465 | + | if err := rows.Scan( | |||
| 466 | + | &i.ID, | |||
| 467 | + | &i.Url, | |||
| 468 | + | &i.Title, | |||
| 469 | + | &i.FaviconUrl, | |||
| 470 | + | &i.VisitCount, | |||
| 471 | + | &i.LastVisited, | |||
| 472 | + | &i.CreatedAt, | |||
| 473 | + | ); err != nil { | |||
| 474 | + | return nil, err | |||
| 475 | + | } | |||
| 476 | + | items = append(items, i) | |||
| 477 | + | } | |||
| 478 | + | if err := rows.Close(); err != nil { | |||
| 479 | + | return nil, err | |||
| 480 | + | } | |||
| 481 | + | if err := rows.Err(); err != nil { | |||
| 482 | + | return nil, err | |||
| 483 | + | } | |||
| 484 | + | return items, nil | |||
| 485 | + | } | |||
| 486 | + | ||||
| 487 | + | const SearchHistoryFTSUrl = `-- name: SearchHistoryFTSUrl :many | |||
| 488 | + | SELECT h.id, h.url, h.title, h.favicon_url, h.visit_count, h.last_visited, h.created_at | |||
| 489 | + | FROM history_fts fts | |||
| 490 | + | JOIN history h ON fts.rowid = h.id | |||
| 491 | + | WHERE fts.url MATCH ?1 | |||
| 492 | + | ORDER BY h.visit_count DESC, h.last_visited DESC | |||
| 493 | + | LIMIT ?2 | |||
| 449 | ` | 494 | ` | ||
| 450 | 495 | ||||
| 451 | - | type SearchHistoryFTSParams struct { | 496 | + | type SearchHistoryFTSUrlParams struct { |
| 452 | - | Url string `json:"url"` | 497 | + | Query string `json:"query"` |
| 453 | Limit int64 `json:"limit"` | 498 | Limit int64 `json:"limit"` | ||
| 454 | } | 499 | } | ||
| 455 | 500 | ||||
| 456 | - | func (q *Queries) SearchHistoryFTS(ctx context.Context, arg SearchHistoryFTSParams) ([]History, error) { | 501 | + | func (q *Queries) SearchHistoryFTSUrl(ctx context.Context, arg SearchHistoryFTSUrlParams) ([]History, error) { |
| 457 | - | rows, err := q.db.QueryContext(ctx, SearchHistoryFTS, arg.Url, arg.Limit) | 502 | + | rows, err := q.db.QueryContext(ctx, SearchHistoryFTSUrl, arg.Query, arg.Limit) |
| 458 | if err != nil { | 503 | if err != nil { | ||
| 459 | return nil, err | 504 | return nil, err | ||
| 460 | } | 505 | } | ||
M internal/infrastructure/persistence/sqlite/sqlc/querier.go +2 -1
| @@ -70,7 +70,8 @@ type Querier interface { | |||
| 70 | 70 | RemoveFromWhitelist(ctx context.Context, domain string) error | |
| 71 | 71 | RemoveTagFromFavorite(ctx context.Context, arg RemoveTagFromFavoriteParams) error | |
| 72 | 72 | SearchHistory(ctx context.Context, arg SearchHistoryParams) ([]History, error) | |
| 73 | - | SearchHistoryFTS(ctx context.Context, arg SearchHistoryFTSParams) ([]History, error) | |
| 73 | + | SearchHistoryFTSTitle(ctx context.Context, arg SearchHistoryFTSTitleParams) ([]History, error) | |
| 74 | + | SearchHistoryFTSUrl(ctx context.Context, arg SearchHistoryFTSUrlParams) ([]History, error) | |
| 74 | 75 | SetFavoriteFolder(ctx context.Context, arg SetFavoriteFolderParams) error | |
| 75 | 76 | SetFavoriteShortcut(ctx context.Context, arg SetFavoriteShortcutParams) error | |
| 76 | 77 | SetZoomLevel(ctx context.Context, arg SetZoomLevelParams) error | |
| @@ -70,7 +70,8 @@ type Querier interface { | |||||
| 70 | RemoveFromWhitelist(ctx context.Context, domain string) error | 70 | RemoveFromWhitelist(ctx context.Context, domain string) error | ||
| 71 | RemoveTagFromFavorite(ctx context.Context, arg RemoveTagFromFavoriteParams) error | 71 | RemoveTagFromFavorite(ctx context.Context, arg RemoveTagFromFavoriteParams) error | ||
| 72 | SearchHistory(ctx context.Context, arg SearchHistoryParams) ([]History, error) | 72 | SearchHistory(ctx context.Context, arg SearchHistoryParams) ([]History, error) | ||
| 73 | - | SearchHistoryFTS(ctx context.Context, arg SearchHistoryFTSParams) ([]History, error) | 73 | + | SearchHistoryFTSTitle(ctx context.Context, arg SearchHistoryFTSTitleParams) ([]History, error) |
| 74 | + | SearchHistoryFTSUrl(ctx context.Context, arg SearchHistoryFTSUrlParams) ([]History, error) | |||
| 74 | SetFavoriteFolder(ctx context.Context, arg SetFavoriteFolderParams) error | 75 | SetFavoriteFolder(ctx context.Context, arg SetFavoriteFolderParams) error | ||
| 75 | SetFavoriteShortcut(ctx context.Context, arg SetFavoriteShortcutParams) error | 76 | SetFavoriteShortcut(ctx context.Context, arg SetFavoriteShortcutParams) error | ||
| 76 | SetZoomLevel(ctx context.Context, arg SetZoomLevelParams) error | 77 | SetZoomLevel(ctx context.Context, arg SetZoomLevelParams) error | ||
A internal/infrastructure/webkit/handlers/clipboard.go +92 -0
| @@ -0,0 +1,92 @@ | |||
| 1 | + | package handlers | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "encoding/json" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/dumber/internal/application/port" | |
| 8 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 9 | + | "github.com/bnema/dumber/internal/infrastructure/webkit" | |
| 10 | + | "github.com/bnema/dumber/internal/logging" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // ClipboardHandler handles clipboard-related messages from webviews. | |
| 14 | + | type ClipboardHandler struct { | |
| 15 | + | clipboard port.Clipboard | |
| 16 | + | configGetter func() *config.Config | |
| 17 | + | onCopied func(textLen int) // Called after successful auto-copy (for toast notification) | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | // NewClipboardHandler creates a new ClipboardHandler. | |
| 21 | + | func NewClipboardHandler(clipboard port.Clipboard, configGetter func() *config.Config, onCopied func(textLen int)) *ClipboardHandler { | |
| 22 | + | return &ClipboardHandler{ | |
| 23 | + | clipboard: clipboard, | |
| 24 | + | configGetter: configGetter, | |
| 25 | + | onCopied: onCopied, | |
| 26 | + | } | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | // autoCopyRequest represents the payload for auto-copy selection messages. | |
| 30 | + | type autoCopyRequest struct { | |
| 31 | + | Text string `json:"text"` | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | // HandleAutoCopySelection handles the auto_copy_selection message from JS. | |
| 35 | + | // It copies the selected text to the clipboard if the feature is enabled. | |
| 36 | + | func (h *ClipboardHandler) HandleAutoCopySelection() webkit.MessageHandler { | |
| 37 | + | return webkit.MessageHandlerFunc(func(ctx context.Context, _ webkit.WebViewID, payload json.RawMessage) (any, error) { | |
| 38 | + | log := logging.FromContext(ctx) | |
| 39 | + | ||
| 40 | + | // Check if feature is enabled | |
| 41 | + | cfg := h.configGetter() | |
| 42 | + | if cfg == nil || !cfg.Clipboard.AutoCopyOnSelection { | |
| 43 | + | return nil, nil | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | var req autoCopyRequest | |
| 47 | + | if err := json.Unmarshal(payload, &req); err != nil { | |
| 48 | + | log.Debug().Err(err).Msg("failed to unmarshal auto-copy payload") | |
| 49 | + | return nil, nil // Silently ignore malformed requests | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | // Skip empty or very short selections (< 2 chars) | |
| 53 | + | if len(req.Text) < 2 { | |
| 54 | + | return nil, nil | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | if err := h.clipboard.WriteText(ctx, req.Text); err != nil { | |
| 58 | + | log.Debug().Err(err).Msg("failed to write selection to clipboard") | |
| 59 | + | return nil, nil // Don't propagate error to JS | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | log.Debug().Int("length", len(req.Text)).Msg("auto-copied selection to clipboard") | |
| 63 | + | ||
| 64 | + | // Notify UI for toast feedback (if callback is set) | |
| 65 | + | if h.onCopied != nil { | |
| 66 | + | h.onCopied(len(req.Text)) | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | return nil, nil | |
| 70 | + | }) | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | // RegisterClipboardHandlers registers clipboard handlers with the router. | |
| 74 | + | func RegisterClipboardHandlers( | |
| 75 | + | ctx context.Context, | |
| 76 | + | router *webkit.MessageRouter, | |
| 77 | + | clipboard port.Clipboard, | |
| 78 | + | configGetter func() *config.Config, | |
| 79 | + | onCopied func(textLen int), | |
| 80 | + | ) error { | |
| 81 | + | handler := NewClipboardHandler(clipboard, configGetter, onCopied) | |
| 82 | + | ||
| 83 | + | // Register auto_copy_selection handler | |
| 84 | + | if err := router.RegisterHandler("auto_copy_selection", handler.HandleAutoCopySelection()); err != nil { | |
| 85 | + | return err | |
| 86 | + | } | |
| 87 | + | ||
| 88 | + | log := logging.FromContext(ctx) | |
| 89 | + | log.Info().Msg("registered clipboard handlers") | |
| 90 | + | ||
| 91 | + | return nil | |
| 92 | + | } | |
| @@ -0,0 +1,92 @@ | |||||
| 1 | + | package handlers | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "encoding/json" | |||
| 6 | + | ||||
| 7 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 8 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |||
| 9 | + | "github.com/bnema/dumber/internal/infrastructure/webkit" | |||
| 10 | + | "github.com/bnema/dumber/internal/logging" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // ClipboardHandler handles clipboard-related messages from webviews. | |||
| 14 | + | type ClipboardHandler struct { | |||
| 15 | + | clipboard port.Clipboard | |||
| 16 | + | configGetter func() *config.Config | |||
| 17 | + | onCopied func(textLen int) // Called after successful auto-copy (for toast notification) | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | // NewClipboardHandler creates a new ClipboardHandler. | |||
| 21 | + | func NewClipboardHandler(clipboard port.Clipboard, configGetter func() *config.Config, onCopied func(textLen int)) *ClipboardHandler { | |||
| 22 | + | return &ClipboardHandler{ | |||
| 23 | + | clipboard: clipboard, | |||
| 24 | + | configGetter: configGetter, | |||
| 25 | + | onCopied: onCopied, | |||
| 26 | + | } | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | // autoCopyRequest represents the payload for auto-copy selection messages. | |||
| 30 | + | type autoCopyRequest struct { | |||
| 31 | + | Text string `json:"text"` | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | // HandleAutoCopySelection handles the auto_copy_selection message from JS. | |||
| 35 | + | // It copies the selected text to the clipboard if the feature is enabled. | |||
| 36 | + | func (h *ClipboardHandler) HandleAutoCopySelection() webkit.MessageHandler { | |||
| 37 | + | return webkit.MessageHandlerFunc(func(ctx context.Context, _ webkit.WebViewID, payload json.RawMessage) (any, error) { | |||
| 38 | + | log := logging.FromContext(ctx) | |||
| 39 | + | ||||
| 40 | + | // Check if feature is enabled | |||
| 41 | + | cfg := h.configGetter() | |||
| 42 | + | if cfg == nil || !cfg.Clipboard.AutoCopyOnSelection { | |||
| 43 | + | return nil, nil | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | var req autoCopyRequest | |||
| 47 | + | if err := json.Unmarshal(payload, &req); err != nil { | |||
| 48 | + | log.Debug().Err(err).Msg("failed to unmarshal auto-copy payload") | |||
| 49 | + | return nil, nil // Silently ignore malformed requests | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | // Skip empty or very short selections (< 2 chars) | |||
| 53 | + | if len(req.Text) < 2 { | |||
| 54 | + | return nil, nil | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | if err := h.clipboard.WriteText(ctx, req.Text); err != nil { | |||
| 58 | + | log.Debug().Err(err).Msg("failed to write selection to clipboard") | |||
| 59 | + | return nil, nil // Don't propagate error to JS | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | log.Debug().Int("length", len(req.Text)).Msg("auto-copied selection to clipboard") | |||
| 63 | + | ||||
| 64 | + | // Notify UI for toast feedback (if callback is set) | |||
| 65 | + | if h.onCopied != nil { | |||
| 66 | + | h.onCopied(len(req.Text)) | |||
| 67 | + | } | |||
| 68 | + | ||||
| 69 | + | return nil, nil | |||
| 70 | + | }) | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | // RegisterClipboardHandlers registers clipboard handlers with the router. | |||
| 74 | + | func RegisterClipboardHandlers( | |||
| 75 | + | ctx context.Context, | |||
| 76 | + | router *webkit.MessageRouter, | |||
| 77 | + | clipboard port.Clipboard, | |||
| 78 | + | configGetter func() *config.Config, | |||
| 79 | + | onCopied func(textLen int), | |||
| 80 | + | ) error { | |||
| 81 | + | handler := NewClipboardHandler(clipboard, configGetter, onCopied) | |||
| 82 | + | ||||
| 83 | + | // Register auto_copy_selection handler | |||
| 84 | + | if err := router.RegisterHandler("auto_copy_selection", handler.HandleAutoCopySelection()); err != nil { | |||
| 85 | + | return err | |||
| 86 | + | } | |||
| 87 | + | ||||
| 88 | + | log := logging.FromContext(ctx) | |||
| 89 | + | log.Info().Msg("registered clipboard handlers") | |||
| 90 | + | ||||
| 91 | + | return nil | |||
| 92 | + | } | |||
M internal/infrastructure/webkit/handlers/register.go +14 -2
| @@ -3,15 +3,20 @@ package handlers | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | ||
| 6 | + | "github.com/bnema/dumber/internal/application/port" | |
| 6 | 7 | "github.com/bnema/dumber/internal/application/usecase" | |
| 8 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 7 | 9 | "github.com/bnema/dumber/internal/infrastructure/webkit" | |
| 8 | 10 | "github.com/bnema/dumber/internal/infrastructure/webkit/handlers/homepage" | |
| 9 | 11 | ) | |
| 10 | 12 | ||
| 11 | 13 | // Config holds all dependencies for message handlers. | |
| 12 | 14 | type Config struct { | |
| 13 | - | HistoryUC *usecase.SearchHistoryUseCase | |
| 14 | - | FavoritesUC *usecase.ManageFavoritesUseCase | |
| 15 | + | HistoryUC *usecase.SearchHistoryUseCase | |
| 16 | + | FavoritesUC *usecase.ManageFavoritesUseCase | |
| 17 | + | Clipboard port.Clipboard | |
| 18 | + | ConfigGetter func() *config.Config | |
| 19 | + | OnClipboardCopied func(textLen int) // Called when auto-copy completes (for toast notification) | |
| 15 | 20 | } | |
| 16 | 21 | ||
| 17 | 22 | // RegisterAll registers all message handlers with the router. | |
| @@ -31,6 +36,13 @@ func RegisterAll(ctx context.Context, router *webkit.MessageRouter, cfg Config) | |||
| 31 | 36 | return err | |
| 32 | 37 | } | |
| 33 | 38 | ||
| 39 | + | // Clipboard handlers (for auto-copy on selection feature) | |
| 40 | + | if cfg.Clipboard != nil && cfg.ConfigGetter != nil { | |
| 41 | + | if err := RegisterClipboardHandlers(ctx, router, cfg.Clipboard, cfg.ConfigGetter, cfg.OnClipboardCopied); err != nil { | |
| 42 | + | return err | |
| 43 | + | } | |
| 44 | + | } | |
| 45 | + | ||
| 34 | 46 | // Future handler groups go here: | |
| 35 | 47 | // - settings handlers | |
| 36 | 48 | // - downloads handlers | |
| @@ -3,15 +3,20 @@ package handlers | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | 5 | ||||
| 6 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 6 | "github.com/bnema/dumber/internal/application/usecase" | 7 | "github.com/bnema/dumber/internal/application/usecase" | ||
| 8 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |||
| 7 | "github.com/bnema/dumber/internal/infrastructure/webkit" | 9 | "github.com/bnema/dumber/internal/infrastructure/webkit" | ||
| 8 | "github.com/bnema/dumber/internal/infrastructure/webkit/handlers/homepage" | 10 | "github.com/bnema/dumber/internal/infrastructure/webkit/handlers/homepage" | ||
| 9 | ) | 11 | ) | ||
| 10 | 12 | ||||
| 11 | // Config holds all dependencies for message handlers. | 13 | // Config holds all dependencies for message handlers. | ||
| 12 | type Config struct { | 14 | type Config struct { | ||
| 13 | - | HistoryUC *usecase.SearchHistoryUseCase | 15 | + | HistoryUC *usecase.SearchHistoryUseCase |
| 14 | - | FavoritesUC *usecase.ManageFavoritesUseCase | 16 | + | FavoritesUC *usecase.ManageFavoritesUseCase |
| 17 | + | Clipboard port.Clipboard | |||
| 18 | + | ConfigGetter func() *config.Config | |||
| 19 | + | OnClipboardCopied func(textLen int) // Called when auto-copy completes (for toast notification) | |||
| 15 | } | 20 | } | ||
| 16 | 21 | ||||
| 17 | // RegisterAll registers all message handlers with the router. | 22 | // RegisterAll registers all message handlers with the router. | ||
| @@ -31,6 +36,13 @@ func RegisterAll(ctx context.Context, router *webkit.MessageRouter, cfg Config) | |||||
| 31 | return err | 36 | return err | ||
| 32 | } | 37 | } | ||
| 33 | 38 | ||||
| 39 | + | // Clipboard handlers (for auto-copy on selection feature) | |||
| 40 | + | if cfg.Clipboard != nil && cfg.ConfigGetter != nil { | |||
| 41 | + | if err := RegisterClipboardHandlers(ctx, router, cfg.Clipboard, cfg.ConfigGetter, cfg.OnClipboardCopied); err != nil { | |||
| 42 | + | return err | |||
| 43 | + | } | |||
| 44 | + | } | |||
| 45 | + | ||||
| 34 | // Future handler groups go here: | 46 | // Future handler groups go here: | ||
| 35 | // - settings handlers | 47 | // - settings handlers | ||
| 36 | // - downloads handlers | 48 | // - downloads handlers | ||
M internal/infrastructure/webkit/injector.go +54 -4
| @@ -114,15 +114,43 @@ const internalDarkModeScriptTemplate = `(function() { | |||
| 114 | 114 | // internalPageAllowList restricts script injection to internal dumb:// pages only. | |
| 115 | 115 | var internalPageAllowList = []string{"dumb://*"} | |
| 116 | 116 | ||
| 117 | + | // autoCopySelectionScript is injected to enable auto-copy on text selection (zellij-style). | |
| 118 | + | // It listens for selection changes, debounces them, and posts to the Go message handler. | |
| 119 | + | // Skips selections in input fields/textareas and requires minimum 2 characters. | |
| 120 | + | const autoCopySelectionScript = `(function() { | |
| 121 | + | var debounceTimer = null; | |
| 122 | + | var MIN_LENGTH = 2; | |
| 123 | + | ||
| 124 | + | document.addEventListener('selectionchange', function() { | |
| 125 | + | // Skip if selection is inside input/textarea/contenteditable | |
| 126 | + | var activeEl = document.activeElement; | |
| 127 | + | if (activeEl && (activeEl.tagName === 'INPUT' || activeEl.tagName === 'TEXTAREA' || activeEl.isContentEditable)) { | |
| 128 | + | return; | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | clearTimeout(debounceTimer); | |
| 132 | + | debounceTimer = setTimeout(function() { | |
| 133 | + | var text = window.getSelection().toString().trim(); | |
| 134 | + | if (text.length >= MIN_LENGTH && window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.dumber) { | |
| 135 | + | window.webkit.messageHandlers.dumber.postMessage({ | |
| 136 | + | type: 'auto_copy_selection', | |
| 137 | + | payload: { text: text } | |
| 138 | + | }); | |
| 139 | + | } | |
| 140 | + | }, 300); | |
| 141 | + | }); | |
| 142 | + | })();` | |
| 143 | + | ||
| 117 | 144 | // ContentInjector encapsulates script injection into WebViews. | |
| 118 | 145 | // It injects dark mode detection scripts for internal pages (dumb://) | |
| 119 | 146 | // and theme CSS variables for WebUI styling. | |
| 120 | 147 | // External pages receive dark mode preference via libadwaita's StyleManager. | |
| 121 | 148 | // Implements port.ContentInjector interface. | |
| 122 | 149 | type ContentInjector struct { | |
| 123 | - | colorResolver port.ColorSchemeResolver | |
| 124 | - | themeCSSVars string // CSS custom property declarations for WebUI | |
| 125 | - | findCSS string // CSS for find-in-page highlight styling | |
| 150 | + | colorResolver port.ColorSchemeResolver | |
| 151 | + | themeCSSVars string // CSS custom property declarations for WebUI | |
| 152 | + | findCSS string // CSS for find-in-page highlight styling | |
| 153 | + | autoCopyConfigGetter func() bool // Dynamic getter for auto-copy config | |
| 126 | 154 | } | |
| 127 | 155 | ||
| 128 | 156 | // NewContentInjector creates a new injector instance. | |
| @@ -133,6 +161,12 @@ func NewContentInjector(resolver port.ColorSchemeResolver) *ContentInjector { | |||
| 133 | 161 | } | |
| 134 | 162 | } | |
| 135 | 163 | ||
| 164 | + | // SetAutoCopyConfigGetter sets the function to dynamically check if auto-copy is enabled. | |
| 165 | + | // This is called during script injection to determine whether to inject the selection listener. | |
| 166 | + | func (ci *ContentInjector) SetAutoCopyConfigGetter(getter func() bool) { | |
| 167 | + | ci.autoCopyConfigGetter = getter | |
| 168 | + | } | |
| 169 | + | ||
| 136 | 170 | // InjectThemeCSS stores CSS variables for injection into internal pages. | |
| 137 | 171 | // Implements port.ContentInjector interface. | |
| 138 | 172 | // The CSS will be injected when InjectScripts is called on WebView creation. | |
| @@ -252,5 +286,21 @@ func (ci *ContentInjector) InjectScripts(ctx context.Context, ucm *webkit.UserCo | |||
| 252 | 286 | } | |
| 253 | 287 | } | |
| 254 | 288 | ||
| 255 | - | log.Debug().Bool("prefers_dark", prefersDark).Msg("scripts injected") | |
| 289 | + | // 5. Inject auto-copy selection script for all pages (if enabled) | |
| 290 | + | autoCopyEnabled := ci.autoCopyConfigGetter != nil && ci.autoCopyConfigGetter() | |
| 291 | + | if autoCopyEnabled { | |
| 292 | + | addScript( | |
| 293 | + | webkit.NewUserScript( | |
| 294 | + | autoCopySelectionScript, | |
| 295 | + | webkit.UserContentInjectTopFrameValue, | |
| 296 | + | webkit.UserScriptInjectAtDocumentEndValue, | |
| 297 | + | nil, // All pages | |
| 298 | + | nil, | |
| 299 | + | ), | |
| 300 | + | "auto-copy-selection", | |
| 301 | + | ) | |
| 302 | + | log.Debug().Msg("auto-copy selection script injected") | |
| 303 | + | } | |
| 304 | + | ||
| 305 | + | log.Debug().Bool("prefers_dark", prefersDark).Bool("auto_copy", autoCopyEnabled).Msg("scripts injected") | |
| 256 | 306 | } | |
| @@ -114,15 +114,43 @@ const internalDarkModeScriptTemplate = `(function() { | |||||
| 114 | // internalPageAllowList restricts script injection to internal dumb:// pages only. | 114 | // internalPageAllowList restricts script injection to internal dumb:// pages only. | ||
| 115 | var internalPageAllowList = []string{"dumb://*"} | 115 | var internalPageAllowList = []string{"dumb://*"} | ||
| 116 | 116 | ||||
| 117 | + | // autoCopySelectionScript is injected to enable auto-copy on text selection (zellij-style). | |||
| 118 | + | // It listens for selection changes, debounces them, and posts to the Go message handler. | |||
| 119 | + | // Skips selections in input fields/textareas and requires minimum 2 characters. | |||
| 120 | + | const autoCopySelectionScript = `(function() { | |||
| 121 | + | var debounceTimer = null; | |||
| 122 | + | var MIN_LENGTH = 2; | |||
| 123 | + | ||||
| 124 | + | document.addEventListener('selectionchange', function() { | |||
| 125 | + | // Skip if selection is inside input/textarea/contenteditable | |||
| 126 | + | var activeEl = document.activeElement; | |||
| 127 | + | if (activeEl && (activeEl.tagName === 'INPUT' || activeEl.tagName === 'TEXTAREA' || activeEl.isContentEditable)) { | |||
| 128 | + | return; | |||
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | clearTimeout(debounceTimer); | |||
| 132 | + | debounceTimer = setTimeout(function() { | |||
| 133 | + | var text = window.getSelection().toString().trim(); | |||
| 134 | + | if (text.length >= MIN_LENGTH && window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.dumber) { | |||
| 135 | + | window.webkit.messageHandlers.dumber.postMessage({ | |||
| 136 | + | type: 'auto_copy_selection', | |||
| 137 | + | payload: { text: text } | |||
| 138 | + | }); | |||
| 139 | + | } | |||
| 140 | + | }, 300); | |||
| 141 | + | }); | |||
| 142 | + | })();` | |||
| 143 | + | ||||
| 117 | // ContentInjector encapsulates script injection into WebViews. | 144 | // ContentInjector encapsulates script injection into WebViews. | ||
| 118 | // It injects dark mode detection scripts for internal pages (dumb://) | 145 | // It injects dark mode detection scripts for internal pages (dumb://) | ||
| 119 | // and theme CSS variables for WebUI styling. | 146 | // and theme CSS variables for WebUI styling. | ||
| 120 | // External pages receive dark mode preference via libadwaita's StyleManager. | 147 | // External pages receive dark mode preference via libadwaita's StyleManager. | ||
| 121 | // Implements port.ContentInjector interface. | 148 | // Implements port.ContentInjector interface. | ||
| 122 | type ContentInjector struct { | 149 | type ContentInjector struct { | ||
| 123 | - | colorResolver port.ColorSchemeResolver | 150 | + | colorResolver port.ColorSchemeResolver |
| 124 | - | themeCSSVars string // CSS custom property declarations for WebUI | 151 | + | themeCSSVars string // CSS custom property declarations for WebUI |
| 125 | - | findCSS string // CSS for find-in-page highlight styling | 152 | + | findCSS string // CSS for find-in-page highlight styling |
| 153 | + | autoCopyConfigGetter func() bool // Dynamic getter for auto-copy config | |||
| 126 | } | 154 | } | ||
| 127 | 155 | ||||
| 128 | // NewContentInjector creates a new injector instance. | 156 | // NewContentInjector creates a new injector instance. | ||
| @@ -133,6 +161,12 @@ func NewContentInjector(resolver port.ColorSchemeResolver) *ContentInjector { | |||||
| 133 | } | 161 | } | ||
| 134 | } | 162 | } | ||
| 135 | 163 | ||||
| 164 | + | // SetAutoCopyConfigGetter sets the function to dynamically check if auto-copy is enabled. | |||
| 165 | + | // This is called during script injection to determine whether to inject the selection listener. | |||
| 166 | + | func (ci *ContentInjector) SetAutoCopyConfigGetter(getter func() bool) { | |||
| 167 | + | ci.autoCopyConfigGetter = getter | |||
| 168 | + | } | |||
| 169 | + | ||||
| 136 | // InjectThemeCSS stores CSS variables for injection into internal pages. | 170 | // InjectThemeCSS stores CSS variables for injection into internal pages. | ||
| 137 | // Implements port.ContentInjector interface. | 171 | // Implements port.ContentInjector interface. | ||
| 138 | // The CSS will be injected when InjectScripts is called on WebView creation. | 172 | // The CSS will be injected when InjectScripts is called on WebView creation. | ||
| @@ -252,5 +286,21 @@ func (ci *ContentInjector) InjectScripts(ctx context.Context, ucm *webkit.UserCo | |||||
| 252 | } | 286 | } | ||
| 253 | } | 287 | } | ||
| 254 | 288 | ||||
| 255 | - | log.Debug().Bool("prefers_dark", prefersDark).Msg("scripts injected") | 289 | + | // 5. Inject auto-copy selection script for all pages (if enabled) |
| 290 | + | autoCopyEnabled := ci.autoCopyConfigGetter != nil && ci.autoCopyConfigGetter() | |||
| 291 | + | if autoCopyEnabled { | |||
| 292 | + | addScript( | |||
| 293 | + | webkit.NewUserScript( | |||
| 294 | + | autoCopySelectionScript, | |||
| 295 | + | webkit.UserContentInjectTopFrameValue, | |||
| 296 | + | webkit.UserScriptInjectAtDocumentEndValue, | |||
| 297 | + | nil, // All pages | |||
| 298 | + | nil, | |||
| 299 | + | ), | |||
| 300 | + | "auto-copy-selection", | |||
| 301 | + | ) | |||
| 302 | + | log.Debug().Msg("auto-copy selection script injected") | |||
| 303 | + | } | |||
| 304 | + | ||||
| 305 | + | log.Debug().Bool("prefers_dark", prefersDark).Bool("auto_copy", autoCopyEnabled).Msg("scripts injected") | |||
| 256 | } | 306 | } | ||
M internal/ui/app.go +18 -2
| @@ -146,8 +146,24 @@ func New(deps *Dependencies) (*App, error) { | |||
| 146 | 146 | // Register message handlers | |
| 147 | 147 | if app.router != nil { | |
| 148 | 148 | if err := handlers.RegisterAll(ctx, app.router, handlers.Config{ | |
| 149 | - | HistoryUC: deps.HistoryUC, | |
| 150 | - | FavoritesUC: deps.FavoritesUC, | |
| 149 | + | HistoryUC: deps.HistoryUC, | |
| 150 | + | FavoritesUC: deps.FavoritesUC, | |
| 151 | + | Clipboard: deps.Clipboard, | |
| 152 | + | ConfigGetter: config.Get, | |
| 153 | + | OnClipboardCopied: func(textLen int) { | |
| 154 | + | // Show brief toast on auto-copy (similar to zellij footer notification) | |
| 155 | + | // Must schedule on GTK main thread since this is called from WebKit handler | |
| 156 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 157 | + | if app.appToaster != nil { | |
| 158 | + | app.appToaster.Show(ctx, "Copied to clipboard", component.ToastInfo, | |
| 159 | + | component.WithDuration(component.ToastBriefDurationMs), | |
| 160 | + | component.WithPosition(component.ToastPositionBottomRight), | |
| 161 | + | ) | |
| 162 | + | } | |
| 163 | + | return false | |
| 164 | + | }) | |
| 165 | + | glib.IdleAdd(&cb, 0) | |
| 166 | + | }, | |
| 151 | 167 | }); err != nil { | |
| 152 | 168 | return nil, err | |
| 153 | 169 | } | |
| @@ -146,8 +146,24 @@ func New(deps *Dependencies) (*App, error) { | |||||
| 146 | // Register message handlers | 146 | // Register message handlers | ||
| 147 | if app.router != nil { | 147 | if app.router != nil { | ||
| 148 | if err := handlers.RegisterAll(ctx, app.router, handlers.Config{ | 148 | if err := handlers.RegisterAll(ctx, app.router, handlers.Config{ | ||
| 149 | - | HistoryUC: deps.HistoryUC, | 149 | + | HistoryUC: deps.HistoryUC, |
| 150 | - | FavoritesUC: deps.FavoritesUC, | 150 | + | FavoritesUC: deps.FavoritesUC, |
| 151 | + | Clipboard: deps.Clipboard, | |||
| 152 | + | ConfigGetter: config.Get, | |||
| 153 | + | OnClipboardCopied: func(textLen int) { | |||
| 154 | + | // Show brief toast on auto-copy (similar to zellij footer notification) | |||
| 155 | + | // Must schedule on GTK main thread since this is called from WebKit handler | |||
| 156 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |||
| 157 | + | if app.appToaster != nil { | |||
| 158 | + | app.appToaster.Show(ctx, "Copied to clipboard", component.ToastInfo, | |||
| 159 | + | component.WithDuration(component.ToastBriefDurationMs), | |||
| 160 | + | component.WithPosition(component.ToastPositionBottomRight), | |||
| 161 | + | ) | |||
| 162 | + | } | |||
| 163 | + | return false | |||
| 164 | + | }) | |||
| 165 | + | glib.IdleAdd(&cb, 0) | |||
| 166 | + | }, | |||
| 151 | }); err != nil { | 167 | }); err != nil { | ||
| 152 | return nil, err | 168 | return nil, err | ||
| 153 | } | 169 | } | ||
M internal/ui/component/link_status.go +40 -4
| @@ -13,6 +13,8 @@ import ( | |||
| 13 | 13 | const ( | |
| 14 | 14 | // linkStatusShowDelayMs is the delay before showing the overlay to avoid flicker. | |
| 15 | 15 | linkStatusShowDelayMs = 100 | |
| 16 | + | // linkStatusAutoHideMs is the delay before auto-hiding the overlay (e.g., during fullscreen video). | |
| 17 | + | linkStatusAutoHideMs = 10000 | |
| 16 | 18 | // linkStatusMaxChars is the maximum characters before truncation. | |
| 17 | 19 | linkStatusMaxChars = 80 | |
| 18 | 20 | ) | |
| @@ -24,10 +26,11 @@ type LinkStatusOverlay struct { | |||
| 24 | 26 | container layout.BoxWidget | |
| 25 | 27 | label layout.LabelWidget | |
| 26 | 28 | ||
| 27 | - | pendingURI string // URI waiting to be shown after delay | |
| 28 | - | showTimer uint // GLib timer for delayed show | |
| 29 | - | visible bool // Whether overlay is currently visible (has .visible class) | |
| 30 | - | mu sync.Mutex | |
| 29 | + | pendingURI string // URI waiting to be shown after delay | |
| 30 | + | showTimer uint // GLib timer for delayed show | |
| 31 | + | autoHideTimer uint // GLib timer for auto-hide after timeout | |
| 32 | + | visible bool // Whether overlay is currently visible (has .visible class) | |
| 33 | + | mu sync.Mutex | |
| 31 | 34 | } | |
| 32 | 35 | ||
| 33 | 36 | // NewLinkStatusOverlay creates a new link status overlay component. | |
| @@ -96,6 +99,8 @@ func (l *LinkStatusOverlay) Show(uri string) { | |||
| 96 | 99 | l.visible = true | |
| 97 | 100 | l.container.AddCssClass("visible") | |
| 98 | 101 | } | |
| 102 | + | // Start or reset auto-hide timer | |
| 103 | + | l.resetAutoHideTimerLocked() | |
| 99 | 104 | } | |
| 100 | 105 | l.showTimer = 0 | |
| 101 | 106 | return false // Don't repeat | |
| @@ -118,6 +123,11 @@ func (l *LinkStatusOverlay) hide() { | |||
| 118 | 123 | glib.SourceRemove(l.showTimer) | |
| 119 | 124 | l.showTimer = 0 | |
| 120 | 125 | } | |
| 126 | + | // Cancel auto-hide timer | |
| 127 | + | if l.autoHideTimer != 0 { | |
| 128 | + | glib.SourceRemove(l.autoHideTimer) | |
| 129 | + | l.autoHideTimer = 0 | |
| 130 | + | } | |
| 121 | 131 | l.pendingURI = "" | |
| 122 | 132 | ||
| 123 | 133 | if l.visible { | |
| @@ -126,6 +136,27 @@ func (l *LinkStatusOverlay) hide() { | |||
| 126 | 136 | } | |
| 127 | 137 | } | |
| 128 | 138 | ||
| 139 | + | // resetAutoHideTimerLocked starts or resets the auto-hide timer (must be called with lock held). | |
| 140 | + | func (l *LinkStatusOverlay) resetAutoHideTimerLocked() { | |
| 141 | + | // Cancel existing auto-hide timer | |
| 142 | + | if l.autoHideTimer != 0 { | |
| 143 | + | glib.SourceRemove(l.autoHideTimer) | |
| 144 | + | l.autoHideTimer = 0 | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | // Start new auto-hide timer | |
| 148 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 149 | + | l.mu.Lock() | |
| 150 | + | defer l.mu.Unlock() | |
| 151 | + | ||
| 152 | + | l.autoHideTimer = 0 | |
| 153 | + | l.hide() | |
| 154 | + | return false // Don't repeat | |
| 155 | + | }) | |
| 156 | + | ||
| 157 | + | l.autoHideTimer = glib.TimeoutAdd(linkStatusAutoHideMs, &cb, 0) | |
| 158 | + | } | |
| 159 | + | ||
| 129 | 160 | // Widget returns the underlying widget for embedding in overlays. | |
| 130 | 161 | func (l *LinkStatusOverlay) Widget() layout.Widget { | |
| 131 | 162 | return l.container | |
| @@ -149,6 +180,11 @@ func (l *LinkStatusOverlay) Cleanup() { | |||
| 149 | 180 | glib.SourceRemove(l.showTimer) | |
| 150 | 181 | l.showTimer = 0 | |
| 151 | 182 | } | |
| 183 | + | // Cancel auto-hide timer | |
| 184 | + | if l.autoHideTimer != 0 { | |
| 185 | + | glib.SourceRemove(l.autoHideTimer) | |
| 186 | + | l.autoHideTimer = 0 | |
| 187 | + | } | |
| 152 | 188 | l.pendingURI = "" | |
| 153 | 189 | l.visible = false | |
| 154 | 190 | } | |
| @@ -13,6 +13,8 @@ import ( | |||||
| 13 | const ( | 13 | const ( | ||
| 14 | // linkStatusShowDelayMs is the delay before showing the overlay to avoid flicker. | 14 | // linkStatusShowDelayMs is the delay before showing the overlay to avoid flicker. | ||
| 15 | linkStatusShowDelayMs = 100 | 15 | linkStatusShowDelayMs = 100 | ||
| 16 | + | // linkStatusAutoHideMs is the delay before auto-hiding the overlay (e.g., during fullscreen video). | |||
| 17 | + | linkStatusAutoHideMs = 10000 | |||
| 16 | // linkStatusMaxChars is the maximum characters before truncation. | 18 | // linkStatusMaxChars is the maximum characters before truncation. | ||
| 17 | linkStatusMaxChars = 80 | 19 | linkStatusMaxChars = 80 | ||
| 18 | ) | 20 | ) | ||
| @@ -24,10 +26,11 @@ type LinkStatusOverlay struct { | |||||
| 24 | container layout.BoxWidget | 26 | container layout.BoxWidget | ||
| 25 | label layout.LabelWidget | 27 | label layout.LabelWidget | ||
| 26 | 28 | ||||
| 27 | - | pendingURI string // URI waiting to be shown after delay | 29 | + | pendingURI string // URI waiting to be shown after delay |
| 28 | - | showTimer uint // GLib timer for delayed show | 30 | + | showTimer uint // GLib timer for delayed show |
| 29 | - | visible bool // Whether overlay is currently visible (has .visible class) | 31 | + | autoHideTimer uint // GLib timer for auto-hide after timeout |
| 30 | - | mu sync.Mutex | 32 | + | visible bool // Whether overlay is currently visible (has .visible class) |
| 33 | + | mu sync.Mutex | |||
| 31 | } | 34 | } | ||
| 32 | 35 | ||||
| 33 | // NewLinkStatusOverlay creates a new link status overlay component. | 36 | // NewLinkStatusOverlay creates a new link status overlay component. | ||
| @@ -96,6 +99,8 @@ func (l *LinkStatusOverlay) Show(uri string) { | |||||
| 96 | l.visible = true | 99 | l.visible = true | ||
| 97 | l.container.AddCssClass("visible") | 100 | l.container.AddCssClass("visible") | ||
| 98 | } | 101 | } | ||
| 102 | + | // Start or reset auto-hide timer | |||
| 103 | + | l.resetAutoHideTimerLocked() | |||
| 99 | } | 104 | } | ||
| 100 | l.showTimer = 0 | 105 | l.showTimer = 0 | ||
| 101 | return false // Don't repeat | 106 | return false // Don't repeat | ||
| @@ -118,6 +123,11 @@ func (l *LinkStatusOverlay) hide() { | |||||
| 118 | glib.SourceRemove(l.showTimer) | 123 | glib.SourceRemove(l.showTimer) | ||
| 119 | l.showTimer = 0 | 124 | l.showTimer = 0 | ||
| 120 | } | 125 | } | ||
| 126 | + | // Cancel auto-hide timer | |||
| 127 | + | if l.autoHideTimer != 0 { | |||
| 128 | + | glib.SourceRemove(l.autoHideTimer) | |||
| 129 | + | l.autoHideTimer = 0 | |||
| 130 | + | } | |||
| 121 | l.pendingURI = "" | 131 | l.pendingURI = "" | ||
| 122 | 132 | ||||
| 123 | if l.visible { | 133 | if l.visible { | ||
| @@ -126,6 +136,27 @@ func (l *LinkStatusOverlay) hide() { | |||||
| 126 | } | 136 | } | ||
| 127 | } | 137 | } | ||
| 128 | 138 | ||||
| 139 | + | // resetAutoHideTimerLocked starts or resets the auto-hide timer (must be called with lock held). | |||
| 140 | + | func (l *LinkStatusOverlay) resetAutoHideTimerLocked() { | |||
| 141 | + | // Cancel existing auto-hide timer | |||
| 142 | + | if l.autoHideTimer != 0 { | |||
| 143 | + | glib.SourceRemove(l.autoHideTimer) | |||
| 144 | + | l.autoHideTimer = 0 | |||
| 145 | + | } | |||
| 146 | + | ||||
| 147 | + | // Start new auto-hide timer | |||
| 148 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |||
| 149 | + | l.mu.Lock() | |||
| 150 | + | defer l.mu.Unlock() | |||
| 151 | + | ||||
| 152 | + | l.autoHideTimer = 0 | |||
| 153 | + | l.hide() | |||
| 154 | + | return false // Don't repeat | |||
| 155 | + | }) | |||
| 156 | + | ||||
| 157 | + | l.autoHideTimer = glib.TimeoutAdd(linkStatusAutoHideMs, &cb, 0) | |||
| 158 | + | } | |||
| 159 | + | ||||
| 129 | // Widget returns the underlying widget for embedding in overlays. | 160 | // Widget returns the underlying widget for embedding in overlays. | ||
| 130 | func (l *LinkStatusOverlay) Widget() layout.Widget { | 161 | func (l *LinkStatusOverlay) Widget() layout.Widget { | ||
| 131 | return l.container | 162 | return l.container | ||
| @@ -149,6 +180,11 @@ func (l *LinkStatusOverlay) Cleanup() { | |||||
| 149 | glib.SourceRemove(l.showTimer) | 180 | glib.SourceRemove(l.showTimer) | ||
| 150 | l.showTimer = 0 | 181 | l.showTimer = 0 | ||
| 151 | } | 182 | } | ||
| 183 | + | // Cancel auto-hide timer | |||
| 184 | + | if l.autoHideTimer != 0 { | |||
| 185 | + | glib.SourceRemove(l.autoHideTimer) | |||
| 186 | + | l.autoHideTimer = 0 | |||
| 187 | + | } | |||
| 152 | l.pendingURI = "" | 188 | l.pendingURI = "" | ||
| 153 | l.visible = false | 189 | l.visible = false | ||
| 154 | } | 190 | } | ||
M internal/ui/component/toaster.go +2 -0
| @@ -16,6 +16,8 @@ import ( | |||
| 16 | 16 | const ( | |
| 17 | 17 | // Default auto-dismiss timeout in milliseconds. | |
| 18 | 18 | toastDismissTimeoutMs = 1500 | |
| 19 | + | // ToastBriefDurationMs is the duration for brief notifications (e.g., clipboard copy). | |
| 20 | + | ToastBriefDurationMs = 800 | |
| 19 | 21 | ) | |
| 20 | 22 | ||
| 21 | 23 | // ToastLevel indicates the visual style of a toast notification. | |
| @@ -16,6 +16,8 @@ import ( | |||||
| 16 | const ( | 16 | const ( | ||
| 17 | // Default auto-dismiss timeout in milliseconds. | 17 | // Default auto-dismiss timeout in milliseconds. | ||
| 18 | toastDismissTimeoutMs = 1500 | 18 | toastDismissTimeoutMs = 1500 | ||
| 19 | + | // ToastBriefDurationMs is the duration for brief notifications (e.g., clipboard copy). | |||
| 20 | + | ToastBriefDurationMs = 800 | |||
| 19 | ) | 21 | ) | ||
| 20 | 22 | ||||
| 21 | // ToastLevel indicates the visual style of a toast notification. | 23 | // ToastLevel indicates the visual style of a toast notification. | ||
M internal/ui/layout/stacked.go +80 -23
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "sync" | |
| 7 | 7 | ||
| 8 | 8 | "github.com/bnema/dumber/internal/logging" | |
| 9 | + | "github.com/jwijenbergh/puregotk/v4/gobject" | |
| 9 | 10 | ) | |
| 10 | 11 | ||
| 11 | 12 | // ErrStackEmpty is returned when operating on an empty stack. | |
| @@ -31,6 +32,12 @@ type stackedPane struct { | |||
| 31 | 32 | favicon ImageWidget | |
| 32 | 33 | label LabelWidget | |
| 33 | 34 | isActive bool | |
| 35 | + | ||
| 36 | + | // Signal handler IDs for cleanup on removal | |
| 37 | + | titleClickSignalID uint32 | |
| 38 | + | closeClickSignalID uint32 | |
| 39 | + | titleButton ButtonWidget // stored for signal disconnection | |
| 40 | + | closeButton ButtonWidget // stored for signal disconnection | |
| 34 | 41 | } | |
| 35 | 42 | ||
| 36 | 43 | // StackedView manages a stack of panes where only one is visible at a time. | |
| @@ -137,22 +144,26 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||
| 137 | 144 | // Create title bar components | |
| 138 | 145 | tb := sv.createTitleBar(title, faviconIconName) | |
| 139 | 146 | ||
| 147 | + | // Connect click handlers using paneID (not index, to handle removals) | |
| 148 | + | titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID) | |
| 149 | + | ||
| 140 | 150 | pane := &stackedPane{ | |
| 141 | - | paneID: paneID, | |
| 142 | - | titleBar: tb.titleBar, | |
| 143 | - | container: container, | |
| 144 | - | title: title, | |
| 145 | - | favicon: tb.favicon, | |
| 146 | - | label: tb.label, | |
| 147 | - | isActive: false, | |
| 151 | + | paneID: paneID, | |
| 152 | + | titleBar: tb.titleBar, | |
| 153 | + | container: container, | |
| 154 | + | title: title, | |
| 155 | + | favicon: tb.favicon, | |
| 156 | + | label: tb.label, | |
| 157 | + | isActive: false, | |
| 158 | + | titleClickSignalID: titleSignalID, | |
| 159 | + | closeClickSignalID: closeSignalID, | |
| 160 | + | titleButton: tb.titleButton, | |
| 161 | + | closeButton: tb.closeBtn, | |
| 148 | 162 | } | |
| 149 | 163 | ||
| 150 | 164 | index := len(sv.panes) | |
| 151 | 165 | sv.panes = append(sv.panes, pane) | |
| 152 | 166 | ||
| 153 | - | // Connect click handlers using paneID (not index, to handle removals) | |
| 154 | - | sv.connectTitleBarHandlers(tb, paneID) | |
| 155 | - | ||
| 156 | 167 | // Add to container | |
| 157 | 168 | log.Debug(). | |
| 158 | 169 | Int("index", index). | |
| @@ -175,9 +186,10 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||
| 175 | 186 | ||
| 176 | 187 | // connectTitleBarHandlers connects the click handlers for a title bar. | |
| 177 | 188 | // Uses paneID lookup instead of captured index to handle pane removals correctly. | |
| 178 | - | func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) { | |
| 189 | + | // Returns the signal IDs for later disconnection. | |
| 190 | + | func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) (titleSignalID, closeSignalID uint32) { | |
| 179 | 191 | // Connect title bar click handler for activation | |
| 180 | - | tb.titleButton.ConnectClicked(func() { | |
| 192 | + | titleSignalID = tb.titleButton.ConnectClicked(func() { | |
| 181 | 193 | sv.mu.RLock() | |
| 182 | 194 | callback := sv.onActivate | |
| 183 | 195 | currentIndex := sv.findPaneIndexInternal(paneID) | |
| @@ -189,7 +201,7 @@ func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID str | |||
| 189 | 201 | }) | |
| 190 | 202 | ||
| 191 | 203 | // Connect close button click handler | |
| 192 | - | tb.closeBtn.ConnectClicked(func() { | |
| 204 | + | closeSignalID = tb.closeBtn.ConnectClicked(func() { | |
| 193 | 205 | sv.mu.RLock() | |
| 194 | 206 | onClose := sv.onClosePane | |
| 195 | 207 | sv.mu.RUnlock() | |
| @@ -198,6 +210,44 @@ func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID str | |||
| 198 | 210 | onClose(paneID) | |
| 199 | 211 | } | |
| 200 | 212 | }) | |
| 213 | + | ||
| 214 | + | return titleSignalID, closeSignalID | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | // disconnectPaneSignals disconnects signal handlers from a pane's buttons. | |
| 218 | + | // This prevents memory leaks when panes are removed from the stack. | |
| 219 | + | // Note: This is a no-op when using mock widgets in tests (GtkWidget returns nil). | |
| 220 | + | func (sv *StackedView) disconnectPaneSignals(pane *stackedPane) { | |
| 221 | + | if pane == nil { | |
| 222 | + | return | |
| 223 | + | } | |
| 224 | + | ||
| 225 | + | // Disconnect title button click signal | |
| 226 | + | disconnectButtonSignal(pane.titleButton, pane.titleClickSignalID) | |
| 227 | + | ||
| 228 | + | // Disconnect close button click signal | |
| 229 | + | disconnectButtonSignal(pane.closeButton, pane.closeClickSignalID) | |
| 230 | + | } | |
| 231 | + | ||
| 232 | + | // disconnectButtonSignal safely disconnects a signal from a button widget. | |
| 233 | + | // Returns silently if button is nil, signal ID is 0, or widget doesn't support GTK operations. | |
| 234 | + | func disconnectButtonSignal(btn ButtonWidget, signalID uint32) { | |
| 235 | + | if btn == nil || signalID == 0 { | |
| 236 | + | return | |
| 237 | + | } | |
| 238 | + | ||
| 239 | + | gtkWidget := btn.GtkWidget() | |
| 240 | + | if gtkWidget == nil { | |
| 241 | + | return | |
| 242 | + | } | |
| 243 | + | ||
| 244 | + | ptr := gtkWidget.GoPointer() | |
| 245 | + | if ptr == 0 { | |
| 246 | + | return | |
| 247 | + | } | |
| 248 | + | ||
| 249 | + | obj := gobject.ObjectNewFromInternalPtr(ptr) | |
| 250 | + | gobject.SignalHandlerDisconnect(obj, signalID) | |
| 201 | 251 | } | |
| 202 | 252 | ||
| 203 | 253 | // InsertPaneAfter inserts a new pane after the specified index position. | |
| @@ -230,14 +280,21 @@ func (sv *StackedView) InsertPaneAfter( | |||
| 230 | 280 | // Create title bar components | |
| 231 | 281 | tb := sv.createTitleBar(title, faviconIconName) | |
| 232 | 282 | ||
| 283 | + | // Connect click handlers using paneID (not index, to handle removals) | |
| 284 | + | titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID) | |
| 285 | + | ||
| 233 | 286 | pane := &stackedPane{ | |
| 234 | - | paneID: paneID, | |
| 235 | - | titleBar: tb.titleBar, | |
| 236 | - | container: container, | |
| 237 | - | title: title, | |
| 238 | - | favicon: tb.favicon, | |
| 239 | - | label: tb.label, | |
| 240 | - | isActive: false, | |
| 287 | + | paneID: paneID, | |
| 288 | + | titleBar: tb.titleBar, | |
| 289 | + | container: container, | |
| 290 | + | title: title, | |
| 291 | + | favicon: tb.favicon, | |
| 292 | + | label: tb.label, | |
| 293 | + | isActive: false, | |
| 294 | + | titleClickSignalID: titleSignalID, | |
| 295 | + | closeClickSignalID: closeSignalID, | |
| 296 | + | titleButton: tb.titleButton, | |
| 297 | + | closeButton: tb.closeBtn, | |
| 241 | 298 | } | |
| 242 | 299 | ||
| 243 | 300 | // Insert into slice at correct position | |
| @@ -245,9 +302,6 @@ func (sv *StackedView) InsertPaneAfter( | |||
| 245 | 302 | copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:]) | |
| 246 | 303 | sv.panes[insertIndex] = pane | |
| 247 | 304 | ||
| 248 | - | // Connect click handlers using paneID (not index, to handle removals) | |
| 249 | - | sv.connectTitleBarHandlers(tb, paneID) | |
| 250 | - | ||
| 251 | 305 | // Insert widgets at correct position in GTK box | |
| 252 | 306 | sv.insertTitleBarWidgets(tb.titleButton, container, insertIndex) | |
| 253 | 307 | ||
| @@ -321,6 +375,9 @@ func (sv *StackedView) RemovePane(ctx context.Context, index int) error { | |||
| 321 | 375 | ||
| 322 | 376 | pane := sv.panes[index] | |
| 323 | 377 | ||
| 378 | + | // Disconnect signal handlers before removing widgets to prevent memory leaks | |
| 379 | + | sv.disconnectPaneSignals(pane) | |
| 380 | + | ||
| 324 | 381 | // Remove from container | |
| 325 | 382 | if pane.titleBar != nil { | |
| 326 | 383 | // The title bar is wrapped in a button, need to get parent | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "sync" | 6 | "sync" | ||
| 7 | 7 | ||||
| 8 | "github.com/bnema/dumber/internal/logging" | 8 | "github.com/bnema/dumber/internal/logging" | ||
| 9 | + | "github.com/jwijenbergh/puregotk/v4/gobject" | |||
| 9 | ) | 10 | ) | ||
| 10 | 11 | ||||
| 11 | // ErrStackEmpty is returned when operating on an empty stack. | 12 | // ErrStackEmpty is returned when operating on an empty stack. | ||
| @@ -31,6 +32,12 @@ type stackedPane struct { | |||||
| 31 | favicon ImageWidget | 32 | favicon ImageWidget | ||
| 32 | label LabelWidget | 33 | label LabelWidget | ||
| 33 | isActive bool | 34 | isActive bool | ||
| 35 | + | ||||
| 36 | + | // Signal handler IDs for cleanup on removal | |||
| 37 | + | titleClickSignalID uint32 | |||
| 38 | + | closeClickSignalID uint32 | |||
| 39 | + | titleButton ButtonWidget // stored for signal disconnection | |||
| 40 | + | closeButton ButtonWidget // stored for signal disconnection | |||
| 34 | } | 41 | } | ||
| 35 | 42 | ||||
| 36 | // StackedView manages a stack of panes where only one is visible at a time. | 43 | // StackedView manages a stack of panes where only one is visible at a time. | ||
| @@ -137,22 +144,26 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||||
| 137 | // Create title bar components | 144 | // Create title bar components | ||
| 138 | tb := sv.createTitleBar(title, faviconIconName) | 145 | tb := sv.createTitleBar(title, faviconIconName) | ||
| 139 | 146 | ||||
| 147 | + | // Connect click handlers using paneID (not index, to handle removals) | |||
| 148 | + | titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID) | |||
| 149 | + | ||||
| 140 | pane := &stackedPane{ | 150 | pane := &stackedPane{ | ||
| 141 | - | paneID: paneID, | 151 | + | paneID: paneID, |
| 142 | - | titleBar: tb.titleBar, | 152 | + | titleBar: tb.titleBar, |
| 143 | - | container: container, | 153 | + | container: container, |
| 144 | - | title: title, | 154 | + | title: title, |
| 145 | - | favicon: tb.favicon, | 155 | + | favicon: tb.favicon, |
| 146 | - | label: tb.label, | 156 | + | label: tb.label, |
| 147 | - | isActive: false, | 157 | + | isActive: false, |
| 158 | + | titleClickSignalID: titleSignalID, | |||
| 159 | + | closeClickSignalID: closeSignalID, | |||
| 160 | + | titleButton: tb.titleButton, | |||
| 161 | + | closeButton: tb.closeBtn, | |||
| 148 | } | 162 | } | ||
| 149 | 163 | ||||
| 150 | index := len(sv.panes) | 164 | index := len(sv.panes) | ||
| 151 | sv.panes = append(sv.panes, pane) | 165 | sv.panes = append(sv.panes, pane) | ||
| 152 | 166 | ||||
| 153 | - | // Connect click handlers using paneID (not index, to handle removals) | |||
| 154 | - | sv.connectTitleBarHandlers(tb, paneID) | |||
| 155 | - | ||||
| 156 | // Add to container | 167 | // Add to container | ||
| 157 | log.Debug(). | 168 | log.Debug(). | ||
| 158 | Int("index", index). | 169 | Int("index", index). | ||
| @@ -175,9 +186,10 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||||
| 175 | 186 | ||||
| 176 | // connectTitleBarHandlers connects the click handlers for a title bar. | 187 | // connectTitleBarHandlers connects the click handlers for a title bar. | ||
| 177 | // Uses paneID lookup instead of captured index to handle pane removals correctly. | 188 | // Uses paneID lookup instead of captured index to handle pane removals correctly. | ||
| 178 | - | func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) { | 189 | + | // Returns the signal IDs for later disconnection. |
| 190 | + | func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) (titleSignalID, closeSignalID uint32) { | |||
| 179 | // Connect title bar click handler for activation | 191 | // Connect title bar click handler for activation | ||
| 180 | - | tb.titleButton.ConnectClicked(func() { | 192 | + | titleSignalID = tb.titleButton.ConnectClicked(func() { |
| 181 | sv.mu.RLock() | 193 | sv.mu.RLock() | ||
| 182 | callback := sv.onActivate | 194 | callback := sv.onActivate | ||
| 183 | currentIndex := sv.findPaneIndexInternal(paneID) | 195 | currentIndex := sv.findPaneIndexInternal(paneID) | ||
| @@ -189,7 +201,7 @@ func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID str | |||||
| 189 | }) | 201 | }) | ||
| 190 | 202 | ||||
| 191 | // Connect close button click handler | 203 | // Connect close button click handler | ||
| 192 | - | tb.closeBtn.ConnectClicked(func() { | 204 | + | closeSignalID = tb.closeBtn.ConnectClicked(func() { |
| 193 | sv.mu.RLock() | 205 | sv.mu.RLock() | ||
| 194 | onClose := sv.onClosePane | 206 | onClose := sv.onClosePane | ||
| 195 | sv.mu.RUnlock() | 207 | sv.mu.RUnlock() | ||
| @@ -198,6 +210,44 @@ func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID str | |||||
| 198 | onClose(paneID) | 210 | onClose(paneID) | ||
| 199 | } | 211 | } | ||
| 200 | }) | 212 | }) | ||
| 213 | + | ||||
| 214 | + | return titleSignalID, closeSignalID | |||
| 215 | + | } | |||
| 216 | + | ||||
| 217 | + | // disconnectPaneSignals disconnects signal handlers from a pane's buttons. | |||
| 218 | + | // This prevents memory leaks when panes are removed from the stack. | |||
| 219 | + | // Note: This is a no-op when using mock widgets in tests (GtkWidget returns nil). | |||
| 220 | + | func (sv *StackedView) disconnectPaneSignals(pane *stackedPane) { | |||
| 221 | + | if pane == nil { | |||
| 222 | + | return | |||
| 223 | + | } | |||
| 224 | + | ||||
| 225 | + | // Disconnect title button click signal | |||
| 226 | + | disconnectButtonSignal(pane.titleButton, pane.titleClickSignalID) | |||
| 227 | + | ||||
| 228 | + | // Disconnect close button click signal | |||
| 229 | + | disconnectButtonSignal(pane.closeButton, pane.closeClickSignalID) | |||
| 230 | + | } | |||
| 231 | + | ||||
| 232 | + | // disconnectButtonSignal safely disconnects a signal from a button widget. | |||
| 233 | + | // Returns silently if button is nil, signal ID is 0, or widget doesn't support GTK operations. | |||
| 234 | + | func disconnectButtonSignal(btn ButtonWidget, signalID uint32) { | |||
| 235 | + | if btn == nil || signalID == 0 { | |||
| 236 | + | return | |||
| 237 | + | } | |||
| 238 | + | ||||
| 239 | + | gtkWidget := btn.GtkWidget() | |||
| 240 | + | if gtkWidget == nil { | |||
| 241 | + | return | |||
| 242 | + | } | |||
| 243 | + | ||||
| 244 | + | ptr := gtkWidget.GoPointer() | |||
| 245 | + | if ptr == 0 { | |||
| 246 | + | return | |||
| 247 | + | } | |||
| 248 | + | ||||
| 249 | + | obj := gobject.ObjectNewFromInternalPtr(ptr) | |||
| 250 | + | gobject.SignalHandlerDisconnect(obj, signalID) | |||
| 201 | } | 251 | } | ||
| 202 | 252 | ||||
| 203 | // InsertPaneAfter inserts a new pane after the specified index position. | 253 | // InsertPaneAfter inserts a new pane after the specified index position. | ||
| @@ -230,14 +280,21 @@ func (sv *StackedView) InsertPaneAfter( | |||||
| 230 | // Create title bar components | 280 | // Create title bar components | ||
| 231 | tb := sv.createTitleBar(title, faviconIconName) | 281 | tb := sv.createTitleBar(title, faviconIconName) | ||
| 232 | 282 | ||||
| 283 | + | // Connect click handlers using paneID (not index, to handle removals) | |||
| 284 | + | titleSignalID, closeSignalID := sv.connectTitleBarHandlers(tb, paneID) | |||
| 285 | + | ||||
| 233 | pane := &stackedPane{ | 286 | pane := &stackedPane{ | ||
| 234 | - | paneID: paneID, | 287 | + | paneID: paneID, |
| 235 | - | titleBar: tb.titleBar, | 288 | + | titleBar: tb.titleBar, |
| 236 | - | container: container, | 289 | + | container: container, |
| 237 | - | title: title, | 290 | + | title: title, |
| 238 | - | favicon: tb.favicon, | 291 | + | favicon: tb.favicon, |
| 239 | - | label: tb.label, | 292 | + | label: tb.label, |
| 240 | - | isActive: false, | 293 | + | isActive: false, |
| 294 | + | titleClickSignalID: titleSignalID, | |||
| 295 | + | closeClickSignalID: closeSignalID, | |||
| 296 | + | titleButton: tb.titleButton, | |||
| 297 | + | closeButton: tb.closeBtn, | |||
| 241 | } | 298 | } | ||
| 242 | 299 | ||||
| 243 | // Insert into slice at correct position | 300 | // Insert into slice at correct position | ||
| @@ -245,9 +302,6 @@ func (sv *StackedView) InsertPaneAfter( | |||||
| 245 | copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:]) | 302 | copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:]) | ||
| 246 | sv.panes[insertIndex] = pane | 303 | sv.panes[insertIndex] = pane | ||
| 247 | 304 | ||||
| 248 | - | // Connect click handlers using paneID (not index, to handle removals) | |||
| 249 | - | sv.connectTitleBarHandlers(tb, paneID) | |||
| 250 | - | ||||
| 251 | // Insert widgets at correct position in GTK box | 305 | // Insert widgets at correct position in GTK box | ||
| 252 | sv.insertTitleBarWidgets(tb.titleButton, container, insertIndex) | 306 | sv.insertTitleBarWidgets(tb.titleButton, container, insertIndex) | ||
| 253 | 307 | ||||
| @@ -321,6 +375,9 @@ func (sv *StackedView) RemovePane(ctx context.Context, index int) error { | |||||
| 321 | 375 | ||||
| 322 | pane := sv.panes[index] | 376 | pane := sv.panes[index] | ||
| 323 | 377 | ||||
| 378 | + | // Disconnect signal handlers before removing widgets to prevent memory leaks | |||
| 379 | + | sv.disconnectPaneSignals(pane) | |||
| 380 | + | ||||
| 324 | // Remove from container | 381 | // Remove from container | ||
| 325 | if pane.titleBar != nil { | 382 | if pane.titleBar != nil { | ||
| 326 | // The title bar is wrapped in a button, need to get parent | 383 | // The title bar is wrapped in a button, need to get parent | ||
M internal/ui/layout/stacked_test.go +4 -0
| @@ -75,6 +75,10 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||
| 75 | 75 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | |
| 76 | 76 | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | |
| 77 | 77 | ||
| 78 | + | // Signal disconnection calls GtkWidget() - return nil to skip actual GTK operations in tests | |
| 79 | + | mockButton.EXPECT().GtkWidget().Return(nil).Maybe() | |
| 80 | + | mockCloseButton.EXPECT().GtkWidget().Return(nil).Maybe() | |
| 81 | + | ||
| 78 | 82 | // Adding to main box | |
| 79 | 83 | mockBox.EXPECT().Append(mockButton).Once() | |
| 80 | 84 | mockBox.EXPECT().Append(mockContainer).Once() | |
| @@ -75,6 +75,10 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||||
| 75 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | 75 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | ||
| 76 | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | 76 | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | ||
| 77 | 77 | ||||
| 78 | + | // Signal disconnection calls GtkWidget() - return nil to skip actual GTK operations in tests | |||
| 79 | + | mockButton.EXPECT().GtkWidget().Return(nil).Maybe() | |||
| 80 | + | mockCloseButton.EXPECT().GtkWidget().Return(nil).Maybe() | |||
| 81 | + | ||||
| 78 | // Adding to main box | 82 | // Adding to main box | ||
| 79 | mockBox.EXPECT().Append(mockButton).Once() | 83 | mockBox.EXPECT().Append(mockButton).Once() | ||
| 80 | mockBox.EXPECT().Append(mockContainer).Once() | 84 | mockBox.EXPECT().Append(mockContainer).Once() | ||