detached 4f21c98

fix: address code review findings (cancel focus, log privacy, timer races, pool cleanup, shortcut reload)

commit
4f21c98d952d09ea58f2bb970af596983f6746c4
author
brice · · 2026-03-13 18:47 UTC
parent
ea0fc29
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
6 files · +115 -7
Unified
UnifiedSplit
M internal/application/usecase/insert_accent.go +13 -6
@@ -137,16 +137,23 @@ func (uc *InsertAccentUseCase) Cancel(ctx context.Context) {
137137 log := logging.FromContext(ctx)
138138
139139 uc.mu.Lock()
140- defer uc.mu.Unlock()
140+ target := uc.targetInput
141+ wasVisible := uc.pickerVisible
142+ uc.cancelTimerLocked()
143+ if wasVisible {
144+ uc.accentPicker.Hide()
145+ uc.pickerVisible = false
146+ }
147+ uc.mu.Unlock()
141148
142- if uc.timer != nil || uc.pickerVisible {
149+ if wasVisible {
143150 log.Debug().Msg("canceling accent picker")
144151 }
145152
146- uc.cancelTimerLocked()
147- if uc.pickerVisible {
148- uc.accentPicker.Hide()
149- uc.pickerVisible = false
153+ if wasVisible {
154+ if focusable, ok := target.(port.Focusable); ok {
155+ focusable.Focus(ctx)
156+ }
150157 }
151158 }
152159
@@ -137,16 +137,23 @@ func (uc *InsertAccentUseCase) Cancel(ctx context.Context) {
137 log := logging.FromContext(ctx)137 log := logging.FromContext(ctx)
138 138
139 uc.mu.Lock()139 uc.mu.Lock()
140- defer uc.mu.Unlock()140+ target := uc.targetInput
141+ wasVisible := uc.pickerVisible
142+ uc.cancelTimerLocked()
143+ if wasVisible {
144+ uc.accentPicker.Hide()
145+ uc.pickerVisible = false
146+ }
147+ uc.mu.Unlock()
141 148
142- if uc.timer != nil || uc.pickerVisible {149+ if wasVisible {
143 log.Debug().Msg("canceling accent picker")150 log.Debug().Msg("canceling accent picker")
144 }151 }
145 152
146- uc.cancelTimerLocked()153+ if wasVisible {
147- if uc.pickerVisible {154+ if focusable, ok := target.(port.Focusable); ok {
148- uc.accentPicker.Hide()155+ focusable.Focus(ctx)
149- uc.pickerVisible = false156+ }
150 }157 }
151 }158 }
152 159
M internal/infrastructure/textinput/webview_target.go +1 -1
@@ -99,7 +99,7 @@ func (t *WebViewTarget) InsertText(ctx context.Context, text string) error {
9999 t.webView.EvaluateJavascript(script, -1, nil, nil, nil, nil, 0)
100100
101101 log.Debug().
102- Str("text", text).
102+ Int("len", len(text)).
103103 Msg("inserted text into WebView via JS")
104104
105105 return nil
@@ -99,7 +99,7 @@ func (t *WebViewTarget) InsertText(ctx context.Context, text string) error {
99 t.webView.EvaluateJavascript(script, -1, nil, nil, nil, nil, 0)99 t.webView.EvaluateJavascript(script, -1, nil, nil, nil, nil, 0)
100 100
101 log.Debug().101 log.Debug().
102- Str("text", text).102+ Int("len", len(text)).
103 Msg("inserted text into WebView via JS")103 Msg("inserted text into WebView via JS")
104 104
105 return nil105 return nil
M internal/infrastructure/webkit/webview.go +1 -0
@@ -1738,6 +1738,7 @@ func (wv *WebView) ResetForPoolReuse() {
17381738 wv.OnAudioStateChanged = nil
17391739 wv.OnLinkHover = nil
17401740 wv.OnWebProcessTerminated = nil
1741+ wv.OnPermissionRequest = nil
17411742
17421743 wv.mu.Lock()
17431744 wv.uri = ""
@@ -1738,6 +1738,7 @@ func (wv *WebView) ResetForPoolReuse() {
1738 wv.OnAudioStateChanged = nil1738 wv.OnAudioStateChanged = nil
1739 wv.OnLinkHover = nil1739 wv.OnLinkHover = nil
1740 wv.OnWebProcessTerminated = nil1740 wv.OnWebProcessTerminated = nil
1741+ wv.OnPermissionRequest = nil
1741 1742
1742 wv.mu.Lock()1743 wv.mu.Lock()
1743 wv.uri = ""1744 wv.uri = ""
M internal/ui/app.go +4 -0
@@ -3053,6 +3053,10 @@ func (a *App) initConfigWatcher(ctx context.Context) {
30533053 if a.keyboardHandler != nil {
30543054 a.keyboardHandler.ReloadShortcuts(ctx, cfgCopy)
30553055 }
3056+ // Reload global shortcuts (removes stale, re-registers from config)
3057+ if a.globalShortcutHandler != nil {
3058+ a.globalShortcutHandler.ReloadShortcuts(ctx, cfgCopy)
3059+ }
30563060 return false
30573061 })
30583062 glib.IdleAdd(&cb, 0)
@@ -3053,6 +3053,10 @@ func (a *App) initConfigWatcher(ctx context.Context) {
3053 if a.keyboardHandler != nil {3053 if a.keyboardHandler != nil {
3054 a.keyboardHandler.ReloadShortcuts(ctx, cfgCopy)3054 a.keyboardHandler.ReloadShortcuts(ctx, cfgCopy)
3055 }3055 }
3056+ // Reload global shortcuts (removes stale, re-registers from config)
3057+ if a.globalShortcutHandler != nil {
3058+ a.globalShortcutHandler.ReloadShortcuts(ctx, cfgCopy)
3059+ }
3056 return false3060 return false
3057 })3061 })
3058 glib.IdleAdd(&cb, 0)3062 glib.IdleAdd(&cb, 0)
M internal/ui/input/global_shortcuts.go +92 -0
@@ -18,6 +18,7 @@ import (
1818 // to intercept shortcuts before they reach the WebView.
1919 type GlobalShortcutHandler struct {
2020 controller *gtk.ShortcutController
21+ window *gtk.ApplicationWindow
2122 kbHandler *KeyboardHandler
2223 onAction ActionHandler
2324 ctx context.Context
@@ -41,6 +42,7 @@ func NewGlobalShortcutHandler(
4142
4243 h := &GlobalShortcutHandler{
4344 controller: gtk.NewShortcutController(),
45+ window: window,
4446 kbHandler: kbHandler,
4547 onAction: onAction,
4648 ctx: ctx,
@@ -244,6 +246,96 @@ func (h *GlobalShortcutHandler) registerShortcut(keyval uint, modifiers gdk.Modi
244246 return true
245247 }
246248
249+// ReloadShortcuts rebuilds global shortcuts from a new config.
250+// It replaces the GTK shortcut controller to ensure stale shortcuts
251+// are removed.
252+func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, cfg *config.Config) {
253+ log := logging.FromContext(ctx)
254+
255+ if h.window == nil || h.controller == nil {
256+ return
257+ }
258+
259+ // Remove old controller from window
260+ h.window.RemoveController(&h.controller.EventController)
261+
262+ // Create fresh controller
263+ h.controller = gtk.NewShortcutController()
264+ if h.controller == nil {
265+ log.Error().Msg("failed to create shortcut controller during reload")
266+ return
267+ }
268+ h.controller.SetScope(gtk.ShortcutScopeGlobalValue)
269+ h.callbacks = make([]gtk.ShortcutFunc, 0)
270+ h.registered = make(map[KeyBinding]Action)
271+
272+ // Re-register hardcoded shortcuts (Alt+1-9, Alt+0, Alt+Tab, Ctrl+Shift+S)
273+ tabActions := []Action{
274+ ActionSwitchTabIndex1, ActionSwitchTabIndex2, ActionSwitchTabIndex3,
275+ ActionSwitchTabIndex4, ActionSwitchTabIndex5, ActionSwitchTabIndex6,
276+ ActionSwitchTabIndex7, ActionSwitchTabIndex8, ActionSwitchTabIndex9,
277+ }
278+ for i, action := range tabActions {
279+ h.registerShortcut(uint(gdk.KEY_1)+uint(i), gdk.AltMaskValue, action)
280+ }
281+ h.registerShortcut(uint(gdk.KEY_0), gdk.AltMaskValue, ActionSwitchTabIndex10)
282+ h.registerShortcut(uint(gdk.KEY_Tab), gdk.AltMaskValue, ActionSwitchLastTab)
283+ h.registerShortcut(uint(gdk.KEY_s), gdk.ControlMaskValue|gdk.ShiftMaskValue, ActionOpenSessionManager)
284+
285+ // Re-register config-driven shortcuts
286+ if cfg != nil {
287+ actionMap := map[string]Action{
288+ "toggle_floating_pane": ActionToggleFloatingPane,
289+ "toggle-floating-pane": ActionToggleFloatingPane,
290+ "consume_or_expel_left": ActionConsumeOrExpelLeft,
291+ "consume_or_expel_right": ActionConsumeOrExpelRight,
292+ "consume_or_expel_up": ActionConsumeOrExpelUp,
293+ "consume_or_expel_down": ActionConsumeOrExpelDown,
294+ }
295+
296+ for actionName, actionBinding := range cfg.Workspace.Shortcuts.Actions {
297+ action, ok := actionMap[actionName]
298+ if !ok {
299+ continue
300+ }
301+ for _, keyStr := range actionBinding.Keys {
302+ binding, ok := ParseKeyString(keyStr)
303+ if !ok {
304+ continue
305+ }
306+ h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action)
307+ }
308+ }
309+
310+ occupied := make(map[KeyBinding]Action, len(h.registered))
311+ for binding, action := range h.registered {
312+ occupied[binding] = action
313+ }
314+ for _, shortcut := range collectFloatingProfileShortcuts(ctx, cfg, occupied) {
315+ if h.registerShortcut(shortcut.Binding.Keyval, gdk.ModifierType(shortcut.Binding.Modifiers), shortcut.Action) {
316+ if url, ok := ParseFloatingProfileAction(shortcut.Action); ok {
317+ log.Trace().Str("shortcut", formatBinding(shortcut.Binding)).Str("url", url).Msg("registered floating profile global shortcut")
318+ }
319+ }
320+ }
321+
322+ shortcuts := NewShortcutSet(ctx, cfg)
323+ for binding, action := range shortcuts.Global {
324+ if _, exists := h.registered[binding]; exists {
325+ continue
326+ }
327+ h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action)
328+ }
329+ }
330+
331+ // Attach new controller to window
332+ h.window.AddController(&h.controller.EventController)
333+
334+ log.Debug().
335+ Int("shortcuts", len(h.callbacks)).
336+ Msg("global shortcuts reloaded")
337+}
338+
247339 // Detach removes the global shortcut handler from the window.
248340 // Note: GTK handles cleanup when the widget is destroyed,
249341 // but we clear our references here.
@@ -18,6 +18,7 @@ import (
18 // to intercept shortcuts before they reach the WebView.18 // to intercept shortcuts before they reach the WebView.
19 type GlobalShortcutHandler struct {19 type GlobalShortcutHandler struct {
20 controller *gtk.ShortcutController20 controller *gtk.ShortcutController
21+ window *gtk.ApplicationWindow
21 kbHandler *KeyboardHandler22 kbHandler *KeyboardHandler
22 onAction ActionHandler23 onAction ActionHandler
23 ctx context.Context24 ctx context.Context
@@ -41,6 +42,7 @@ func NewGlobalShortcutHandler(
41 42
42 h := &GlobalShortcutHandler{43 h := &GlobalShortcutHandler{
43 controller: gtk.NewShortcutController(),44 controller: gtk.NewShortcutController(),
45+ window: window,
44 kbHandler: kbHandler,46 kbHandler: kbHandler,
45 onAction: onAction,47 onAction: onAction,
46 ctx: ctx,48 ctx: ctx,
@@ -244,6 +246,96 @@ func (h *GlobalShortcutHandler) registerShortcut(keyval uint, modifiers gdk.Modi
244 return true246 return true
245 }247 }
246 248
249+// ReloadShortcuts rebuilds global shortcuts from a new config.
250+// It replaces the GTK shortcut controller to ensure stale shortcuts
251+// are removed.
252+func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, cfg *config.Config) {
253+ log := logging.FromContext(ctx)
254+
255+ if h.window == nil || h.controller == nil {
256+ return
257+ }
258+
259+ // Remove old controller from window
260+ h.window.RemoveController(&h.controller.EventController)
261+
262+ // Create fresh controller
263+ h.controller = gtk.NewShortcutController()
264+ if h.controller == nil {
265+ log.Error().Msg("failed to create shortcut controller during reload")
266+ return
267+ }
268+ h.controller.SetScope(gtk.ShortcutScopeGlobalValue)
269+ h.callbacks = make([]gtk.ShortcutFunc, 0)
270+ h.registered = make(map[KeyBinding]Action)
271+
272+ // Re-register hardcoded shortcuts (Alt+1-9, Alt+0, Alt+Tab, Ctrl+Shift+S)
273+ tabActions := []Action{
274+ ActionSwitchTabIndex1, ActionSwitchTabIndex2, ActionSwitchTabIndex3,
275+ ActionSwitchTabIndex4, ActionSwitchTabIndex5, ActionSwitchTabIndex6,
276+ ActionSwitchTabIndex7, ActionSwitchTabIndex8, ActionSwitchTabIndex9,
277+ }
278+ for i, action := range tabActions {
279+ h.registerShortcut(uint(gdk.KEY_1)+uint(i), gdk.AltMaskValue, action)
280+ }
281+ h.registerShortcut(uint(gdk.KEY_0), gdk.AltMaskValue, ActionSwitchTabIndex10)
282+ h.registerShortcut(uint(gdk.KEY_Tab), gdk.AltMaskValue, ActionSwitchLastTab)
283+ h.registerShortcut(uint(gdk.KEY_s), gdk.ControlMaskValue|gdk.ShiftMaskValue, ActionOpenSessionManager)
284+
285+ // Re-register config-driven shortcuts
286+ if cfg != nil {
287+ actionMap := map[string]Action{
288+ "toggle_floating_pane": ActionToggleFloatingPane,
289+ "toggle-floating-pane": ActionToggleFloatingPane,
290+ "consume_or_expel_left": ActionConsumeOrExpelLeft,
291+ "consume_or_expel_right": ActionConsumeOrExpelRight,
292+ "consume_or_expel_up": ActionConsumeOrExpelUp,
293+ "consume_or_expel_down": ActionConsumeOrExpelDown,
294+ }
295+
296+ for actionName, actionBinding := range cfg.Workspace.Shortcuts.Actions {
297+ action, ok := actionMap[actionName]
298+ if !ok {
299+ continue
300+ }
301+ for _, keyStr := range actionBinding.Keys {
302+ binding, ok := ParseKeyString(keyStr)
303+ if !ok {
304+ continue
305+ }
306+ h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action)
307+ }
308+ }
309+
310+ occupied := make(map[KeyBinding]Action, len(h.registered))
311+ for binding, action := range h.registered {
312+ occupied[binding] = action
313+ }
314+ for _, shortcut := range collectFloatingProfileShortcuts(ctx, cfg, occupied) {
315+ if h.registerShortcut(shortcut.Binding.Keyval, gdk.ModifierType(shortcut.Binding.Modifiers), shortcut.Action) {
316+ if url, ok := ParseFloatingProfileAction(shortcut.Action); ok {
317+ log.Trace().Str("shortcut", formatBinding(shortcut.Binding)).Str("url", url).Msg("registered floating profile global shortcut")
318+ }
319+ }
320+ }
321+
322+ shortcuts := NewShortcutSet(ctx, cfg)
323+ for binding, action := range shortcuts.Global {
324+ if _, exists := h.registered[binding]; exists {
325+ continue
326+ }
327+ h.registerShortcut(binding.Keyval, gdk.ModifierType(binding.Modifiers), action)
328+ }
329+ }
330+
331+ // Attach new controller to window
332+ h.window.AddController(&h.controller.EventController)
333+
334+ log.Debug().
335+ Int("shortcuts", len(h.callbacks)).
336+ Msg("global shortcuts reloaded")
337+}
338+
247 // Detach removes the global shortcut handler from the window.339 // Detach removes the global shortcut handler from the window.
248 // Note: GTK handles cleanup when the widget is destroyed,340 // Note: GTK handles cleanup when the widget is destroyed,
249 // but we clear our references here.341 // but we clear our references here.
M internal/ui/input/modal.go +4 -0
@@ -94,6 +94,9 @@ func NewModalState(ctx context.Context) *ModalState {
9494 func (m *ModalState) SetMainThreadScheduler(fn func(func())) {
9595 m.mu.Lock()
9696 defer m.mu.Unlock()
97+ if fn == nil {
98+ return
99+ }
97100 m.scheduleOnMainThread = fn
98101 }
99102
@@ -208,6 +211,7 @@ func (m *ModalState) cancelTimerLocked() {
208211 if m.timer != nil {
209212 m.timer.Stop()
210213 m.timer = nil
214+ m.timerGen++
211215 }
212216 }
213217
@@ -94,6 +94,9 @@ func NewModalState(ctx context.Context) *ModalState {
94 func (m *ModalState) SetMainThreadScheduler(fn func(func())) {94 func (m *ModalState) SetMainThreadScheduler(fn func(func())) {
95 m.mu.Lock()95 m.mu.Lock()
96 defer m.mu.Unlock()96 defer m.mu.Unlock()
97+ if fn == nil {
98+ return
99+ }
97 m.scheduleOnMainThread = fn100 m.scheduleOnMainThread = fn
98 }101 }
99 102
@@ -208,6 +211,7 @@ func (m *ModalState) cancelTimerLocked() {
208 if m.timer != nil {211 if m.timer != nil {
209 m.timer.Stop()212 m.timer.Stop()
210 m.timer = nil213 m.timer = nil
214+ m.timerGen++
211 }215 }
212 }216 }
213 217

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