detached 7fd7cd4

fix(omnibox): address review follow-ups

Keep standalone argv handling and favorite indicator updates aligned with the current UI behavior, and make detached process release failures non-fatal.
commit
7fd7cd45d1edd3af4d298ff12c5d79ca38ed9614
author
brice · · 2026-04-07 18:58 UTC
parent
a25cbb9
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
7 files · +114 -8
Unified
UnifiedSplit
M internal/infrastructure/desktop/adapter.go +4 -1
@@ -432,7 +432,10 @@ func startDetachedProcess(cmd *exec.Cmd) error {
432432 }
433433 if cmd.Process != nil {
434434 if err := cmd.Process.Release(); err != nil {
435- return err
435+ logging.FromContext(context.Background()).Warn().
436+ Err(err).
437+ Str("path", cmd.Path).
438+ Msg("startDetachedProcess: cmd.Process.Release failed after start (non-fatal)")
436439 }
437440 }
438441
@@ -432,7 +432,10 @@ func startDetachedProcess(cmd *exec.Cmd) error {
432 }432 }
433 if cmd.Process != nil {433 if cmd.Process != nil {
434 if err := cmd.Process.Release(); err != nil {434 if err := cmd.Process.Release(); err != nil {
435- return err435+ logging.FromContext(context.Background()).Warn().
436+ Err(err).
437+ Str("path", cmd.Path).
438+ Msg("startDetachedProcess: cmd.Process.Release failed after start (non-fatal)")
436 }439 }
437 }440 }
438 441
M internal/ui/component/omnibox.go +67 -5
@@ -327,6 +327,10 @@ func resultsContainerState(rowCount int) (visible, expand, listVisible bool) {
327327 return rowCount > 0, rowCount > 0, rowCount > 0
328328 }
329329
330+func shouldApplyEmptyResultsState(rowCount int, visible bool) bool {
331+ return !visible || rowCount <= 0
332+}
333+
330334 func (o *Omnibox) setResultsContainerState(rowCount int) {
331335 if o.scrolledWin == nil {
332336 return
@@ -338,7 +342,11 @@ func (o *Omnibox) setResultsContainerState(rowCount int) {
338342 o.listBox.SetVisible(listVisible)
339343 }
340344 if o.mainBox != nil {
341- o.mainBox.RemoveCssClass("omnibox-empty")
345+ if shouldApplyEmptyResultsState(rowCount, visible) {
346+ o.mainBox.AddCssClass("omnibox-empty")
347+ } else {
348+ o.mainBox.RemoveCssClass("omnibox-empty")
349+ }
342350 }
343351 }
344352
@@ -1771,11 +1779,52 @@ func (o *Omnibox) createFaviconImage(rawURL, fallbackIcon string) *gtk.Image {
17711779 }
17721780
17731781 const favoriteStarBaseSize = 18
1782+const favoriteStarSlotClass = "omnibox-favorite-star-slot"
17741783
17751784 func favoriteStarSize(scale float64) int { return ScaleValue(favoriteStarBaseSize, scale) }
17761785
17771786 func shouldShowFavoriteStar(s Suggestion) bool { return s.IsFavorite }
17781787
1788+func clearBoxChildren(box *gtk.Box) {
1789+ if box == nil {
1790+ return
1791+ }
1792+ for child := box.GetFirstChild(); child != nil; {
1793+ next := child.GetNextSibling()
1794+ box.Remove(child)
1795+ child = next
1796+ }
1797+}
1798+
1799+func (o *Omnibox) syncFavoriteStarSlot(slot *gtk.Box, showFavoriteStar bool) {
1800+ if slot == nil {
1801+ return
1802+ }
1803+ clearBoxChildren(slot)
1804+ if showFavoriteStar {
1805+ if star := o.createFavoriteStarIcon(); star != nil {
1806+ slot.Append(&star.Widget)
1807+ }
1808+ }
1809+ slot.SetVisible(showFavoriteStar)
1810+}
1811+
1812+func favoriteStarSlotForRow(row *gtk.ListBoxRow) *gtk.Box {
1813+ if row == nil {
1814+ return nil
1815+ }
1816+ hbox := row.GetChild()
1817+ if hbox == nil {
1818+ return nil
1819+ }
1820+ for child := hbox.GetFirstChild(); child != nil; child = child.GetNextSibling() {
1821+ if child.HasCssClass(favoriteStarSlotClass) {
1822+ return gtk.BoxNewFromInternalPtr(child.GoPointer())
1823+ }
1824+ }
1825+ return nil
1826+}
1827+
17791828 func (o *Omnibox) createFavoriteStarIcon() *gtk.Image {
17801829 star := gtk.NewImage()
17811830 if star == nil {
@@ -1817,10 +1866,12 @@ func (o *Omnibox) appendSuggestionTitleAndURL(textBox *gtk.Box, title, displayUR
18171866 }
18181867
18191868 func (o *Omnibox) appendFavoriteStarAndShortcut(hbox *gtk.Box, showFavoriteStar bool, index int) {
1820- if showFavoriteStar {
1821- if star := o.createFavoriteStarIcon(); star != nil {
1822- hbox.Append(&star.Widget)
1823- }
1869+ starSlot := gtk.NewBox(gtk.OrientationHorizontalValue, 0)
1870+ if starSlot != nil {
1871+ starSlot.AddCssClass(favoriteStarSlotClass)
1872+ starSlot.SetValign(gtk.AlignCenterValue)
1873+ o.syncFavoriteStarSlot(starSlot, showFavoriteStar)
1874+ hbox.Append(&starSlot.Widget)
18241875 }
18251876
18261877 const maxShortcutIndex = 9
@@ -2256,6 +2307,17 @@ func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) {
22562307 } else {
22572308 row.RemoveCssClass("omnibox-row-favorite")
22582309 }
2310+ if slot := favoriteStarSlotForRow(row); slot != nil {
2311+ o.syncFavoriteStarSlot(slot, isFavorite)
2312+ slot.QueueDraw()
2313+ slot.Show()
2314+ }
2315+ if child := row.GetChild(); child != nil {
2316+ child.QueueDraw()
2317+ child.Show()
2318+ }
2319+ row.QueueDraw()
2320+ row.Show()
22592321 }
22602322
22612323 // yankSelectedURL copies the URL of the selected item to clipboard.
@@ -327,6 +327,10 @@ func resultsContainerState(rowCount int) (visible, expand, listVisible bool) {
327 return rowCount > 0, rowCount > 0, rowCount > 0327 return rowCount > 0, rowCount > 0, rowCount > 0
328 }328 }
329 329
330+func shouldApplyEmptyResultsState(rowCount int, visible bool) bool {
331+ return !visible || rowCount <= 0
332+}
333+
330 func (o *Omnibox) setResultsContainerState(rowCount int) {334 func (o *Omnibox) setResultsContainerState(rowCount int) {
331 if o.scrolledWin == nil {335 if o.scrolledWin == nil {
332 return336 return
@@ -338,7 +342,11 @@ func (o *Omnibox) setResultsContainerState(rowCount int) {
338 o.listBox.SetVisible(listVisible)342 o.listBox.SetVisible(listVisible)
339 }343 }
340 if o.mainBox != nil {344 if o.mainBox != nil {
341- o.mainBox.RemoveCssClass("omnibox-empty")345+ if shouldApplyEmptyResultsState(rowCount, visible) {
346+ o.mainBox.AddCssClass("omnibox-empty")
347+ } else {
348+ o.mainBox.RemoveCssClass("omnibox-empty")
349+ }
342 }350 }
343 }351 }
344 352
@@ -1771,11 +1779,52 @@ func (o *Omnibox) createFaviconImage(rawURL, fallbackIcon string) *gtk.Image {
1771 }1779 }
1772 1780
1773 const favoriteStarBaseSize = 181781 const favoriteStarBaseSize = 18
1782+const favoriteStarSlotClass = "omnibox-favorite-star-slot"
1774 1783
1775 func favoriteStarSize(scale float64) int { return ScaleValue(favoriteStarBaseSize, scale) }1784 func favoriteStarSize(scale float64) int { return ScaleValue(favoriteStarBaseSize, scale) }
1776 1785
1777 func shouldShowFavoriteStar(s Suggestion) bool { return s.IsFavorite }1786 func shouldShowFavoriteStar(s Suggestion) bool { return s.IsFavorite }
1778 1787
1788+func clearBoxChildren(box *gtk.Box) {
1789+ if box == nil {
1790+ return
1791+ }
1792+ for child := box.GetFirstChild(); child != nil; {
1793+ next := child.GetNextSibling()
1794+ box.Remove(child)
1795+ child = next
1796+ }
1797+}
1798+
1799+func (o *Omnibox) syncFavoriteStarSlot(slot *gtk.Box, showFavoriteStar bool) {
1800+ if slot == nil {
1801+ return
1802+ }
1803+ clearBoxChildren(slot)
1804+ if showFavoriteStar {
1805+ if star := o.createFavoriteStarIcon(); star != nil {
1806+ slot.Append(&star.Widget)
1807+ }
1808+ }
1809+ slot.SetVisible(showFavoriteStar)
1810+}
1811+
1812+func favoriteStarSlotForRow(row *gtk.ListBoxRow) *gtk.Box {
1813+ if row == nil {
1814+ return nil
1815+ }
1816+ hbox := row.GetChild()
1817+ if hbox == nil {
1818+ return nil
1819+ }
1820+ for child := hbox.GetFirstChild(); child != nil; child = child.GetNextSibling() {
1821+ if child.HasCssClass(favoriteStarSlotClass) {
1822+ return gtk.BoxNewFromInternalPtr(child.GoPointer())
1823+ }
1824+ }
1825+ return nil
1826+}
1827+
1779 func (o *Omnibox) createFavoriteStarIcon() *gtk.Image {1828 func (o *Omnibox) createFavoriteStarIcon() *gtk.Image {
1780 star := gtk.NewImage()1829 star := gtk.NewImage()
1781 if star == nil {1830 if star == nil {
@@ -1817,10 +1866,12 @@ func (o *Omnibox) appendSuggestionTitleAndURL(textBox *gtk.Box, title, displayUR
1817 }1866 }
1818 1867
1819 func (o *Omnibox) appendFavoriteStarAndShortcut(hbox *gtk.Box, showFavoriteStar bool, index int) {1868 func (o *Omnibox) appendFavoriteStarAndShortcut(hbox *gtk.Box, showFavoriteStar bool, index int) {
1820- if showFavoriteStar {1869+ starSlot := gtk.NewBox(gtk.OrientationHorizontalValue, 0)
1821- if star := o.createFavoriteStarIcon(); star != nil {1870+ if starSlot != nil {
1822- hbox.Append(&star.Widget)1871+ starSlot.AddCssClass(favoriteStarSlotClass)
1823- }1872+ starSlot.SetValign(gtk.AlignCenterValue)
1873+ o.syncFavoriteStarSlot(starSlot, showFavoriteStar)
1874+ hbox.Append(&starSlot.Widget)
1824 }1875 }
1825 1876
1826 const maxShortcutIndex = 91877 const maxShortcutIndex = 9
@@ -2256,6 +2307,17 @@ func (o *Omnibox) updateRowFavoriteIndicator(index int, isFavorite bool) {
2256 } else {2307 } else {
2257 row.RemoveCssClass("omnibox-row-favorite")2308 row.RemoveCssClass("omnibox-row-favorite")
2258 }2309 }
2310+ if slot := favoriteStarSlotForRow(row); slot != nil {
2311+ o.syncFavoriteStarSlot(slot, isFavorite)
2312+ slot.QueueDraw()
2313+ slot.Show()
2314+ }
2315+ if child := row.GetChild(); child != nil {
2316+ child.QueueDraw()
2317+ child.Show()
2318+ }
2319+ row.QueueDraw()
2320+ row.Show()
2259 }2321 }
2260 2322
2261 // yankSelectedURL copies the URL of the selected item to clipboard.2323 // yankSelectedURL copies the URL of the selected item to clipboard.
M internal/ui/component/omnibox_favorite_icon_test.go +19 -0
@@ -15,3 +15,22 @@ func TestOmniboxFavoriteStarVisibility(t *testing.T) {
1515 assert.True(t, shouldShowFavoriteStar(Suggestion{IsFavorite: true}))
1616 assert.False(t, shouldShowFavoriteStar(Suggestion{IsFavorite: false}))
1717 }
18+
19+func TestShouldApplyEmptyResultsState(t *testing.T) {
20+ tests := []struct {
21+ name string
22+ rowCount int
23+ visible bool
24+ want bool
25+ }{
26+ {name: "negative rows uses empty state", rowCount: -1, visible: false, want: true},
27+ {name: "zero rows uses empty state", rowCount: 0, visible: false, want: true},
28+ {name: "positive rows clears empty state", rowCount: 2, visible: true, want: false},
29+ }
30+
31+ for _, tt := range tests {
32+ t.Run(tt.name, func(t *testing.T) {
33+ assert.Equal(t, tt.want, shouldApplyEmptyResultsState(tt.rowCount, tt.visible))
34+ })
35+ }
36+}
@@ -15,3 +15,22 @@ func TestOmniboxFavoriteStarVisibility(t *testing.T) {
15 assert.True(t, shouldShowFavoriteStar(Suggestion{IsFavorite: true}))15 assert.True(t, shouldShowFavoriteStar(Suggestion{IsFavorite: true}))
16 assert.False(t, shouldShowFavoriteStar(Suggestion{IsFavorite: false}))16 assert.False(t, shouldShowFavoriteStar(Suggestion{IsFavorite: false}))
17 }17 }
18+
19+func TestShouldApplyEmptyResultsState(t *testing.T) {
20+ tests := []struct {
21+ name string
22+ rowCount int
23+ visible bool
24+ want bool
25+ }{
26+ {name: "negative rows uses empty state", rowCount: -1, visible: false, want: true},
27+ {name: "zero rows uses empty state", rowCount: 0, visible: false, want: true},
28+ {name: "positive rows clears empty state", rowCount: 2, visible: true, want: false},
29+ }
30+
31+ for _, tt := range tests {
32+ t.Run(tt.name, func(t *testing.T) {
33+ assert.Equal(t, tt.want, shouldApplyEmptyResultsState(tt.rowCount, tt.visible))
34+ })
35+ }
36+}
M internal/ui/component/omnibox_navigation_state_test.go +1 -0
@@ -95,6 +95,7 @@ func TestResultsContainerState(t *testing.T) {
9595 wantExpand bool
9696 wantListVisible bool
9797 }{
98+ {name: "negative rows stays hidden", rowCount: -1, wantVisible: false, wantExpand: false, wantListVisible: false},
9899 {name: "no rows stays hidden", rowCount: 0, wantVisible: false, wantExpand: false, wantListVisible: false},
99100 {name: "rows show and expand", rowCount: 3, wantVisible: true, wantExpand: true, wantListVisible: true},
100101 }
@@ -95,6 +95,7 @@ func TestResultsContainerState(t *testing.T) {
95 wantExpand bool95 wantExpand bool
96 wantListVisible bool96 wantListVisible bool
97 }{97 }{
98+ {name: "negative rows stays hidden", rowCount: -1, wantVisible: false, wantExpand: false, wantListVisible: false},
98 {name: "no rows stays hidden", rowCount: 0, wantVisible: false, wantExpand: false, wantListVisible: false},99 {name: "no rows stays hidden", rowCount: 0, wantVisible: false, wantExpand: false, wantListVisible: false},
99 {name: "rows show and expand", rowCount: 3, wantVisible: true, wantExpand: true, wantListVisible: true},100 {name: "rows show and expand", rowCount: 3, wantVisible: true, wantExpand: true, wantListVisible: true},
100 }101 }
M internal/ui/standalone_omnibox.go +3 -1
@@ -84,9 +84,11 @@ func standaloneOmniboxArgv(args []string) []string {
8484 }
8585
8686 filtered := []string{args[0]}
87+ subcommandSkipped := false
8788 for i := 1; i < len(args); i++ {
8889 arg := args[i]
89- if arg == "omnibox" {
90+ if arg == "omnibox" && !subcommandSkipped {
91+ subcommandSkipped = true
9092 continue
9193 }
9294 if strings.HasPrefix(arg, "-test.") {
@@ -84,9 +84,11 @@ func standaloneOmniboxArgv(args []string) []string {
84 }84 }
85 85
86 filtered := []string{args[0]}86 filtered := []string{args[0]}
87+ subcommandSkipped := false
87 for i := 1; i < len(args); i++ {88 for i := 1; i < len(args); i++ {
88 arg := args[i]89 arg := args[i]
89- if arg == "omnibox" {90+ if arg == "omnibox" && !subcommandSkipped {
91+ subcommandSkipped = true
90 continue92 continue
91 }93 }
92 if strings.HasPrefix(arg, "-test.") {94 if strings.HasPrefix(arg, "-test.") {
M internal/ui/standalone_omnibox_test.go +17 -0
@@ -158,6 +158,23 @@ func TestStandaloneOmniboxArgv_RemovesGoTestFlags(t *testing.T) {
158158 }
159159 }
160160
161+func TestStandaloneOmniboxArgv_RemovesOnlyLeadingOmniboxSubcommand(t *testing.T) {
162+ argv := standaloneOmniboxArgv([]string{"/tmp/testbin", "omnibox", "https://example.com/omnibox", "omnibox"})
163+
164+ if len(argv) != 3 {
165+ t.Fatalf("expected 3 args, got %d: %#v", len(argv), argv)
166+ }
167+ if argv[0] != "/tmp/testbin" {
168+ t.Fatalf("expected binary path to be preserved, got %q", argv[0])
169+ }
170+ if argv[1] != "https://example.com/omnibox" {
171+ t.Fatalf("expected non-subcommand omnibox value to be preserved, got %#v", argv)
172+ }
173+ if argv[2] != "omnibox" {
174+ t.Fatalf("expected later omnibox arg to be preserved, got %#v", argv)
175+ }
176+}
177+
161178 func TestTryInitLayerShell_NilWindowReturnsFalse(t *testing.T) {
162179 if tryInitLayerShell(nil, DefaultStandaloneOmniboxWindowConfig()) {
163180 t.Fatalf("expected nil window to skip layer-shell")
@@ -158,6 +158,23 @@ func TestStandaloneOmniboxArgv_RemovesGoTestFlags(t *testing.T) {
158 }158 }
159 }159 }
160 160
161+func TestStandaloneOmniboxArgv_RemovesOnlyLeadingOmniboxSubcommand(t *testing.T) {
162+ argv := standaloneOmniboxArgv([]string{"/tmp/testbin", "omnibox", "https://example.com/omnibox", "omnibox"})
163+
164+ if len(argv) != 3 {
165+ t.Fatalf("expected 3 args, got %d: %#v", len(argv), argv)
166+ }
167+ if argv[0] != "/tmp/testbin" {
168+ t.Fatalf("expected binary path to be preserved, got %q", argv[0])
169+ }
170+ if argv[1] != "https://example.com/omnibox" {
171+ t.Fatalf("expected non-subcommand omnibox value to be preserved, got %#v", argv)
172+ }
173+ if argv[2] != "omnibox" {
174+ t.Fatalf("expected later omnibox arg to be preserved, got %#v", argv)
175+ }
176+}
177+
161 func TestTryInitLayerShell_NilWindowReturnsFalse(t *testing.T) {178 func TestTryInitLayerShell_NilWindowReturnsFalse(t *testing.T) {
162 if tryInitLayerShell(nil, DefaultStandaloneOmniboxWindowConfig()) {179 if tryInitLayerShell(nil, DefaultStandaloneOmniboxWindowConfig()) {
163 t.Fatalf("expected nil window to skip layer-shell")180 t.Fatalf("expected nil window to skip layer-shell")
M internal/ui/theme/css_test.go +3 -1
@@ -1,6 +1,7 @@
11 package theme
22
33 import (
4+ "regexp"
45 "strings"
56 "testing"
67 )
@@ -11,7 +12,8 @@ func TestGenerateCSS_StandaloneOmniboxWindowUsesTransparentBackground(t *testing
1112 if !strings.Contains(css, "window.standalone-omnibox-window") {
1213 t.Fatalf("expected standalone omnibox window selector in CSS, got %q", css)
1314 }
14- if !strings.Contains(css, "window.standalone-omnibox-window {\n\tbackground-color: transparent;") {
15+ re := regexp.MustCompile(`window\.standalone-omnibox-window\s*\{\s*background-color:\s*transparent\s*;?\s*\}`)
16+ if !re.MatchString(css) {
1517 t.Fatalf("expected standalone omnibox window background to be transparent")
1618 }
1719 }
@@ -1,6 +1,7 @@
1 package theme1 package theme
2 2
3 import (3 import (
4+ "regexp"
4 "strings"5 "strings"
5 "testing"6 "testing"
6 )7 )
@@ -11,7 +12,8 @@ func TestGenerateCSS_StandaloneOmniboxWindowUsesTransparentBackground(t *testing
11 if !strings.Contains(css, "window.standalone-omnibox-window") {12 if !strings.Contains(css, "window.standalone-omnibox-window") {
12 t.Fatalf("expected standalone omnibox window selector in CSS, got %q", css)13 t.Fatalf("expected standalone omnibox window selector in CSS, got %q", css)
13 }14 }
14- if !strings.Contains(css, "window.standalone-omnibox-window {\n\tbackground-color: transparent;") {15+ re := regexp.MustCompile(`window\.standalone-omnibox-window\s*\{\s*background-color:\s*transparent\s*;?\s*\}`)
16+ if !re.MatchString(css) {
15 t.Fatalf("expected standalone omnibox window background to be transparent")17 t.Fatalf("expected standalone omnibox window background to be transparent")
16 }18 }
17 }19 }

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