ac7ac23 fix(rules): skip remaps, keep covered layouts and reject anchor breakouts · neferwl

M internal/adapters/config/config.go +5 -4


@@ -1025,12 +1025,13 @@ func setRule(r *ports.WindowRule, field, v string) error {
1025	1025		if v == "" {
1026	1026		return fmt.Errorf("needs a regular expression")
1027	1027		}
1028		-	// The whole app ID must match: "steam" does not match "steam_app_1".
1029		-	re, err := regexp.Compile("^(?:" + v + ")$")
1030		-	if err != nil {
	1028	+	// Compiled alone first, so a value cannot close the anchoring
	1029	+	// group ("a)\|(b").
	1030	+	if _, err := regexp.Compile(v); err != nil {
1031	1031		return fmt.Errorf("invalid regular expression: %v", err)
1032	1032		}
1033		-	r.AppID = re
	1033	+	// The whole app ID must match: "steam" does not match "steam_app_1".
	1034	+	r.AppID = regexp.MustCompile("^(?:" + v + ")$")
1034	1035		case "floating":
1035	1036		b, err := onOff(v)
1036	1037		if err != nil {

 		if v == "" {
 			return fmt.Errorf("needs a regular expression")
 		}
-		// The whole app ID must match: "steam" does not match "steam_app_1".
+		// Compiled alone first, so a value cannot close the anchoring
-		re, err := regexp.Compile("^(?:" + v + ")$")
+		// group ("a)|(b").
-		if err != nil {
+		if _, err := regexp.Compile(v); err != nil {
 			return fmt.Errorf("invalid regular expression: %v", err)
 		}
-		r.AppID = re
+		// The whole app ID must match: "steam" does not match "steam_app_1".
+		r.AppID = regexp.MustCompile("^(?:" + v + ")$")
 	case "floating":
 		b, err := onOff(v)
 		if err != nil {

M internal/adapters/config/rules_test.go +2 -0


@@ -59,6 +59,8 @@ func TestRuleWarnings(t *testing.T) {
59	59		line, rules int
60	60		}{
61	61		{"bad regex", "rule.a.app-id = (\n", "rule.a.app-id: invalid regular expression", 1, 0},
	62	+	// Closing the anchoring group would match any app ID.
	63	+	{"anchor breakout", "rule.a.app-id = steam)\|(.*\n", "rule.a.app-id: invalid regular expression", 1, 0},
62	64		{"missing app-id", "rule.a.floating = on\n", "rule.a: no app-id, rule ignored", 1, 0},
63	65		{"unknown field", "rule.a.app-id = foo\nrule.a.title = x\n", "rule.a.title: unknown key", 2, 1},
64	66		{"bad floating", "rule.a.app-id = foo\nrule.a.floating = maybe\n", "rule.a.floating: must be on or off", 2, 0},


@@ -59,6 +59,8 @@ func TestRuleWarnings(t *testing.T) {
59	line, rules int	59		line, rules int
60	}{	60		}{
61	{"bad regex", "rule.a.app-id = (\n", "rule.a.app-id: invalid regular expression", 1, 0},	61		{"bad regex", "rule.a.app-id = (\n", "rule.a.app-id: invalid regular expression", 1, 0},
		62	+	// Closing the anchoring group would match any app ID.
		63	+	{"anchor breakout", "rule.a.app-id = steam)\|(.*\n", "rule.a.app-id: invalid regular expression", 1, 0},
62	{"missing app-id", "rule.a.floating = on\n", "rule.a: no app-id, rule ignored", 1, 0},	64		{"missing app-id", "rule.a.floating = on\n", "rule.a: no app-id, rule ignored", 1, 0},
63	{"unknown field", "rule.a.app-id = foo\nrule.a.title = x\n", "rule.a.title: unknown key", 2, 1},	65		{"unknown field", "rule.a.app-id = foo\nrule.a.title = x\n", "rule.a.title: unknown key", 2, 1},
64	{"bad floating", "rule.a.app-id = foo\nrule.a.floating = maybe\n", "rule.a.floating: must be on or off", 2, 0},	66		{"bad floating", "rule.a.app-id = foo\nrule.a.floating = maybe\n", "rule.a.floating: must be on or off", 2, 0},

M internal/adapters/wayland/foreigntoplevel_test.go +6 -2


@@ -157,8 +157,12 @@ func TestForeignToplevel(t *testing.T) {
157	157		registerWireProxy(c, xp)
158	158		requestProtocol(t, c, surf, wayland.SurfaceRequestCommit)
159	159		ackAndAttach(t, c, surf, xdg, shmBuffer(t, c), serials)
160		-	if again := mapped(t, events, 2*time.Second); again.ID != w.ID {
161		-	t.Fatalf("remapped as %d, want %d", again.ID, w.ID)
	160	+	// Core learns it is a remap: window rules only place a first map.
	161	+	if w.Remap {
	162	+	t.Fatal("first map flagged as a remap")
	163	+	}
	164	+	if again := mapped(t, events, 2*time.Second); again.ID != w.ID \|\| !again.Remap {
	165	+	t.Fatalf("remapped as %+v, want ID %d and Remap", again, w.ID)
162	166		}
163	167		if got := takeEvents(t, c, p.events); len(got) == 0 \|\| got[len(got)-1] != "done" {
164	168		t.Fatalf("remap = %q", got)

 	registerWireProxy(c, xp)
 	requestProtocol(t, c, surf, wayland.SurfaceRequestCommit)
 	ackAndAttach(t, c, surf, xdg, shmBuffer(t, c), serials)
-	if again := mapped(t, events, 2*time.Second); again.ID != w.ID {
+	// Core learns it is a remap: window rules only place a first map.
-		t.Fatalf("remapped as %d, want %d", again.ID, w.ID)
+	if w.Remap {
+		t.Fatal("first map flagged as a remap")
+	}
+	if again := mapped(t, events, 2*time.Second); again.ID != w.ID || !again.Remap {
+		t.Fatalf("remapped as %+v, want ID %d and Remap", again, w.ID)
 	}
 	if got := takeEvents(t, c, p.events); len(got) == 0 || got[len(got)-1] != "done" {
 		t.Fatalf("remap = %q", got)

M internal/adapters/wayland/xdg.go +1 -1


@@ -243,7 +243,7 @@ func (x xdgSurface) GetToplevel(r xdgshell.Surface, id uint32) {
243	243		if w.parent != nil && w.parent.mapped {
244	244		parent = w.parent.id
245	245		}
246		-	x.server.emit(ports.WindowMapped{ID: w.id, AppID: w.appID, Slot: slot, PID: r.Client().PID(), Floating: w.floating, Width: w.floatW, Height: w.floatH, Parent: parent})
	246	+	x.server.emit(ports.WindowMapped{ID: w.id, AppID: w.appID, Slot: slot, PID: r.Client().PID(), Floating: w.floating, Width: w.floatW, Height: w.floatH, Parent: parent, Remap: w.gen > 1})
247	247		x.server.syncInhibitors()
248	248		x.server.toplevelChanged(w)
249	249		x.server.log.Info().Uint64("id", uint64(w.id)).Str("app_id", w.appID).Str("slot", slot).Bool("floating", w.floating).Msg("window mapped")


@@ -243,7 +243,7 @@ func (x xdgSurface) GetToplevel(r xdgshell.Surface, id uint32) {
243		if w.parent != nil && w.parent.mapped {	243		if w.parent != nil && w.parent.mapped {
244		parent = w.parent.id	244		parent = w.parent.id
245		}	245		}
246	-	x.server.emit(ports.WindowMapped{ID: w.id, AppID: w.appID, Slot: slot, PID: r.Client().PID(), Floating: w.floating, Width: w.floatW, Height: w.floatH, Parent: parent})	246	+	x.server.emit(ports.WindowMapped{ID: w.id, AppID: w.appID, Slot: slot, PID: r.Client().PID(), Floating: w.floating, Width: w.floatW, Height: w.floatH, Parent: parent, Remap: w.gen > 1})
247		x.server.syncInhibitors()	247		x.server.syncInhibitors()
248		x.server.toplevelChanged(w)	248		x.server.toplevelChanged(w)
249		x.server.log.Info().Uint64("id", uint64(w.id)).Str("app_id", w.appID).Str("slot", slot).Bool("floating", w.floating).Msg("window mapped")	249		x.server.log.Info().Uint64("id", uint64(w.id)).Str("app_id", w.appID).Str("slot", slot).Bool("floating", w.floating).Msg("window mapped")

M internal/core/rules.go +17 -3


@@ -71,11 +71,16 @@ func parseRules(rs []ports.WindowRule) ([]rule, error) {
71	71		return out, nil
72	72		}
73	73
74		-	// ruleFor merges the rules matching appID; ok is false when none does.
75		-	func (c *Core) ruleFor(appID string) (eff ruleEffect, ok bool) {
	74	+	// ruleFor merges the rules matching v's app ID; ok is false when none does.
	75	+	// A toplevel mapping again after an unmap (a tray app restored) opens as
	76	+	// any other window: rules only place its first map.
	77	+	func (c *Core) ruleFor(v ports.WindowMapped) (eff ruleEffect, ok bool) {
	78	+	if v.Remap {
	79	+	return eff, false
	80	+	}
76	81		for i := range c.rules {
77	82		r := &c.rules[i]
78		-	if !r.appID.MatchString(appID) {
	83	+	if !r.appID.MatchString(v.AppID) {
79	84		continue
80	85		}
81	86		ok = true

@@ -178,6 +183,15 @@ func (w *Workspace) addFloat(f Float, quiet bool) {
178	183		// where it is (ADR 011 golden rule).
179	184		func (w *Workspace) addColumnQuiet(col Column) {
180	185		at := len(w.Columns)
	186	+	if w.cover() != 0 {
	187	+	// As AddWindow: under a covering fullscreen window the column
	188	+	// waits hidden, and a maximized layout stays as it is.
	189	+	if !w.policy().equalCells {
	190	+	at = min(w.Focus+1, at)
	191	+	}
	192	+	w.Columns = slices.Insert(w.Columns, at, col)
	193	+	return
	194	+	}
181	195		if w.policy().equalCells {
182	196		if at > 0 {
183	197		w.unmaximize()

 	return out, nil
 }
-// ruleFor merges the rules matching appID; ok is false when none does.
+// ruleFor merges the rules matching v's app ID; ok is false when none does.
-func (c *Core) ruleFor(appID string) (eff ruleEffect, ok bool) {
+// A toplevel mapping again after an unmap (a tray app restored) opens as
+// any other window: rules only place its first map.
+func (c *Core) ruleFor(v ports.WindowMapped) (eff ruleEffect, ok bool) {
+	if v.Remap {
+		return eff, false
+	}
 	for i := range c.rules {
 		r := &c.rules[i]
-		if !r.appID.MatchString(appID) {
+		if !r.appID.MatchString(v.AppID) {
 			continue
 		}
 		ok = true
 // where it is (ADR 011 golden rule).
 func (w *Workspace) addColumnQuiet(col Column) {
 	at := len(w.Columns)
+	if w.cover() != 0 {
+		// As AddWindow: under a covering fullscreen window the column
+		// waits hidden, and a maximized layout stays as it is.
+		if !w.policy().equalCells {
+			at = min(w.Focus+1, at)
+		}
+		w.Columns = slices.Insert(w.Columns, at, col)
+		return
+	}
 	if w.policy().equalCells {
 		if at > 0 {
 			w.unmaximize()

A internal/core/rules_internal_test.go +29 -0

@@ -0,0 +1,29 @@
	1	+	package core
	2	+
	3	+	import "testing"
	4	+
	5	+	// Under a covering fullscreen window, a rule's quiet column waits hidden:
	6	+	// focus, fullscreen and a fixed-overflow maximized column stay as they are,
	7	+	// as for any new window (AddWindow's cover branch).
	8	+	func TestAddColumnQuietUnderCover(t *testing.T) {
	9	+	w := workspace()
	10	+	w.Overflow = OverflowFixed
	11	+	w.AddWindow(1)
	12	+	w.AddWindow(2)
	13	+	w.FocusID(1)
	14	+	w.maximize(w.Focus)
	15	+	w.ToggleFullscreen()
	16	+	if w.cover() != 1 {
	17	+	t.Fatalf("setup: cover %d", w.cover())
	18	+	}
	19	+	w.addColumnQuiet(Column{Windows: []WindowID{3}})
	20	+	if !w.has(3) {
	21	+	t.Fatal("column not added")
	22	+	}
	23	+	if id, _ := w.Focused(); id != 1 \|\| w.fullscreen != 1 {
	24	+	t.Fatalf("focus %d fullscreen %d", id, w.fullscreen)
	25	+	}
	26	+	if i := w.columnOf(1); i < 0 \|\| !w.Columns[i].FullWidth \|\| len(w.maximized) == 0 \|\| w.maximized[0] != 1 {
	27	+	t.Fatalf("maximized layout lost: %+v %v", w.Columns, w.maximized)
	28	+	}
	29	+	}

@@ -0,0 +1,29 @@
	1	+	package core
	2	+
	3	+	import "testing"
	4	+
	5	+	// Under a covering fullscreen window, a rule's quiet column waits hidden:
	6	+	// focus, fullscreen and a fixed-overflow maximized column stay as they are,
	7	+	// as for any new window (AddWindow's cover branch).
	8	+	func TestAddColumnQuietUnderCover(t *testing.T) {
	9	+	w := workspace()
	10	+	w.Overflow = OverflowFixed
	11	+	w.AddWindow(1)
	12	+	w.AddWindow(2)
	13	+	w.FocusID(1)
	14	+	w.maximize(w.Focus)
	15	+	w.ToggleFullscreen()
	16	+	if w.cover() != 1 {
	17	+	t.Fatalf("setup: cover %d", w.cover())
	18	+	}
	19	+	w.addColumnQuiet(Column{Windows: []WindowID{3}})
	20	+	if !w.has(3) {
	21	+	t.Fatal("column not added")
	22	+	}
	23	+	if id, _ := w.Focused(); id != 1 \|\| w.fullscreen != 1 {
	24	+	t.Fatalf("focus %d fullscreen %d", id, w.fullscreen)
	25	+	}
	26	+	if i := w.columnOf(1); i < 0 \|\| !w.Columns[i].FullWidth \|\| len(w.maximized) == 0 \|\| w.maximized[0] != 1 {
	27	+	t.Fatalf("maximized layout lost: %+v %v", w.Columns, w.maximized)
	28	+	}
	29	+	}

M internal/core/rules_test.go +19 -0


@@ -338,6 +338,25 @@ func TestRemapKeepsPlace(t *testing.T) {
338	338		}
339	339		}
340	340
	341	+	// A toplevel that maps again after an unmap (a tray app restored) opens as
	342	+	// any other window: on the workspace the user looks at, not its rule's.
	343	+	func TestRuleSkipsRemapAfterUnmap(t *testing.T) {
	344	+	r := startMulti(t, func(c *ports.Config) {
	345	+	declareDev(c)
	346	+	c.Rules = []ports.WindowRule{winRule("dev", "discord", func(r *ports.WindowRule) { r.Workspace = "dev" })}
	347	+	}, left)
	348	+	if w := stWindow(r.mapApp(t, 1, "discord"), 1); w.WorkspaceName != "dev" {
	349	+	t.Fatalf("first map not placed by the rule: %+v", w)
	350	+	}
	351	+	r.client <- ports.WindowUnmapped{ID: 1}
	352	+	stateAfter(t, r.state, func(st ports.State) bool { return stWindow(st, 1) == nil })
	353	+	r.client <- ports.WindowMapped{ID: 1, AppID: "discord", Remap: true}
	354	+	st := r.stateWith(t, 1)
	355	+	if w := stWindow(st, 1); w.WorkspaceName != "" \|\| w.Workspace != 1 \|\| stFocus(st) != 1 {
	356	+	t.Fatalf("remap placed by the rule: %+v focus %d", w, stFocus(st))
	357	+	}
	358	+	}
	359	+
341	360		func TestRuleReloadAffectsFutureMapsOnly(t *testing.T) {
342	361		r := startMulti(t, func(c *ports.Config) {
343	362		declareDev(c)


@@ -338,6 +338,25 @@ func TestRemapKeepsPlace(t *testing.T) {
338	}	338		}
339	}	339		}
340		340
		341	+	// A toplevel that maps again after an unmap (a tray app restored) opens as
		342	+	// any other window: on the workspace the user looks at, not its rule's.
		343	+	func TestRuleSkipsRemapAfterUnmap(t *testing.T) {
		344	+	r := startMulti(t, func(c *ports.Config) {
		345	+	declareDev(c)
		346	+	c.Rules = []ports.WindowRule{winRule("dev", "discord", func(r *ports.WindowRule) { r.Workspace = "dev" })}
		347	+	}, left)
		348	+	if w := stWindow(r.mapApp(t, 1, "discord"), 1); w.WorkspaceName != "dev" {
		349	+	t.Fatalf("first map not placed by the rule: %+v", w)
		350	+	}
		351	+	r.client <- ports.WindowUnmapped{ID: 1}
		352	+	stateAfter(t, r.state, func(st ports.State) bool { return stWindow(st, 1) == nil })
		353	+	r.client <- ports.WindowMapped{ID: 1, AppID: "discord", Remap: true}
		354	+	st := r.stateWith(t, 1)
		355	+	if w := stWindow(st, 1); w.WorkspaceName != "" \|\| w.Workspace != 1 \|\| stFocus(st) != 1 {
		356	+	t.Fatalf("remap placed by the rule: %+v focus %d", w, stFocus(st))
		357	+	}
		358	+	}
		359	+
341	func TestRuleReloadAffectsFutureMapsOnly(t *testing.T) {	360		func TestRuleReloadAffectsFutureMapsOnly(t *testing.T) {
342	r := startMulti(t, func(c *ports.Config) {	361		r := startMulti(t, func(c *ports.Config) {
343	declareDev(c)	362		declareDev(c)

M internal/core/spawn_placement.go +2 -2


@@ -85,7 +85,7 @@ func (p spawnPlacement) place(c Core, v ports.WindowMapped) {
85	85		return
86	86		}
87	87		// Rules come after the dialog: it stays over its parent.
88		-	if eff, ok := c.ruleFor(v.AppID); ok {
	88	+	if eff, ok := c.ruleFor(v); ok {
89	89		c.placeByRule(v, eff)
90	90		return
91	91		}

@@ -115,7 +115,7 @@ func (p spawnPlacement) place(c Core, v ports.WindowMapped) {
115	115		}
116	116		}
117	117		if s, _ := c.screenOf(v.ID); s == nil {
118		-	if eff, ok := c.ruleFor(v.AppID); ok {
	118	+	if eff, ok := c.ruleFor(v); ok {
119	119		c.placeByRule(v, eff)
120	120		return
121	121		}


@@ -85,7 +85,7 @@ func (p spawnPlacement) place(c Core, v ports.WindowMapped) {
85		return	85		return
86		}	86		}
87		// Rules come after the dialog: it stays over its parent.	87		// Rules come after the dialog: it stays over its parent.
88	-	if eff, ok := c.ruleFor(v.AppID); ok {	88	+	if eff, ok := c.ruleFor(v); ok {
89		c.placeByRule(v, eff)	89		c.placeByRule(v, eff)
90		return	90		return
91		}	91		}

@@ -115,7 +115,7 @@ func (p spawnPlacement) place(c Core, v ports.WindowMapped) {
115		}	115		}
116		}	116		}
117		if s, _ := c.screenOf(v.ID); s == nil {	117		if s, _ := c.screenOf(v.ID); s == nil {
118	-	if eff, ok := c.ruleFor(v.AppID); ok {	118	+	if eff, ok := c.ruleFor(v); ok {
119		c.placeByRule(v, eff)	119		c.placeByRule(v, eff)
120		return	120		return
121		}	121		}

M internal/ports/client_events.go +3 -0


@@ -20,6 +20,9 @@ type WindowMapped struct {
20	20		// xdg-foreign), 0 when none. A dialog of a fullscreen window shows over
21	21		// it.
22	22		Parent WindowID
	23	+	// Remap is set when the toplevel mapped before and was unmapped (a tray
	24	+	// app restored): window rules only place its first map.
	25	+	Remap bool
23	26		}
24	27
25	28		// SlotEnv is the environment variable neferwl sets on processes it spawns


@@ -20,6 +20,9 @@ type WindowMapped struct {
20	// xdg-foreign), 0 when none. A dialog of a fullscreen window shows over	20		// xdg-foreign), 0 when none. A dialog of a fullscreen window shows over
21	// it.	21		// it.
22	Parent WindowID	22		Parent WindowID
		23	+	// Remap is set when the toplevel mapped before and was unmapped (a tray
		24	+	// app restored): window rules only place its first map.
		25	+	Remap bool
23	}	26		}
24		27
25	// SlotEnv is the environment variable neferwl sets on processes it spawns	28		// SlotEnv is the environment variable neferwl sets on processes it spawns