fix(window-restore): re-read workspace/window state after the wofi pick

The picker blocks on user input for however long they take. In that
window niri can destroy a dynamic workspace (its last window closed)
and renumber every later one down, so the pre-pick current["idx"] can
point at the wrong workspace by the time move-window-to-workspace
runs — restoring to the wrong place. The stashed window itself can
also have closed in the meantime, in which case the move/focus calls
would silently no-op. main() now re-reads workspaces/windows after
pick() returns, recomputes the current workspace, and confirms the
chosen window still exists before acting; each bails with a notify
and the documented exit code otherwise. Adds window_exists() as a
pure, tested helper alongside the existing pure functions.

Also: clarify the focus=false comment in niri/config.kdl (it read as
if focus=false caused the problem it actually prevents), and note in
the design doc's Out of scope section that the stash's index-1
pinning is per-output, so a second monitor would need its own
analysis of the Mod+1..9 offset.
This commit is contained in:
funman300
2026-07-16 10:10:40 -07:00
parent 41f0ee88ff
commit 05cc026d75
4 changed files with 42 additions and 2 deletions
@@ -226,3 +226,7 @@ the stash holds index 1).
- Restoring to a window's *original* workspace. Rejected during design: it needs - Restoring to a window's *original* workspace. Rejected during design: it needs
per-window origin tracking that can go stale. per-window origin tracking that can go stale.
- A waybar module showing a stash count — the workspace pill already signals it. - A waybar module showing a stash count — the workspace pill already signals it.
- Multi-monitor correctness: the stash is pinned to index 1 only on its own
output, so a second monitor's workspaces would start at 1 with no stash,
throwing off the `Mod+1..9` +1 offset there. Correct on this single-panel
(`eDP-1`) machine; latent if a second output is ever attached.
+2 -2
View File
@@ -110,8 +110,8 @@ binds {
Mod+Print { spawn "screenshot"; } Mod+Print { spawn "screenshot"; }
// focus=false or focus follows the window into the stash — the opposite of // focus=false is required here — otherwise focus follows the window into
// minimizing. // the stash, the opposite of minimizing.
Mod+M { move-window-to-workspace "minimized" focus=false; } Mod+M { move-window-to-workspace "minimized" focus=false; }
Mod+Shift+M { spawn "window-restore"; } Mod+Shift+M { spawn "window-restore"; }
+5
View File
@@ -78,6 +78,11 @@ DUPES = [
] ]
check("duplicate labels resolve to the first", wr.resolve(DUPES, "Alacritty — same")["id"], 20) check("duplicate labels resolve to the first", wr.resolve(DUPES, "Alacritty — same")["id"], 20)
# --- window_exists -----------------------------------------------------------
check("window_exists is True for a present window", wr.window_exists(WINDOWS, 11), True)
check("window_exists is False once the window has closed", wr.window_exists(WINDOWS, 999), False)
check("window_exists is False for an empty list", wr.window_exists([], 11), False)
# --- pick ------------------------------------------------------------------ # --- pick ------------------------------------------------------------------
check("pick returns the chooser's selection", check("pick returns the chooser's selection",
wr.pick(["a", "b"], lambda text: "b"), "b") wr.pick(["a", "b"], lambda text: "b"), "b")
+31
View File
@@ -50,6 +50,13 @@ def resolve(windows, chosen):
return next((w for w in windows if label(w) == chosen), None) return next((w for w in windows if label(w) == chosen), None)
def window_exists(windows, window_id):
"""Whether window_id is still present in a fresh `windows` list. Used after
the picker returns, since wofi can block long enough for the chosen window
to have closed out from under us."""
return any(w.get("id") == window_id for w in windows)
def pick(labels, chooser): def pick(labels, chooser):
"""chooser(text) -> selected line. Injected so tests need no wofi.""" """chooser(text) -> selected line. Injected so tests need no wofi."""
return chooser("\n".join(labels)) return chooser("\n".join(labels))
@@ -140,6 +147,30 @@ def main():
if window is None: if window is None:
return 0 return 0
# wofi may have sat open for however long the user took. In that time niri
# can destroy a dynamic workspace (its last window closed) and renumber
# every later one down, so the pre-pick `current["idx"]` may now name the
# wrong workspace; the stashed window may also have closed. Re-read state
# rather than trust anything captured before pick() blocked.
workspaces = niri_json("workspaces")
if workspaces is None:
notify("niri IPC unavailable")
return 1
current = current_workspace(workspaces)
if current is None:
notify("no focused workspace")
return 1
windows = niri_json("windows")
if windows is None:
notify("niri IPC unavailable")
return 1
if not window_exists(windows, window["id"]):
notify("that window closed before it could be restored")
return 0
# The window is not focused (it is in the stash), so --focus does not apply; # The window is not focused (it is in the stash), so --focus does not apply;
# focus it explicitly afterwards. # focus it explicitly afterwards.
niri_action("move-window-to-workspace", "--window-id", str(window["id"]), niri_action("move-window-to-workspace", "--window-id", str(window["id"]),