refactor(omnibox): clean up layer-shell follow-ups
Make layer-shell library lookup explicit in tests and remove a few small cleanup issues from the standalone omnibox path.
- commit
- 861c3720029e53a70b07fb5a61dbf702ce025746
- author
- brice · · 2026-04-07 17:33 UTC
- parent
- 5017596
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +19 -28
M internal/bootstrap/layershell_preload.go +3 -3
| @@ -40,8 +40,8 @@ func CurrentEnvMap() map[string]string { | |||
| 40 | 40 | return env | |
| 41 | 41 | } | |
| 42 | 42 | ||
| 43 | - | func FindLayerShellLibrary(exists func(path string) bool) string { | |
| 44 | - | for _, path := range layerShellLibrarySearchPaths(CurrentEnvMap()) { | |
| 43 | + | func FindLayerShellLibrary(env map[string]string, exists func(path string) bool) string { | |
| 44 | + | for _, path := range layerShellLibrarySearchPaths(env) { | |
| 45 | 45 | if exists(path) { | |
| 46 | 46 | return path | |
| 47 | 47 | } | |
| @@ -51,7 +51,7 @@ func FindLayerShellLibrary(exists func(path string) bool) string { | |||
| 51 | 51 | } | |
| 52 | 52 | ||
| 53 | 53 | func LayerShellLibraryPath() string { | |
| 54 | - | return FindLayerShellLibrary(func(path string) bool { | |
| 54 | + | return FindLayerShellLibrary(CurrentEnvMap(), func(path string) bool { | |
| 55 | 55 | info, err := os.Stat(filepath.Clean(path)) | |
| 56 | 56 | return err == nil && !info.IsDir() | |
| 57 | 57 | }) | |
| @@ -40,8 +40,8 @@ func CurrentEnvMap() map[string]string { | |||||
| 40 | return env | 40 | return env | ||
| 41 | } | 41 | } | ||
| 42 | 42 | ||||
| 43 | - | func FindLayerShellLibrary(exists func(path string) bool) string { | 43 | + | func FindLayerShellLibrary(env map[string]string, exists func(path string) bool) string { |
| 44 | - | for _, path := range layerShellLibrarySearchPaths(CurrentEnvMap()) { | 44 | + | for _, path := range layerShellLibrarySearchPaths(env) { |
| 45 | if exists(path) { | 45 | if exists(path) { | ||
| 46 | return path | 46 | return path | ||
| 47 | } | 47 | } | ||
| @@ -51,7 +51,7 @@ func FindLayerShellLibrary(exists func(path string) bool) string { | |||||
| 51 | } | 51 | } | ||
| 52 | 52 | ||||
| 53 | func LayerShellLibraryPath() string { | 53 | func LayerShellLibraryPath() string { | ||
| 54 | - | return FindLayerShellLibrary(func(path string) bool { | 54 | + | return FindLayerShellLibrary(CurrentEnvMap(), func(path string) bool { |
| 55 | info, err := os.Stat(filepath.Clean(path)) | 55 | info, err := os.Stat(filepath.Clean(path)) | ||
| 56 | return err == nil && !info.IsDir() | 56 | return err == nil && !info.IsDir() | ||
| 57 | }) | 57 | }) | ||
M internal/bootstrap/layershell_preload_test.go +12 -10
| @@ -13,14 +13,16 @@ func TestShouldPreloadLayerShell_WaylandWithoutPreloadReturnsTrue(t *testing.T) | |||
| 13 | 13 | } | |
| 14 | 14 | } | |
| 15 | 15 | ||
| 16 | - | func TestShouldPreloadLayerShell_SkipsWithoutLibraryPath(t *testing.T) { | |
| 17 | - | env := map[string]string{ | |
| 18 | - | "WAYLAND_DISPLAY": "wayland-1", | |
| 19 | - | "LD_PRELOAD": "", | |
| 20 | - | } | |
| 16 | + | func TestFindLayerShellLibrary_UsesLDLibraryPathCandidates(t *testing.T) { | |
| 17 | + | customCandidate := "/opt/custom/lib/libgtk4-layer-shell.so.0" | |
| 18 | + | got := FindLayerShellLibrary(map[string]string{ | |
| 19 | + | "LD_LIBRARY_PATH": "/opt/custom/lib", | |
| 20 | + | }, func(path string) bool { | |
| 21 | + | return path == customCandidate | |
| 22 | + | }) | |
| 21 | 23 | ||
| 22 | - | if !ShouldPreloadLayerShell(env) { | |
| 23 | - | t.Fatalf("expected preload to be required before library probing") | |
| 24 | + | if got != customCandidate { | |
| 25 | + | t.Fatalf("expected %q, got %q", customCandidate, got) | |
| 24 | 26 | } | |
| 25 | 27 | } | |
| 26 | 28 | ||
| @@ -52,7 +54,7 @@ func TestLayerShellPreloadPresent_DetectsLoadedLibrarySubstring(t *testing.T) { | |||
| 52 | 54 | } | |
| 53 | 55 | ||
| 54 | 56 | func TestFindLayerShellLibrary_ReturnsFirstExistingCandidate(t *testing.T) { | |
| 55 | - | got := FindLayerShellLibrary(func(path string) bool { | |
| 57 | + | got := FindLayerShellLibrary(map[string]string{}, func(path string) bool { | |
| 56 | 58 | return path == layerShellLibraryCandidates[1] | |
| 57 | 59 | }) | |
| 58 | 60 | ||
| @@ -63,7 +65,7 @@ func TestFindLayerShellLibrary_ReturnsFirstExistingCandidate(t *testing.T) { | |||
| 63 | 65 | ||
| 64 | 66 | func TestFindLayerShellLibrary_ReturnsArm64Candidate(t *testing.T) { | |
| 65 | 67 | arm64Candidate := "/usr/lib/aarch64-linux-gnu/libgtk4-layer-shell.so.0" | |
| 66 | - | got := FindLayerShellLibrary(func(path string) bool { | |
| 68 | + | got := FindLayerShellLibrary(map[string]string{}, func(path string) bool { | |
| 67 | 69 | return path == arm64Candidate | |
| 68 | 70 | }) | |
| 69 | 71 | ||
| @@ -73,7 +75,7 @@ func TestFindLayerShellLibrary_ReturnsArm64Candidate(t *testing.T) { | |||
| 73 | 75 | } | |
| 74 | 76 | ||
| 75 | 77 | func TestFindLayerShellLibrary_NoMatchReturnsEmpty(t *testing.T) { | |
| 76 | - | if got := FindLayerShellLibrary(func(string) bool { return false }); got != "" { | |
| 78 | + | if got := FindLayerShellLibrary(map[string]string{}, func(string) bool { return false }); got != "" { | |
| 77 | 79 | t.Fatalf("expected empty result, got %q", got) | |
| 78 | 80 | } | |
| 79 | 81 | } | |
| @@ -13,14 +13,16 @@ func TestShouldPreloadLayerShell_WaylandWithoutPreloadReturnsTrue(t *testing.T) | |||||
| 13 | } | 13 | } | ||
| 14 | } | 14 | } | ||
| 15 | 15 | ||||
| 16 | - | func TestShouldPreloadLayerShell_SkipsWithoutLibraryPath(t *testing.T) { | 16 | + | func TestFindLayerShellLibrary_UsesLDLibraryPathCandidates(t *testing.T) { |
| 17 | - | env := map[string]string{ | 17 | + | customCandidate := "/opt/custom/lib/libgtk4-layer-shell.so.0" |
| 18 | - | "WAYLAND_DISPLAY": "wayland-1", | 18 | + | got := FindLayerShellLibrary(map[string]string{ |
| 19 | - | "LD_PRELOAD": "", | 19 | + | "LD_LIBRARY_PATH": "/opt/custom/lib", |
| 20 | - | } | 20 | + | }, func(path string) bool { |
| 21 | + | return path == customCandidate | |||
| 22 | + | }) | |||
| 21 | 23 | ||||
| 22 | - | if !ShouldPreloadLayerShell(env) { | 24 | + | if got != customCandidate { |
| 23 | - | t.Fatalf("expected preload to be required before library probing") | 25 | + | t.Fatalf("expected %q, got %q", customCandidate, got) |
| 24 | } | 26 | } | ||
| 25 | } | 27 | } | ||
| 26 | 28 | ||||
| @@ -52,7 +54,7 @@ func TestLayerShellPreloadPresent_DetectsLoadedLibrarySubstring(t *testing.T) { | |||||
| 52 | } | 54 | } | ||
| 53 | 55 | ||||
| 54 | func TestFindLayerShellLibrary_ReturnsFirstExistingCandidate(t *testing.T) { | 56 | func TestFindLayerShellLibrary_ReturnsFirstExistingCandidate(t *testing.T) { | ||
| 55 | - | got := FindLayerShellLibrary(func(path string) bool { | 57 | + | got := FindLayerShellLibrary(map[string]string{}, func(path string) bool { |
| 56 | return path == layerShellLibraryCandidates[1] | 58 | return path == layerShellLibraryCandidates[1] | ||
| 57 | }) | 59 | }) | ||
| 58 | 60 | ||||
| @@ -63,7 +65,7 @@ func TestFindLayerShellLibrary_ReturnsFirstExistingCandidate(t *testing.T) { | |||||
| 63 | 65 | ||||
| 64 | func TestFindLayerShellLibrary_ReturnsArm64Candidate(t *testing.T) { | 66 | func TestFindLayerShellLibrary_ReturnsArm64Candidate(t *testing.T) { | ||
| 65 | arm64Candidate := "/usr/lib/aarch64-linux-gnu/libgtk4-layer-shell.so.0" | 67 | arm64Candidate := "/usr/lib/aarch64-linux-gnu/libgtk4-layer-shell.so.0" | ||
| 66 | - | got := FindLayerShellLibrary(func(path string) bool { | 68 | + | got := FindLayerShellLibrary(map[string]string{}, func(path string) bool { |
| 67 | return path == arm64Candidate | 69 | return path == arm64Candidate | ||
| 68 | }) | 70 | }) | ||
| 69 | 71 | ||||
| @@ -73,7 +75,7 @@ func TestFindLayerShellLibrary_ReturnsArm64Candidate(t *testing.T) { | |||||
| 73 | } | 75 | } | ||
| 74 | 76 | ||||
| 75 | func TestFindLayerShellLibrary_NoMatchReturnsEmpty(t *testing.T) { | 77 | func TestFindLayerShellLibrary_NoMatchReturnsEmpty(t *testing.T) { | ||
| 76 | - | if got := FindLayerShellLibrary(func(string) bool { return false }); got != "" { | 78 | + | if got := FindLayerShellLibrary(map[string]string{}, func(string) bool { return false }); got != "" { |
| 77 | t.Fatalf("expected empty result, got %q", got) | 79 | t.Fatalf("expected empty result, got %q", got) | ||
| 78 | } | 80 | } | ||
| 79 | } | 81 | } | ||
M internal/ui/component/omnibox.go +3 -2
| @@ -13,6 +13,7 @@ import ( | |||
| 13 | 13 | "github.com/bnema/puregotk/v4/glib" | |
| 14 | 14 | "github.com/bnema/puregotk/v4/graphene" | |
| 15 | 15 | "github.com/bnema/puregotk/v4/gtk" | |
| 16 | + | "github.com/bnema/puregotk/v4/pango" | |
| 16 | 17 | ||
| 17 | 18 | "github.com/bnema/dumber/internal/application/usecase" | |
| 18 | 19 | "github.com/bnema/dumber/internal/domain/autocomplete" | |
| @@ -1799,7 +1800,7 @@ func (o *Omnibox) appendSuggestionTitleAndURL(textBox *gtk.Box, title, displayUR | |||
| 1799 | 1800 | titleLabel.SetText(displayTitle) | |
| 1800 | 1801 | titleLabel.AddCssClass("omnibox-suggestion-title") | |
| 1801 | 1802 | titleLabel.SetHalign(gtk.AlignStartValue) | |
| 1802 | - | titleLabel.SetEllipsize(2) | |
| 1803 | + | titleLabel.SetEllipsize(pango.EllipsizeEndValue) | |
| 1803 | 1804 | textBox.Append(&titleLabel.Widget) | |
| 1804 | 1805 | } | |
| 1805 | 1806 | ||
| @@ -1809,7 +1810,7 @@ func (o *Omnibox) appendSuggestionTitleAndURL(textBox *gtk.Box, title, displayUR | |||
| 1809 | 1810 | urlLabel.SetText(displayURL) | |
| 1810 | 1811 | urlLabel.AddCssClass("omnibox-suggestion-url") | |
| 1811 | 1812 | urlLabel.SetHalign(gtk.AlignStartValue) | |
| 1812 | - | urlLabel.SetEllipsize(2) | |
| 1813 | + | urlLabel.SetEllipsize(pango.EllipsizeEndValue) | |
| 1813 | 1814 | textBox.Append(&urlLabel.Widget) | |
| 1814 | 1815 | } | |
| 1815 | 1816 | } | |
| @@ -13,6 +13,7 @@ import ( | |||||
| 13 | "github.com/bnema/puregotk/v4/glib" | 13 | "github.com/bnema/puregotk/v4/glib" | ||
| 14 | "github.com/bnema/puregotk/v4/graphene" | 14 | "github.com/bnema/puregotk/v4/graphene" | ||
| 15 | "github.com/bnema/puregotk/v4/gtk" | 15 | "github.com/bnema/puregotk/v4/gtk" | ||
| 16 | + | "github.com/bnema/puregotk/v4/pango" | |||
| 16 | 17 | ||||
| 17 | "github.com/bnema/dumber/internal/application/usecase" | 18 | "github.com/bnema/dumber/internal/application/usecase" | ||
| 18 | "github.com/bnema/dumber/internal/domain/autocomplete" | 19 | "github.com/bnema/dumber/internal/domain/autocomplete" | ||
| @@ -1799,7 +1800,7 @@ func (o *Omnibox) appendSuggestionTitleAndURL(textBox *gtk.Box, title, displayUR | |||||
| 1799 | titleLabel.SetText(displayTitle) | 1800 | titleLabel.SetText(displayTitle) | ||
| 1800 | titleLabel.AddCssClass("omnibox-suggestion-title") | 1801 | titleLabel.AddCssClass("omnibox-suggestion-title") | ||
| 1801 | titleLabel.SetHalign(gtk.AlignStartValue) | 1802 | titleLabel.SetHalign(gtk.AlignStartValue) | ||
| 1802 | - | titleLabel.SetEllipsize(2) | 1803 | + | titleLabel.SetEllipsize(pango.EllipsizeEndValue) |
| 1803 | textBox.Append(&titleLabel.Widget) | 1804 | textBox.Append(&titleLabel.Widget) | ||
| 1804 | } | 1805 | } | ||
| 1805 | 1806 | ||||
| @@ -1809,7 +1810,7 @@ func (o *Omnibox) appendSuggestionTitleAndURL(textBox *gtk.Box, title, displayUR | |||||
| 1809 | urlLabel.SetText(displayURL) | 1810 | urlLabel.SetText(displayURL) | ||
| 1810 | urlLabel.AddCssClass("omnibox-suggestion-url") | 1811 | urlLabel.AddCssClass("omnibox-suggestion-url") | ||
| 1811 | urlLabel.SetHalign(gtk.AlignStartValue) | 1812 | urlLabel.SetHalign(gtk.AlignStartValue) | ||
| 1812 | - | urlLabel.SetEllipsize(2) | 1813 | + | urlLabel.SetEllipsize(pango.EllipsizeEndValue) |
| 1813 | textBox.Append(&urlLabel.Widget) | 1814 | textBox.Append(&urlLabel.Widget) | ||
| 1814 | } | 1815 | } | ||
| 1815 | } | 1816 | } | ||
M internal/ui/coordinator/navigation_test.go +0 -11
| @@ -39,14 +39,3 @@ func TestNavigationCoordinator_OmniboxCallbackDoesNotPanicOnNavigateError(t *tes | |||
| 39 | 39 | provider.onNavigate("https://example.com") | |
| 40 | 40 | }) | |
| 41 | 41 | } | |
| 42 | - | ||
| 43 | - | func TestNavigationCoordinator_SetOmniboxProviderInstallsNavigateCallback(t *testing.T) { | |
| 44 | - | c := &NavigationCoordinator{} | |
| 45 | - | provider := &fakeOmniboxProvider{} | |
| 46 | - | ||
| 47 | - | c.SetOmniboxProvider(provider) | |
| 48 | - | ||
| 49 | - | if provider.onNavigate == nil { | |
| 50 | - | t.Fatalf("expected omnibox navigate callback to be installed") | |
| 51 | - | } | |
| 52 | - | } | |
| @@ -39,14 +39,3 @@ func TestNavigationCoordinator_OmniboxCallbackDoesNotPanicOnNavigateError(t *tes | |||||
| 39 | provider.onNavigate("https://example.com") | 39 | provider.onNavigate("https://example.com") | ||
| 40 | }) | 40 | }) | ||
| 41 | } | 41 | } | ||
| 42 | - | ||||
| 43 | - | func TestNavigationCoordinator_SetOmniboxProviderInstallsNavigateCallback(t *testing.T) { | |||
| 44 | - | c := &NavigationCoordinator{} | |||
| 45 | - | provider := &fakeOmniboxProvider{} | |||
| 46 | - | ||||
| 47 | - | c.SetOmniboxProvider(provider) | |||
| 48 | - | ||||
| 49 | - | if provider.onNavigate == nil { | |||
| 50 | - | t.Fatalf("expected omnibox navigate callback to be installed") | |||
| 51 | - | } | |||
| 52 | - | } | |||
M internal/ui/standalone_omnibox.go +1 -2
| @@ -222,7 +222,6 @@ func buildStandaloneOmniboxHost( | |||
| 222 | 222 | ) (*gtk.Widget, *component.Omnibox) { | |
| 223 | 223 | factory := layout.NewGtkWidgetFactory() | |
| 224 | 224 | overlay := factory.NewOverlay() | |
| 225 | - | hostLayout := standaloneOmniboxHostLayout(false) | |
| 226 | 225 | if overlay == nil { | |
| 227 | 226 | logging.FromContext(ctx).Error().Msg("failed to create standalone omnibox host widgets") | |
| 228 | 227 | return nil, nil | |
| @@ -230,7 +229,7 @@ func buildStandaloneOmniboxHost( | |||
| 230 | 229 | ||
| 231 | 230 | cfg := runtimeCfg.OmniboxCfg | |
| 232 | 231 | cfg.OnNavigate = wrapStandaloneOmniboxNavigate(cfg.OnNavigate, gtkApp.Quit) | |
| 233 | - | cfg.SizeConfig = hostLayout.OmniboxSize | |
| 232 | + | cfg.SizeConfig = standaloneOmniboxSizeConfig() | |
| 234 | 233 | ||
| 235 | 234 | omnibox := component.NewOmnibox(ctx, cfg) | |
| 236 | 235 | if omnibox == nil { | |
| @@ -222,7 +222,6 @@ func buildStandaloneOmniboxHost( | |||||
| 222 | ) (*gtk.Widget, *component.Omnibox) { | 222 | ) (*gtk.Widget, *component.Omnibox) { | ||
| 223 | factory := layout.NewGtkWidgetFactory() | 223 | factory := layout.NewGtkWidgetFactory() | ||
| 224 | overlay := factory.NewOverlay() | 224 | overlay := factory.NewOverlay() | ||
| 225 | - | hostLayout := standaloneOmniboxHostLayout(false) | |||
| 226 | if overlay == nil { | 225 | if overlay == nil { | ||
| 227 | logging.FromContext(ctx).Error().Msg("failed to create standalone omnibox host widgets") | 226 | logging.FromContext(ctx).Error().Msg("failed to create standalone omnibox host widgets") | ||
| 228 | return nil, nil | 227 | return nil, nil | ||
| @@ -230,7 +229,7 @@ func buildStandaloneOmniboxHost( | |||||
| 230 | 229 | ||||
| 231 | cfg := runtimeCfg.OmniboxCfg | 230 | cfg := runtimeCfg.OmniboxCfg | ||
| 232 | cfg.OnNavigate = wrapStandaloneOmniboxNavigate(cfg.OnNavigate, gtkApp.Quit) | 231 | cfg.OnNavigate = wrapStandaloneOmniboxNavigate(cfg.OnNavigate, gtkApp.Quit) | ||
| 233 | - | cfg.SizeConfig = hostLayout.OmniboxSize | 232 | + | cfg.SizeConfig = standaloneOmniboxSizeConfig() |
| 234 | 233 | ||||
| 235 | omnibox := component.NewOmnibox(ctx, cfg) | 234 | omnibox := component.NewOmnibox(ctx, cfg) | ||
| 236 | if omnibox == nil { | 235 | if omnibox == nil { | ||