fix(core): remove an empty first workspace once left (#133)
* fix(core): remove an empty first workspace once left normalize kept workspace 1 even when empty and not on screen, so the first non-empty workspace never moved up. It now follows the same rule as the others: an empty unnamed workspace is dropped once it is not active. * test(core): cover an empty first workspace under a named workspace
- commit
- ae511380d2e2f5313b29e10cff85d060a281ce8c
- author
- bnema · · 2026-10-04 16:53 UTC
- committer
- GitHub · · 2026-10-04 16:53 UTC
- parent
- 073f55f
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
2 files · +52 -12
M internal/core/monitor.go +4 -5
| @@ -7,8 +7,8 @@ import ( | |||
| 7 | 7 | ||
| 8 | 8 | // Monitor owns an ordered list of workspaces for one output (ADR 011). Numbers | |
| 9 | 9 | // are positions: Cmd+N targets Workspaces[N-1]. An empty workspace always sits | |
| 10 | - | // below the last one, and an empty unnamed workspace is removed once left, | |
| 11 | - | // except the first. Output-wide settings apply to every workspace. | |
| 10 | + | // below the last one, and an empty unnamed workspace is removed once left. | |
| 11 | + | // Output-wide settings apply to every workspace. | |
| 12 | 12 | // | |
| 13 | 13 | // Named workspaces come from config and are never removed while configured. | |
| 14 | 14 | // Named workspaces are outside normal numbered up/down navigation. Their | |
| @@ -167,11 +167,10 @@ func (m *Monitor) newWorkspace() *Workspace { | |||
| 167 | 167 | } | |
| 168 | 168 | ||
| 169 | 169 | // normalize keeps one trailing empty unnamed workspace and drops other empty | |
| 170 | - | // unnamed ones that are neither the first nor active. The active workspace | |
| 171 | - | // never changes. | |
| 170 | + | // unnamed ones that are not active. The active workspace never changes. | |
| 172 | 171 | func (m *Monitor) normalize() { | |
| 173 | 172 | spare := func(i int) bool { return m.Workspaces[i].empty() && m.Workspaces[i].Name == "" } | |
| 174 | - | for i := len(m.Workspaces) - 2; i >= 1; i-- { | |
| 173 | + | for i := len(m.Workspaces) - 2; i >= 0; i-- { | |
| 175 | 174 | if spare(i) && i != m.Active { | |
| 176 | 175 | m.Workspaces = append(m.Workspaces[:i], m.Workspaces[i+1:]...) | |
| 177 | 176 | if i < m.Active { | |
| @@ -7,8 +7,8 @@ import ( | |||||
| 7 | 7 | ||||
| 8 | // Monitor owns an ordered list of workspaces for one output (ADR 011). Numbers | 8 | // Monitor owns an ordered list of workspaces for one output (ADR 011). Numbers | ||
| 9 | // are positions: Cmd+N targets Workspaces[N-1]. An empty workspace always sits | 9 | // are positions: Cmd+N targets Workspaces[N-1]. An empty workspace always sits | ||
| 10 | - | // below the last one, and an empty unnamed workspace is removed once left, | 10 | + | // below the last one, and an empty unnamed workspace is removed once left. |
| 11 | - | // except the first. Output-wide settings apply to every workspace. | 11 | + | // Output-wide settings apply to every workspace. |
| 12 | // | 12 | // | ||
| 13 | // Named workspaces come from config and are never removed while configured. | 13 | // Named workspaces come from config and are never removed while configured. | ||
| 14 | // Named workspaces are outside normal numbered up/down navigation. Their | 14 | // Named workspaces are outside normal numbered up/down navigation. Their | ||
| @@ -167,11 +167,10 @@ func (m *Monitor) newWorkspace() *Workspace { | |||||
| 167 | } | 167 | } | ||
| 168 | 168 | ||||
| 169 | // normalize keeps one trailing empty unnamed workspace and drops other empty | 169 | // normalize keeps one trailing empty unnamed workspace and drops other empty | ||
| 170 | - | // unnamed ones that are neither the first nor active. The active workspace | 170 | + | // unnamed ones that are not active. The active workspace never changes. |
| 171 | - | // never changes. | |||
| 172 | func (m *Monitor) normalize() { | 171 | func (m *Monitor) normalize() { | ||
| 173 | spare := func(i int) bool { return m.Workspaces[i].empty() && m.Workspaces[i].Name == "" } | 172 | spare := func(i int) bool { return m.Workspaces[i].empty() && m.Workspaces[i].Name == "" } | ||
| 174 | - | for i := len(m.Workspaces) - 2; i >= 1; i-- { | 173 | + | for i := len(m.Workspaces) - 2; i >= 0; i-- { |
| 175 | if spare(i) && i != m.Active { | 174 | if spare(i) && i != m.Active { | ||
| 176 | m.Workspaces = append(m.Workspaces[:i], m.Workspaces[i+1:]...) | 175 | m.Workspaces = append(m.Workspaces[:i], m.Workspaces[i+1:]...) | ||
| 177 | if i < m.Active { | 176 | if i < m.Active { | ||
M internal/core/monitor_test.go +48 -7
| @@ -72,23 +72,41 @@ func TestMonitorRemovesEmptyWorkspaceOnceLeft(t *testing.T) { | |||
| 72 | 72 | } | |
| 73 | 73 | } | |
| 74 | 74 | ||
| 75 | - | func TestMonitorFirstWorkspaceStays(t *testing.T) { | |
| 75 | + | func TestMonitorEmptyFirstWorkspaceRemovedOnceLeft(t *testing.T) { | |
| 76 | 76 | m := monitor() | |
| 77 | 77 | m.AddWindow(1) | |
| 78 | 78 | m.RemoveWindow(1) | |
| 79 | 79 | m.FocusNumber(2) | |
| 80 | + | // The only workspace stays. | |
| 80 | 81 | if len(m.Workspaces) != 1 || m.Active != 0 { | |
| 81 | 82 | t.Fatal(windows(m), m.Active) | |
| 82 | 83 | } | |
| 83 | - | // Leaving an empty first workspace does not remove it. | |
| 84 | 84 | m.AddWindow(1) | |
| 85 | 85 | m.FocusNumber(2) | |
| 86 | 86 | m.AddWindow(2) | |
| 87 | 87 | m.FocusNumber(1) | |
| 88 | 88 | m.RemoveWindow(1) | |
| 89 | + | // Still on the now-empty workspace 1: it stays while active. | |
| 90 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) || m.Active != 0 { | |
| 91 | + | t.Fatal(windows(m), m.Active) | |
| 92 | + | } | |
| 93 | + | // Once left it is removed: workspace 2 becomes 1. | |
| 89 | 94 | m.FocusNumber(2) | |
| 90 | - | if !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) { | |
| 91 | - | t.Fatal(windows(m)) | |
| 95 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {}}) || m.Active != 0 { | |
| 96 | + | t.Fatal(windows(m), m.Active) | |
| 97 | + | } | |
| 98 | + | } | |
| 99 | + | ||
| 100 | + | // A window closing elsewhere empties workspace 1: the first non-empty | |
| 101 | + | // workspace is promoted at once and the view stays on its windows. | |
| 102 | + | func TestMonitorEmptyFirstWorkspaceRemovedFromElsewhere(t *testing.T) { | |
| 103 | + | m := monitor() | |
| 104 | + | m.AddWindow(1) | |
| 105 | + | m.FocusNumber(2) | |
| 106 | + | m.AddWindow(2) | |
| 107 | + | m.RemoveWindow(1) | |
| 108 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {}}) || m.Active != 0 { | |
| 109 | + | t.Fatal(windows(m), m.Active) | |
| 92 | 110 | } | |
| 93 | 111 | } | |
| 94 | 112 | ||
| @@ -153,9 +171,9 @@ func TestMonitorActions(t *testing.T) { | |||
| 153 | 171 | if id, _ := m.Focused(); id != 1 { | |
| 154 | 172 | t.Fatal(id) | |
| 155 | 173 | } | |
| 156 | - | // Workspace 1 is empty but first: it stays. | |
| 157 | - | if len(m.Workspaces) != 4 { | |
| 158 | - | t.Fatal(windows(m)) | |
| 174 | + | // The empty workspace 1 is removed once left. | |
| 175 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {1}, {}}) || m.Active != 1 { | |
| 176 | + | t.Fatal(windows(m), m.Active) | |
| 159 | 177 | } | |
| 160 | 178 | if e := m.Apply("spawn foot"); !e.Spawn { | |
| 161 | 179 | t.Fatal(e) | |
| @@ -291,6 +309,29 @@ func named(m *Monitor, specs ...NamedWorkspace) *Monitor { | |||
| 291 | 309 | return m | |
| 292 | 310 | } | |
| 293 | 311 | ||
| 312 | + | // An empty workspace 1 stays while a named workspace is shown over it, so | |
| 313 | + | // the toggle can return to it; leaving it for another number drops it. | |
| 314 | + | func TestEmptyFirstWorkspaceUnderNamed(t *testing.T) { | |
| 315 | + | m := named(monitor(), NamedWorkspace{Name: "dev"}) | |
| 316 | + | m.AddWindow(1) | |
| 317 | + | m.FocusNumber(2) | |
| 318 | + | m.AddWindow(2) | |
| 319 | + | m.FocusNumber(1) | |
| 320 | + | m.RemoveWindow(1) | |
| 321 | + | m.Apply("workspace dev") | |
| 322 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) || m.Current().Name != "dev" { | |
| 323 | + | t.Fatal(windows(m), m.Current().Name) | |
| 324 | + | } | |
| 325 | + | m.Apply("workspace dev") | |
| 326 | + | if m.Current() != m.Workspaces[0] || !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) { | |
| 327 | + | t.Fatal(windows(m), m.Active) | |
| 328 | + | } | |
| 329 | + | m.FocusNumber(2) | |
| 330 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {}}) || m.Active != 0 { | |
| 331 | + | t.Fatal(windows(m), m.Active) | |
| 332 | + | } | |
| 333 | + | } | |
| 334 | + | ||
| 294 | 335 | func TestHiddenWorkspaceToggle(t *testing.T) { | |
| 295 | 336 | m := named(monitor(), NamedWorkspace{Name: "dev"}) | |
| 296 | 337 | m.AddWindow(1) | |
| @@ -72,23 +72,41 @@ func TestMonitorRemovesEmptyWorkspaceOnceLeft(t *testing.T) { | |||||
| 72 | } | 72 | } | ||
| 73 | } | 73 | } | ||
| 74 | 74 | ||||
| 75 | - | func TestMonitorFirstWorkspaceStays(t *testing.T) { | 75 | + | func TestMonitorEmptyFirstWorkspaceRemovedOnceLeft(t *testing.T) { |
| 76 | m := monitor() | 76 | m := monitor() | ||
| 77 | m.AddWindow(1) | 77 | m.AddWindow(1) | ||
| 78 | m.RemoveWindow(1) | 78 | m.RemoveWindow(1) | ||
| 79 | m.FocusNumber(2) | 79 | m.FocusNumber(2) | ||
| 80 | + | // The only workspace stays. | |||
| 80 | if len(m.Workspaces) != 1 || m.Active != 0 { | 81 | if len(m.Workspaces) != 1 || m.Active != 0 { | ||
| 81 | t.Fatal(windows(m), m.Active) | 82 | t.Fatal(windows(m), m.Active) | ||
| 82 | } | 83 | } | ||
| 83 | - | // Leaving an empty first workspace does not remove it. | |||
| 84 | m.AddWindow(1) | 84 | m.AddWindow(1) | ||
| 85 | m.FocusNumber(2) | 85 | m.FocusNumber(2) | ||
| 86 | m.AddWindow(2) | 86 | m.AddWindow(2) | ||
| 87 | m.FocusNumber(1) | 87 | m.FocusNumber(1) | ||
| 88 | m.RemoveWindow(1) | 88 | m.RemoveWindow(1) | ||
| 89 | + | // Still on the now-empty workspace 1: it stays while active. | |||
| 90 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) || m.Active != 0 { | |||
| 91 | + | t.Fatal(windows(m), m.Active) | |||
| 92 | + | } | |||
| 93 | + | // Once left it is removed: workspace 2 becomes 1. | |||
| 89 | m.FocusNumber(2) | 94 | m.FocusNumber(2) | ||
| 90 | - | if !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) { | 95 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {}}) || m.Active != 0 { |
| 91 | - | t.Fatal(windows(m)) | 96 | + | t.Fatal(windows(m), m.Active) |
| 97 | + | } | |||
| 98 | + | } | |||
| 99 | + | ||||
| 100 | + | // A window closing elsewhere empties workspace 1: the first non-empty | |||
| 101 | + | // workspace is promoted at once and the view stays on its windows. | |||
| 102 | + | func TestMonitorEmptyFirstWorkspaceRemovedFromElsewhere(t *testing.T) { | |||
| 103 | + | m := monitor() | |||
| 104 | + | m.AddWindow(1) | |||
| 105 | + | m.FocusNumber(2) | |||
| 106 | + | m.AddWindow(2) | |||
| 107 | + | m.RemoveWindow(1) | |||
| 108 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {}}) || m.Active != 0 { | |||
| 109 | + | t.Fatal(windows(m), m.Active) | |||
| 92 | } | 110 | } | ||
| 93 | } | 111 | } | ||
| 94 | 112 | ||||
| @@ -153,9 +171,9 @@ func TestMonitorActions(t *testing.T) { | |||||
| 153 | if id, _ := m.Focused(); id != 1 { | 171 | if id, _ := m.Focused(); id != 1 { | ||
| 154 | t.Fatal(id) | 172 | t.Fatal(id) | ||
| 155 | } | 173 | } | ||
| 156 | - | // Workspace 1 is empty but first: it stays. | 174 | + | // The empty workspace 1 is removed once left. |
| 157 | - | if len(m.Workspaces) != 4 { | 175 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {1}, {}}) || m.Active != 1 { |
| 158 | - | t.Fatal(windows(m)) | 176 | + | t.Fatal(windows(m), m.Active) |
| 159 | } | 177 | } | ||
| 160 | if e := m.Apply("spawn foot"); !e.Spawn { | 178 | if e := m.Apply("spawn foot"); !e.Spawn { | ||
| 161 | t.Fatal(e) | 179 | t.Fatal(e) | ||
| @@ -291,6 +309,29 @@ func named(m *Monitor, specs ...NamedWorkspace) *Monitor { | |||||
| 291 | return m | 309 | return m | ||
| 292 | } | 310 | } | ||
| 293 | 311 | ||||
| 312 | + | // An empty workspace 1 stays while a named workspace is shown over it, so | |||
| 313 | + | // the toggle can return to it; leaving it for another number drops it. | |||
| 314 | + | func TestEmptyFirstWorkspaceUnderNamed(t *testing.T) { | |||
| 315 | + | m := named(monitor(), NamedWorkspace{Name: "dev"}) | |||
| 316 | + | m.AddWindow(1) | |||
| 317 | + | m.FocusNumber(2) | |||
| 318 | + | m.AddWindow(2) | |||
| 319 | + | m.FocusNumber(1) | |||
| 320 | + | m.RemoveWindow(1) | |||
| 321 | + | m.Apply("workspace dev") | |||
| 322 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) || m.Current().Name != "dev" { | |||
| 323 | + | t.Fatal(windows(m), m.Current().Name) | |||
| 324 | + | } | |||
| 325 | + | m.Apply("workspace dev") | |||
| 326 | + | if m.Current() != m.Workspaces[0] || !reflect.DeepEqual(windows(m), [][]WindowID{{}, {2}, {}}) { | |||
| 327 | + | t.Fatal(windows(m), m.Active) | |||
| 328 | + | } | |||
| 329 | + | m.FocusNumber(2) | |||
| 330 | + | if !reflect.DeepEqual(windows(m), [][]WindowID{{2}, {}}) || m.Active != 0 { | |||
| 331 | + | t.Fatal(windows(m), m.Active) | |||
| 332 | + | } | |||
| 333 | + | } | |||
| 334 | + | ||||
| 294 | func TestHiddenWorkspaceToggle(t *testing.T) { | 335 | func TestHiddenWorkspaceToggle(t *testing.T) { | ||
| 295 | m := named(monitor(), NamedWorkspace{Name: "dev"}) | 336 | m := named(monitor(), NamedWorkspace{Name: "dev"}) | ||
| 296 | m.AddWindow(1) | 337 | m.AddWindow(1) | ||