261020.19 · f2e0dd99f7594889ff9894aa56b54e032a87589d
fix(win): pin parent NVIM in spawn env, always quit CLI on attach
Address the second review round on the Windows spawn route.
`fzf.raw_fzf` pins `NVIM` to `vim.v.servername` in the `win_spawn` spec environment. The spec env is also consumed by the TUI mediator (`win_spawn.lua`), where `libuv.uv_spawn` fills missing variables from the process that spawns fzf (`tbl_extend("keep", opts.env, env)`), so without the pin fzf's children (`nvim --server $NVIM` commands, nvim_server previewer, item commands) would receive the mediator's own server address instead of the editor that owns the picker. The in-process CLI route is unchanged (`uv_spawn` already injected the same value there); in the mediator route the pinned key wins over the mediator's own `NVIM` injection.
The CLI server-list action quits after `win_exec` regardless of the launch result, superseding the conditional from 981d47d1. By the time the takeover runs, the action has already closed every channel and terminated every child (fzf included): the picker cannot continue and a surviving CLI process would only hang the console it was supposed to hand over. `win_exec` still reports spawn failures via `utils.warn`.
`child.stop` in the test helpers warns when a child is still alive after `jobstop` before returning, still keeping `child.job` for a later retry. `vim.notify` over `error()`, which would not surface since callers `pcall(child.stop)`; a silent return would hide a stuck child from CI logs.
Review note (verified, no change): `test.screenshot.with_retry` was asked to call `capture()` outside the retry `pcall` - it already does. Argument expressions are evaluated before `pcall` is entered, so `capture()` errors propagate immediately and never consume a retry; only `compare` failures (the intended retry target) are caught.
Tests: `fzf_spec` win_spawn case asserts the pinned `NVIM`. Windows full suite green on nightly (313 cases, 0 fails) and stable (313/0), focused `{win}` group green, emmylua --warnings-as-errors clean.
reading declarations
@@ -230,7 +230,15 @@ function M.raw_fzf(contents, fzf_cli_args, opts) } -- the direct route (`win_spawn`) receives the same environment in its spec -- (used in-process by the CLI host and serialized for the TUI mediator)- if win_spec then win_spec.env = job_env end+ if win_spec then+ -- pin `NVIM` to this instance: `libuv.uv_spawn` fills missing variables+ -- from the process that spawns fzf, so without this the TUI mediator's+ -- spawns would expose the mediator's own server address to fzf's children+ -- (`nvim --server $NVIM`, nvim_server previewer, item commands) instead+ -- of the editor that owns the picker+ job_env["NVIM"] = vim.v.servername+ win_spec.env = job_env+ end -- write the mediator spec file now that it carries the full environment if win_spec and win_spec_path then local payload = libuv.serialize(win_spec, true)
@@ -96,9 +96,9 @@ end -- spaces or quotes in paths. Optional trailing args may be nil (no lnum -- or col) and are filtered out since `uv.spawn` needs a dense argv. -- Spawn failures are reported via `utils.warn` and returned as `false`; the--- wrapper never raises, so callers must check the return and skip their--- takeover `quit()` when the launch failed, otherwise the picker exits--- without anything taking the console over.+-- wrapper never raises. Callers quit afterwards either way: a failed launch+-- only means nothing takes the console over, and leaving the dead picker+-- process around would hang the console instead of returning to the shell. -- When the parent's stdout is not a console (e.g. PowerShell captures the -- picker output through a pipe, as the shell widgets do), `{ 0, 1, 2 }` -- would give the child a stdout it cannot render a TUI on and the CLI@@ -283,11 +283,12 @@ return { enable_stdio_inheritance() if _is_win then -- no `execl` replacement on Windows: `win_exec` returns and the- -- picker must quit explicitly to hand the console over; a failed- -- launch (reported by `win_exec`) keeps this picker alive- if win_exec(NVIM_BIN, "--remote-ui", "--server", remote) then- quit()- end+ -- picker must quit explicitly to hand the console over. Quit even+ -- on a failed launch (reported by `win_exec`): the channels and+ -- children above are already torn down, the picker cannot continue+ -- and a leftover CLI would hang the console+ win_exec(NVIM_BIN, "--remote-ui", "--server", remote)+ quit() else posix_exec(NVIM_BIN, "--remote-ui", "--server", remote) end
@@ -233,7 +233,11 @@ M.new_child_neovim = function() pcall(vim.fn.jobstop, job_id) if vim.fn.jobwait({ job_id }, 1000)[1] == -1 then -- still running after jobstop: keep `child.job` so a later `stop` can- -- retry instead of deleting the address of a live process+ -- retry instead of deleting the address of a live process, but report+ -- it (`error()` would not surface: callers `pcall(child.stop)`)+ vim.notify(string.format(+ "child.stop: job %d still alive after `jobstop`, kept for a retry",+ job_id), vim.log.levels.WARN) return end end
@@ -101,6 +101,9 @@ describe("Testing fzf launcher", function() assert.is.True(#spec.outfile > 0) eq(captured.env.SHELL, "cmd.exe") eq(captured.env.FZF_DEFAULT_COMMAND, "echo foo || break")+ -- `NVIM` is pinned to this instance (the spec env is also consumed by the+ -- TUI mediator's spawns, see `fzf.raw_fzf`)+ eq(captured.env.NVIM, vim.v.servername) end) it("has no cmd.exe/PowerShell length limit for long arguments", function()