detached e83c735

fix(review): resolve remaining CodeRabbit findings

Harden floating-pane rollback behavior under navigation errors, make floating profile validation deterministic, and preserve callback logging context in omnibox navigation. Improve installer prerelease parsing reliability with jq-first extraction and documented awk fallback limits.
commit
e83c735b8f99eb08972165801af6e82282a7773a
author
brice · · 2026-02-09 14:59 UTC
parent
921cc59
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
7 files · +145 -12
Split
UnifiedSplit
M install.sh +37 -3
@@ -42,12 +42,34 @@ TARBALL="dumber_linux_x86_64.tar.gz"
4242 # Determine version to download
4343 if [ "$VERSION" = "latest" ] && [ -n "$DUMBER_PRERELEASE" ]; then
4444 echo "Finding latest pre-release..."
45- RELEASE_DATA=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases?per_page=1" 2>/dev/null || echo "")
45+ RELEASE_DATA=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases?per_page=100" 2>/dev/null || echo "")
4646 if [ -z "$RELEASE_DATA" ]; then
4747 echo "Error: Failed to fetch release information from GitHub API"
4848 exit 1
4949 fi
50- VERSION=$(echo "$RELEASE_DATA" | grep -m1 '"tag_name"' | sed -E 's/.*"tag_name":\s*"([^"]+)".*/\1/')
50+ # Prefer jq for reliable JSON parsing of RELEASE_DATA into VERSION.
51+ # The awk branch is a portability fallback that assumes prerelease and
52+ # tag_name appear in the same '{' fragment; JSON strings containing '{'
53+ # or API shape changes can break this extraction.
54+ if command -v jq >/dev/null 2>&1; then
55+ VERSION=$(echo "$RELEASE_DATA" | jq -r '.[] | select(.prerelease == true) | .tag_name' | head -1)
56+ else
57+ VERSION=$(echo "$RELEASE_DATA" | awk '
58+ BEGIN { RS = "{" }
59+ /"prerelease"[[:space:]]*:[[:space:]]*true/ {
60+ tag = ""
61+ if (match($0, /"tag_name"[[:space:]]*:[[:space:]]*"[^"]+"/)) {
62+ tag = substr($0, RSTART, RLENGTH)
63+ sub(/.*"tag_name"[[:space:]]*:[[:space:]]*"/, "", tag)
64+ sub(/".*/, "", tag)
65+ }
66+ if (tag != "") {
67+ print tag
68+ exit
69+ }
70+ }
71+ ')
72+ fi
5173 if [ -z "$VERSION" ]; then
5274 echo "Error: Could not determine latest pre-release version"
5375 exit 1
@@ -60,7 +82,19 @@ elif [ "$VERSION" = "latest" ]; then
6082 echo "Error: Failed to fetch latest release information from GitHub API"
6183 exit 1
6284 fi
63- VERSION=$(echo "$RELEASE_DATA" | grep -m1 '"tag_name"' | sed -E 's/.*"tag_name":\s*"([^"]+)".*/\1/')
85+ if command -v jq >/dev/null 2>&1; then
86+ VERSION=$(echo "$RELEASE_DATA" | jq -r '.tag_name')
87+ else
88+ VERSION=$(echo "$RELEASE_DATA" | awk '
89+ /"tag_name"[[:space:]]*:/ {
90+ tag = $0
91+ sub(/.*"tag_name"[[:space:]]*:[[:space:]]*"/, "", tag)
92+ sub(/".*/, "", tag)
93+ print tag
94+ exit
95+ }
96+ ')
97+ fi
6498 if [ -z "$VERSION" ]; then
6599 echo "Error: Could not determine latest stable version"
66100 exit 1
@@ -42,12 +42,34 @@ TARBALL="dumber_linux_x86_64.tar.gz"
42 # Determine version to download42 # Determine version to download
43 if [ "$VERSION" = "latest" ] && [ -n "$DUMBER_PRERELEASE" ]; then43 if [ "$VERSION" = "latest" ] && [ -n "$DUMBER_PRERELEASE" ]; then
44 echo "Finding latest pre-release..."44 echo "Finding latest pre-release..."
45- RELEASE_DATA=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases?per_page=1" 2>/dev/null || echo "")45+ RELEASE_DATA=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases?per_page=100" 2>/dev/null || echo "")
46 if [ -z "$RELEASE_DATA" ]; then46 if [ -z "$RELEASE_DATA" ]; then
47 echo "Error: Failed to fetch release information from GitHub API"47 echo "Error: Failed to fetch release information from GitHub API"
48 exit 148 exit 1
49 fi49 fi
50- VERSION=$(echo "$RELEASE_DATA" | grep -m1 '"tag_name"' | sed -E 's/.*"tag_name":\s*"([^"]+)".*/\1/')50+ # Prefer jq for reliable JSON parsing of RELEASE_DATA into VERSION.
51+ # The awk branch is a portability fallback that assumes prerelease and
52+ # tag_name appear in the same '{' fragment; JSON strings containing '{'
53+ # or API shape changes can break this extraction.
54+ if command -v jq >/dev/null 2>&1; then
55+ VERSION=$(echo "$RELEASE_DATA" | jq -r '.[] | select(.prerelease == true) | .tag_name' | head -1)
56+ else
57+ VERSION=$(echo "$RELEASE_DATA" | awk '
58+ BEGIN { RS = "{" }
59+ /"prerelease"[[:space:]]*:[[:space:]]*true/ {
60+ tag = ""
61+ if (match($0, /"tag_name"[[:space:]]*:[[:space:]]*"[^"]+"/)) {
62+ tag = substr($0, RSTART, RLENGTH)
63+ sub(/.*"tag_name"[[:space:]]*:[[:space:]]*"/, "", tag)
64+ sub(/".*/, "", tag)
65+ }
66+ if (tag != "") {
67+ print tag
68+ exit
69+ }
70+ }
71+ ')
72+ fi
51 if [ -z "$VERSION" ]; then73 if [ -z "$VERSION" ]; then
52 echo "Error: Could not determine latest pre-release version"74 echo "Error: Could not determine latest pre-release version"
53 exit 175 exit 1
@@ -60,7 +82,19 @@ elif [ "$VERSION" = "latest" ]; then
60 echo "Error: Failed to fetch latest release information from GitHub API"82 echo "Error: Failed to fetch latest release information from GitHub API"
61 exit 183 exit 1
62 fi84 fi
63- VERSION=$(echo "$RELEASE_DATA" | grep -m1 '"tag_name"' | sed -E 's/.*"tag_name":\s*"([^"]+)".*/\1/')85+ if command -v jq >/dev/null 2>&1; then
86+ VERSION=$(echo "$RELEASE_DATA" | jq -r '.tag_name')
87+ else
88+ VERSION=$(echo "$RELEASE_DATA" | awk '
89+ /"tag_name"[[:space:]]*:/ {
90+ tag = $0
91+ sub(/.*"tag_name"[[:space:]]*:[[:space:]]*"/, "", tag)
92+ sub(/".*/, "", tag)
93+ print tag
94+ exit
95+ }
96+ ')
97+ fi
64 if [ -z "$VERSION" ]; then98 if [ -z "$VERSION" ]; then
65 echo "Error: Could not determine latest stable version"99 echo "Error: Could not determine latest stable version"
66 exit 1100 exit 1
M internal/infrastructure/config/migrate.go +3 -2
@@ -173,7 +173,7 @@ func (m *Migrator) DetectChanges() ([]port.KeyChange, error) {
173173 func (m *Migrator) findDeprecatedKeys(userKeys map[string]any, defaultKeys map[string]bool) []string {
174174 var deprecated []string
175175 for userKey := range userKeys {
176- if userKey == "database.path" {
176+ if userKey == databasePathKey {
177177 continue
178178 }
179179 if !m.keyOrRelatedExistsInDefaults(userKey, defaultKeys) {
@@ -264,6 +264,7 @@ const (
264264 configFormatTOML = "toml"
265265 configFormatYAML = "yaml"
266266 configFormatJSON = "json"
267+ databasePathKey = "database.path"
267268 workspaceShortcutsActionsKey = "workspace.shortcuts.actions"
268269 )
269270
@@ -629,7 +630,7 @@ func (m *Migrator) getAllDefaultKeys() []string {
629630 filtered := make([]string, 0, len(keys))
630631 for _, key := range keys {
631632 // Skip database.path as it's set dynamically
632- if key == "database.path" {
633+ if key == databasePathKey {
633634 continue
634635 }
635636 filtered = append(filtered, key)
@@ -173,7 +173,7 @@ func (m *Migrator) DetectChanges() ([]port.KeyChange, error) {
173 func (m *Migrator) findDeprecatedKeys(userKeys map[string]any, defaultKeys map[string]bool) []string {173 func (m *Migrator) findDeprecatedKeys(userKeys map[string]any, defaultKeys map[string]bool) []string {
174 var deprecated []string174 var deprecated []string
175 for userKey := range userKeys {175 for userKey := range userKeys {
176- if userKey == "database.path" {176+ if userKey == databasePathKey {
177 continue177 continue
178 }178 }
179 if !m.keyOrRelatedExistsInDefaults(userKey, defaultKeys) {179 if !m.keyOrRelatedExistsInDefaults(userKey, defaultKeys) {
@@ -264,6 +264,7 @@ const (
264 configFormatTOML = "toml"264 configFormatTOML = "toml"
265 configFormatYAML = "yaml"265 configFormatYAML = "yaml"
266 configFormatJSON = "json"266 configFormatJSON = "json"
267+ databasePathKey = "database.path"
267 workspaceShortcutsActionsKey = "workspace.shortcuts.actions"268 workspaceShortcutsActionsKey = "workspace.shortcuts.actions"
268 )269 )
269 270
@@ -629,7 +630,7 @@ func (m *Migrator) getAllDefaultKeys() []string {
629 filtered := make([]string, 0, len(keys))630 filtered := make([]string, 0, len(keys))
630 for _, key := range keys {631 for _, key := range keys {
631 // Skip database.path as it's set dynamically632 // Skip database.path as it's set dynamically
632- if key == "database.path" {633+ if key == databasePathKey {
633 continue634 continue
634 }635 }
635 filtered = append(filtered, key)636 filtered = append(filtered, key)
M internal/infrastructure/config/validation.go +9 -1
@@ -3,6 +3,7 @@ package config
33 import (
44 "fmt"
55 "net/url"
6+ "sort"
67 "strings"
78
89 domainurl "github.com/bnema/dumber/internal/domain/url"
@@ -237,7 +238,14 @@ func validateFloatingPane(config *Config) []string {
237238 validationErrors = append(validationErrors, "workspace.floating_pane.height_pct must be in (0,1]")
238239 }
239240
240- for profileName, profile := range config.Workspace.FloatingPane.Profiles {
241+ profileNames := make([]string, 0, len(config.Workspace.FloatingPane.Profiles))
242+ for profileName := range config.Workspace.FloatingPane.Profiles {
243+ profileNames = append(profileNames, profileName)
244+ }
245+ sort.Strings(profileNames)
246+
247+ for _, profileName := range profileNames {
248+ profile := config.Workspace.FloatingPane.Profiles[profileName]
241249 if strings.TrimSpace(profile.URL) == "" {
242250 validationErrors = append(validationErrors, fmt.Sprintf(
243251 "workspace.floating_pane.profiles.%s.url is required",
@@ -3,6 +3,7 @@ package config
3 import (3 import (
4 "fmt"4 "fmt"
5 "net/url"5 "net/url"
6+ "sort"
6 "strings"7 "strings"
7 8
8 domainurl "github.com/bnema/dumber/internal/domain/url"9 domainurl "github.com/bnema/dumber/internal/domain/url"
@@ -237,7 +238,14 @@ func validateFloatingPane(config *Config) []string {
237 validationErrors = append(validationErrors, "workspace.floating_pane.height_pct must be in (0,1]")238 validationErrors = append(validationErrors, "workspace.floating_pane.height_pct must be in (0,1]")
238 }239 }
239 240
240- for profileName, profile := range config.Workspace.FloatingPane.Profiles {241+ profileNames := make([]string, 0, len(config.Workspace.FloatingPane.Profiles))
242+ for profileName := range config.Workspace.FloatingPane.Profiles {
243+ profileNames = append(profileNames, profileName)
244+ }
245+ sort.Strings(profileNames)
246+
247+ for _, profileName := range profileNames {
248+ profile := config.Workspace.FloatingPane.Profiles[profileName]
241 if strings.TrimSpace(profile.URL) == "" {249 if strings.TrimSpace(profile.URL) == "" {
242 validationErrors = append(validationErrors, fmt.Sprintf(250 validationErrors = append(validationErrors, fmt.Sprintf(
243 "workspace.floating_pane.profiles.%s.url is required",251 "workspace.floating_pane.profiles.%s.url is required",
M internal/ui/component/floating_pane.go +30 -3
@@ -33,6 +33,7 @@ type FloatingPane struct {
3333 omniboxVisible bool
3434 sessionStarted bool
3535 currentURL string
36+ stateVersion uint64
3637 calculatedWidth int
3738 calculatedHeight int
3839 }
@@ -67,9 +68,12 @@ func NewFloatingPane(parent layout.OverlayWidget, opts FloatingPaneOptions) *Flo
6768 // ShowToggle toggles pane visibility; first open initializes a blank session.
6869 func (fp *FloatingPane) ShowToggle(ctx context.Context) error {
6970 fp.mu.Lock()
71+ previousVisible := fp.visible
72+ previousOmniboxVisible := fp.omniboxVisible
7073 if fp.visible {
7174 fp.visible = false
7275 fp.omniboxVisible = false
76+ fp.stateVersion++
7377 fp.mu.Unlock()
7478 return nil
7579 }
@@ -78,6 +82,8 @@ func (fp *FloatingPane) ShowToggle(ctx context.Context) error {
7882 showOmnibox := navigateToBlank || fp.currentURL == defaultFloatingPaneURL
7983 fp.visible = true
8084 fp.omniboxVisible = showOmnibox
85+ fp.stateVersion++
86+ rollbackVersion := fp.stateVersion
8187 // Release lock before Navigate: callbacks may block and must not run while
8288 // holding fp.mu. On error, rollback below restores visibility state.
8389 fp.mu.Unlock()
@@ -85,8 +91,11 @@ func (fp *FloatingPane) ShowToggle(ctx context.Context) error {
8591 if navigateToBlank {
8692 if err := fp.Navigate(ctx, defaultFloatingPaneURL); err != nil {
8793 fp.mu.Lock()
88- fp.visible = false
89- fp.omniboxVisible = false
94+ if fp.stateVersion == rollbackVersion {
95+ fp.visible = previousVisible
96+ fp.omniboxVisible = previousOmniboxVisible
97+ fp.stateVersion++
98+ }
9099 fp.mu.Unlock()
91100 return err
92101 }
@@ -102,6 +111,7 @@ func (fp *FloatingPane) Show() {
102111 defer fp.mu.Unlock()
103112 fp.visible = true
104113 fp.omniboxVisible = false
114+ fp.stateVersion++
105115 }
106116
107117 // SessionStarted reports whether the floating pane has navigated at least once.
@@ -119,13 +129,26 @@ func (fp *FloatingPane) ShowURL(ctx context.Context, url string) error {
119129 }
120130
121131 fp.mu.Lock()
132+ previousVisible := fp.visible
133+ previousOmniboxVisible := fp.omniboxVisible
122134 fp.visible = true
123135 fp.omniboxVisible = false
124136 shouldNavigate := !fp.sessionStarted || fp.currentURL != url
137+ fp.stateVersion++
138+ rollbackVersion := fp.stateVersion
125139 fp.mu.Unlock()
126140
127141 if shouldNavigate {
128- return fp.Navigate(ctx, url)
142+ if err := fp.Navigate(ctx, url); err != nil {
143+ fp.mu.Lock()
144+ if fp.stateVersion == rollbackVersion {
145+ fp.visible = previousVisible
146+ fp.omniboxVisible = previousOmniboxVisible
147+ fp.stateVersion++
148+ }
149+ fp.mu.Unlock()
150+ return err
151+ }
129152 }
130153
131154 return nil
@@ -137,6 +160,7 @@ func (fp *FloatingPane) Hide(_ context.Context) {
137160 defer fp.mu.Unlock()
138161 fp.visible = false
139162 fp.omniboxVisible = false
163+ fp.stateVersion++
140164 }
141165
142166 // Navigate loads a URL into the floating pane's persistent session.
@@ -159,6 +183,7 @@ func (fp *FloatingPane) Navigate(ctx context.Context, url string) error {
159183 fp.mu.Lock()
160184 fp.currentURL = url
161185 fp.sessionStarted = true
186+ fp.stateVersion++
162187 fp.mu.Unlock()
163188
164189 return nil
@@ -217,6 +242,7 @@ func (fp *FloatingPane) IsOmniboxVisible() bool {
217242 func (fp *FloatingPane) SetOmniboxVisible(visible bool) {
218243 fp.mu.Lock()
219244 fp.omniboxVisible = visible
245+ fp.stateVersion++
220246 fp.mu.Unlock()
221247 }
222248
@@ -230,5 +256,6 @@ func (fp *FloatingPane) RecordLoadedURL(url string) {
230256 fp.mu.Lock()
231257 fp.currentURL = url
232258 fp.sessionStarted = true
259+ fp.stateVersion++
233260 fp.mu.Unlock()
234261 }
@@ -33,6 +33,7 @@ type FloatingPane struct {
33 omniboxVisible bool33 omniboxVisible bool
34 sessionStarted bool34 sessionStarted bool
35 currentURL string35 currentURL string
36+ stateVersion uint64
36 calculatedWidth int37 calculatedWidth int
37 calculatedHeight int38 calculatedHeight int
38 }39 }
@@ -67,9 +68,12 @@ func NewFloatingPane(parent layout.OverlayWidget, opts FloatingPaneOptions) *Flo
67 // ShowToggle toggles pane visibility; first open initializes a blank session.68 // ShowToggle toggles pane visibility; first open initializes a blank session.
68 func (fp *FloatingPane) ShowToggle(ctx context.Context) error {69 func (fp *FloatingPane) ShowToggle(ctx context.Context) error {
69 fp.mu.Lock()70 fp.mu.Lock()
71+ previousVisible := fp.visible
72+ previousOmniboxVisible := fp.omniboxVisible
70 if fp.visible {73 if fp.visible {
71 fp.visible = false74 fp.visible = false
72 fp.omniboxVisible = false75 fp.omniboxVisible = false
76+ fp.stateVersion++
73 fp.mu.Unlock()77 fp.mu.Unlock()
74 return nil78 return nil
75 }79 }
@@ -78,6 +82,8 @@ func (fp *FloatingPane) ShowToggle(ctx context.Context) error {
78 showOmnibox := navigateToBlank || fp.currentURL == defaultFloatingPaneURL82 showOmnibox := navigateToBlank || fp.currentURL == defaultFloatingPaneURL
79 fp.visible = true83 fp.visible = true
80 fp.omniboxVisible = showOmnibox84 fp.omniboxVisible = showOmnibox
85+ fp.stateVersion++
86+ rollbackVersion := fp.stateVersion
81 // Release lock before Navigate: callbacks may block and must not run while87 // Release lock before Navigate: callbacks may block and must not run while
82 // holding fp.mu. On error, rollback below restores visibility state.88 // holding fp.mu. On error, rollback below restores visibility state.
83 fp.mu.Unlock()89 fp.mu.Unlock()
@@ -85,8 +91,11 @@ func (fp *FloatingPane) ShowToggle(ctx context.Context) error {
85 if navigateToBlank {91 if navigateToBlank {
86 if err := fp.Navigate(ctx, defaultFloatingPaneURL); err != nil {92 if err := fp.Navigate(ctx, defaultFloatingPaneURL); err != nil {
87 fp.mu.Lock()93 fp.mu.Lock()
88- fp.visible = false94+ if fp.stateVersion == rollbackVersion {
89- fp.omniboxVisible = false95+ fp.visible = previousVisible
96+ fp.omniboxVisible = previousOmniboxVisible
97+ fp.stateVersion++
98+ }
90 fp.mu.Unlock()99 fp.mu.Unlock()
91 return err100 return err
92 }101 }
@@ -102,6 +111,7 @@ func (fp *FloatingPane) Show() {
102 defer fp.mu.Unlock()111 defer fp.mu.Unlock()
103 fp.visible = true112 fp.visible = true
104 fp.omniboxVisible = false113 fp.omniboxVisible = false
114+ fp.stateVersion++
105 }115 }
106 116
107 // SessionStarted reports whether the floating pane has navigated at least once.117 // SessionStarted reports whether the floating pane has navigated at least once.
@@ -119,13 +129,26 @@ func (fp *FloatingPane) ShowURL(ctx context.Context, url string) error {
119 }129 }
120 130
121 fp.mu.Lock()131 fp.mu.Lock()
132+ previousVisible := fp.visible
133+ previousOmniboxVisible := fp.omniboxVisible
122 fp.visible = true134 fp.visible = true
123 fp.omniboxVisible = false135 fp.omniboxVisible = false
124 shouldNavigate := !fp.sessionStarted || fp.currentURL != url136 shouldNavigate := !fp.sessionStarted || fp.currentURL != url
137+ fp.stateVersion++
138+ rollbackVersion := fp.stateVersion
125 fp.mu.Unlock()139 fp.mu.Unlock()
126 140
127 if shouldNavigate {141 if shouldNavigate {
128- return fp.Navigate(ctx, url)142+ if err := fp.Navigate(ctx, url); err != nil {
143+ fp.mu.Lock()
144+ if fp.stateVersion == rollbackVersion {
145+ fp.visible = previousVisible
146+ fp.omniboxVisible = previousOmniboxVisible
147+ fp.stateVersion++
148+ }
149+ fp.mu.Unlock()
150+ return err
151+ }
129 }152 }
130 153
131 return nil154 return nil
@@ -137,6 +160,7 @@ func (fp *FloatingPane) Hide(_ context.Context) {
137 defer fp.mu.Unlock()160 defer fp.mu.Unlock()
138 fp.visible = false161 fp.visible = false
139 fp.omniboxVisible = false162 fp.omniboxVisible = false
163+ fp.stateVersion++
140 }164 }
141 165
142 // Navigate loads a URL into the floating pane's persistent session.166 // Navigate loads a URL into the floating pane's persistent session.
@@ -159,6 +183,7 @@ func (fp *FloatingPane) Navigate(ctx context.Context, url string) error {
159 fp.mu.Lock()183 fp.mu.Lock()
160 fp.currentURL = url184 fp.currentURL = url
161 fp.sessionStarted = true185 fp.sessionStarted = true
186+ fp.stateVersion++
162 fp.mu.Unlock()187 fp.mu.Unlock()
163 188
164 return nil189 return nil
@@ -217,6 +242,7 @@ func (fp *FloatingPane) IsOmniboxVisible() bool {
217 func (fp *FloatingPane) SetOmniboxVisible(visible bool) {242 func (fp *FloatingPane) SetOmniboxVisible(visible bool) {
218 fp.mu.Lock()243 fp.mu.Lock()
219 fp.omniboxVisible = visible244 fp.omniboxVisible = visible
245+ fp.stateVersion++
220 fp.mu.Unlock()246 fp.mu.Unlock()
221 }247 }
222 248
@@ -230,5 +256,6 @@ func (fp *FloatingPane) RecordLoadedURL(url string) {
230 fp.mu.Lock()256 fp.mu.Lock()
231 fp.currentURL = url257 fp.currentURL = url
232 fp.sessionStarted = true258 fp.sessionStarted = true
259+ fp.stateVersion++
233 fp.mu.Unlock()260 fp.mu.Unlock()
234 }261 }
M internal/ui/component/floating_pane_test.go +52 -0
@@ -2,6 +2,7 @@ package component_test
22
33 import (
44 "context"
5+ "errors"
56 "sync"
67 "testing"
78
@@ -150,3 +151,54 @@ func TestFloatingPane_ConcurrentResizeAndParentSwitch(t *testing.T) {
150151 assert.Positive(t, width)
151152 assert.Positive(t, height)
152153 }
154+
155+func TestFloatingPane_ShowURL_RollsBackVisibilityOnNavigateError(t *testing.T) {
156+ expectedErr := errors.New("navigate failed")
157+ fp := component.NewFloatingPane(nil, component.FloatingPaneOptions{
158+ WidthPct: 0.82,
159+ HeightPct: 0.72,
160+ FallbackWidth: 1200,
161+ FallbackHeight: 800,
162+ OnNavigate: func(context.Context, string) error {
163+ return expectedErr
164+ },
165+ })
166+
167+ err := fp.ShowURL(context.Background(), "https://example.com")
168+ require.ErrorIs(t, err, expectedErr)
169+ assert.False(t, fp.IsVisible())
170+ assert.False(t, fp.IsOmniboxVisible())
171+ assert.Empty(t, fp.CurrentURL())
172+}
173+
174+func TestFloatingPane_ShowToggleNavigateError_DoesNotClobberConcurrentStateChange(t *testing.T) {
175+ navigationStarted := make(chan struct{})
176+ releaseNavigation := make(chan struct{})
177+ expectedErr := errors.New("navigate failed")
178+
179+ fp := component.NewFloatingPane(nil, component.FloatingPaneOptions{
180+ WidthPct: 0.82,
181+ HeightPct: 0.72,
182+ FallbackWidth: 1200,
183+ FallbackHeight: 800,
184+ OnNavigate: func(context.Context, string) error {
185+ close(navigationStarted)
186+ <-releaseNavigation
187+ return expectedErr
188+ },
189+ })
190+
191+ errCh := make(chan error, 1)
192+ go func() {
193+ errCh <- fp.ShowToggle(context.Background())
194+ }()
195+
196+ <-navigationStarted
197+ fp.Show()
198+ close(releaseNavigation)
199+
200+ err := <-errCh
201+ require.ErrorIs(t, err, expectedErr)
202+ assert.True(t, fp.IsVisible())
203+ assert.False(t, fp.IsOmniboxVisible())
204+}
@@ -2,6 +2,7 @@ package component_test
2 2
3 import (3 import (
4 "context"4 "context"
5+ "errors"
5 "sync"6 "sync"
6 "testing"7 "testing"
7 8
@@ -150,3 +151,54 @@ func TestFloatingPane_ConcurrentResizeAndParentSwitch(t *testing.T) {
150 assert.Positive(t, width)151 assert.Positive(t, width)
151 assert.Positive(t, height)152 assert.Positive(t, height)
152 }153 }
154+
155+func TestFloatingPane_ShowURL_RollsBackVisibilityOnNavigateError(t *testing.T) {
156+ expectedErr := errors.New("navigate failed")
157+ fp := component.NewFloatingPane(nil, component.FloatingPaneOptions{
158+ WidthPct: 0.82,
159+ HeightPct: 0.72,
160+ FallbackWidth: 1200,
161+ FallbackHeight: 800,
162+ OnNavigate: func(context.Context, string) error {
163+ return expectedErr
164+ },
165+ })
166+
167+ err := fp.ShowURL(context.Background(), "https://example.com")
168+ require.ErrorIs(t, err, expectedErr)
169+ assert.False(t, fp.IsVisible())
170+ assert.False(t, fp.IsOmniboxVisible())
171+ assert.Empty(t, fp.CurrentURL())
172+}
173+
174+func TestFloatingPane_ShowToggleNavigateError_DoesNotClobberConcurrentStateChange(t *testing.T) {
175+ navigationStarted := make(chan struct{})
176+ releaseNavigation := make(chan struct{})
177+ expectedErr := errors.New("navigate failed")
178+
179+ fp := component.NewFloatingPane(nil, component.FloatingPaneOptions{
180+ WidthPct: 0.82,
181+ HeightPct: 0.72,
182+ FallbackWidth: 1200,
183+ FallbackHeight: 800,
184+ OnNavigate: func(context.Context, string) error {
185+ close(navigationStarted)
186+ <-releaseNavigation
187+ return expectedErr
188+ },
189+ })
190+
191+ errCh := make(chan error, 1)
192+ go func() {
193+ errCh <- fp.ShowToggle(context.Background())
194+ }()
195+
196+ <-navigationStarted
197+ fp.Show()
198+ close(releaseNavigation)
199+
200+ err := <-errCh
201+ require.ErrorIs(t, err, expectedErr)
202+ assert.True(t, fp.IsVisible())
203+ assert.False(t, fp.IsOmniboxVisible())
204+}
M internal/ui/coordinator/navigation.go +11 -3
@@ -20,6 +20,7 @@ type OmniboxProvider interface {
2020
2121 // NavigationCoordinator handles URL navigation, history, and browser controls.
2222 type NavigationCoordinator struct {
23+ contextProvider func() context.Context
2324 navigateUC *usecase.NavigateUseCase
2425 contentCoord *ContentCoordinator
2526 omniboxProvider OmniboxProvider
@@ -35,8 +36,12 @@ func NewNavigationCoordinator(
3536 ) *NavigationCoordinator {
3637 log := logging.FromContext(ctx)
3738 log.Debug().Msg("creating navigation coordinator")
39+ callbackLogger := *log
3840
3941 return &NavigationCoordinator{
42+ contextProvider: func() context.Context {
43+ return logging.WithContext(context.Background(), callbackLogger)
44+ },
4045 navigateUC: navigateUC,
4146 contentCoord: contentCoord,
4247 }
@@ -47,9 +52,12 @@ func (c *NavigationCoordinator) SetOmniboxProvider(provider OmniboxProvider) {
4752 c.omniboxProvider = provider
4853 if c.omniboxProvider != nil {
4954 c.omniboxProvider.SetOmniboxOnNavigate(func(url string) {
50- ctx := context.Background()
51- if err := c.Navigate(ctx, url); err != nil {
52- logging.FromContext(ctx).Warn().Err(err).Str("url", url).Msg("omnibox-initiated navigation failed")
55+ navCtx := context.Background()
56+ if c.contextProvider != nil {
57+ navCtx = c.contextProvider()
58+ }
59+ if err := c.Navigate(navCtx, url); err != nil {
60+ logging.FromContext(navCtx).Warn().Err(err).Str("url", url).Msg("omnibox-initiated navigation failed")
5361 }
5462 })
5563 }
@@ -20,6 +20,7 @@ type OmniboxProvider interface {
20 20
21 // NavigationCoordinator handles URL navigation, history, and browser controls.21 // NavigationCoordinator handles URL navigation, history, and browser controls.
22 type NavigationCoordinator struct {22 type NavigationCoordinator struct {
23+ contextProvider func() context.Context
23 navigateUC *usecase.NavigateUseCase24 navigateUC *usecase.NavigateUseCase
24 contentCoord *ContentCoordinator25 contentCoord *ContentCoordinator
25 omniboxProvider OmniboxProvider26 omniboxProvider OmniboxProvider
@@ -35,8 +36,12 @@ func NewNavigationCoordinator(
35 ) *NavigationCoordinator {36 ) *NavigationCoordinator {
36 log := logging.FromContext(ctx)37 log := logging.FromContext(ctx)
37 log.Debug().Msg("creating navigation coordinator")38 log.Debug().Msg("creating navigation coordinator")
39+ callbackLogger := *log
38 40
39 return &NavigationCoordinator{41 return &NavigationCoordinator{
42+ contextProvider: func() context.Context {
43+ return logging.WithContext(context.Background(), callbackLogger)
44+ },
40 navigateUC: navigateUC,45 navigateUC: navigateUC,
41 contentCoord: contentCoord,46 contentCoord: contentCoord,
42 }47 }
@@ -47,9 +52,12 @@ func (c *NavigationCoordinator) SetOmniboxProvider(provider OmniboxProvider) {
47 c.omniboxProvider = provider52 c.omniboxProvider = provider
48 if c.omniboxProvider != nil {53 if c.omniboxProvider != nil {
49 c.omniboxProvider.SetOmniboxOnNavigate(func(url string) {54 c.omniboxProvider.SetOmniboxOnNavigate(func(url string) {
50- ctx := context.Background()55+ navCtx := context.Background()
51- if err := c.Navigate(ctx, url); err != nil {56+ if c.contextProvider != nil {
52- logging.FromContext(ctx).Warn().Err(err).Str("url", url).Msg("omnibox-initiated navigation failed")57+ navCtx = c.contextProvider()
58+ }
59+ if err := c.Navigate(navCtx, url); err != nil {
60+ logging.FromContext(navCtx).Warn().Err(err).Str("url", url).Msg("omnibox-initiated navigation failed")
53 }61 }
54 })62 })
55 }63 }
M internal/ui/floating_pane_test.go +3 -0
@@ -96,6 +96,9 @@ func TestFloatingAllocationRect_ClampsToOverlayBounds(t *testing.T) {
9696 }
9797
9898 func TestWriteOverlayAllocation_WritesRectangleFields(t *testing.T) {
99+ // cGtkAllocation mirrors GTK's GdkRectangle/GtkAllocation layout used by
100+ // writeOverlayAllocation: four int32 fields in order. allocationPtr uses
101+ // unsafe.Pointer to simulate the C allocation callback memory.
99102 type cGtkAllocation struct {
100103 X int32
101104 Y int32
@@ -96,6 +96,9 @@ func TestFloatingAllocationRect_ClampsToOverlayBounds(t *testing.T) {
96 }96 }
97 97
98 func TestWriteOverlayAllocation_WritesRectangleFields(t *testing.T) {98 func TestWriteOverlayAllocation_WritesRectangleFields(t *testing.T) {
99+ // cGtkAllocation mirrors GTK's GdkRectangle/GtkAllocation layout used by
100+ // writeOverlayAllocation: four int32 fields in order. allocationPtr uses
101+ // unsafe.Pointer to simulate the C allocation callback memory.
99 type cGtkAllocation struct {102 type cGtkAllocation struct {
100 X int32103 X int32
101 Y int32104 Y int32

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close