mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 08:02:24 +00:00
fix(cli): a refused split left a live pane nothing was holding
`tty7 split` spawns the shell and only then asks the tree to hold it, so any refusal in between leaves a pane running that nothing references — `pane ls --all` shows it, the tree does not, and `pane close --orphans` is the only way to be rid of it. Two attempts at `--ratio nan` left two. NaN is the case that gets there, because it cannot be serialised onto the control connection at all. The link drops mid-request, so the daemon's own "a split ratio must be a finite number" never comes back: the caller sees "control connection lost (request 2)" instead, which says nothing about the ratio, and the pane is already running. Refuse a non-finite ratio before the spawn, in the daemon's own terms. Its clamp to a usable range is deliberate and stays its business — 0, 1 and 5.0 still land on the clamp, as intended; this only refuses what the daemon would refuse anyway. Then take the pane back down if the split fails for any other reason too. Nobody asked for a pane that no tree holds, and leaving it for the user to find with `--orphans` is not a refusal, it is a mess. Verified against a live server: two refused splits now leave the pane count where it was, and a valid `--ratio 0.3` split still works. Swept the rest of the value-taking flags while here, all sound: `-m` on an unknown machine, `--key` on an unknown key (which lists the real ones), `--timeout 0` (exits 124, the conventional code) and a negative timeout are each refused with the right message.
This commit is contained in:
@@ -519,8 +519,17 @@ fn pane_split(args: SplitArgs, ctx: &Context, backend: &mut dyn Backend) -> Resu
|
||||
} else {
|
||||
Axis::Vertical
|
||||
};
|
||||
// Before the spawn, because the pane exists before the tree hears about
|
||||
// it. `--ratio nan` cannot even be put on the wire — serialising it breaks
|
||||
// the control connection, so the daemon's own "a split ratio must be a
|
||||
// finite number" never arrives — and by then a shell is already running
|
||||
// that no tree holds. The daemon's clamp to a usable range is deliberate
|
||||
// and stays its business; this only refuses what it would refuse too.
|
||||
if !args.ratio.is_finite() {
|
||||
bail!("--ratio {} is not a finite number", args.ratio);
|
||||
}
|
||||
let new = backend.spawn_shell(workspace, cwd.clone())?;
|
||||
backend.control(ControlRequest::PaneSplit {
|
||||
let split = backend.control(ControlRequest::PaneSplit {
|
||||
workspace,
|
||||
pane,
|
||||
axis,
|
||||
@@ -533,7 +542,15 @@ fn pane_split(args: SplitArgs, ctx: &Context, backend: &mut dyn Backend) -> Resu
|
||||
shell: None,
|
||||
},
|
||||
first: false,
|
||||
})?;
|
||||
});
|
||||
if let Err(e) = split {
|
||||
// Whatever the refusal was, the pane spawned above is now running with
|
||||
// nothing referencing it: `pane ls --all` shows it, the tree does not,
|
||||
// and it takes `pane close --orphans` to be rid of it. Nobody asked for
|
||||
// that pane, so take it back down rather than leave it for the user.
|
||||
let _ = backend.kill_pane(new);
|
||||
return Err(e);
|
||||
}
|
||||
report(format!("%{new}"), json!({ "pane": new }))
|
||||
}
|
||||
|
||||
@@ -3012,6 +3029,31 @@ mod tests {
|
||||
assert_eq!(backend.runs.len(), 1, "the routed run must still happen");
|
||||
}
|
||||
|
||||
/// A ratio the split cannot use is refused before a pane is spawned.
|
||||
///
|
||||
/// `split` spawns the shell and only then asks the tree to hold it, so any
|
||||
/// refusal in between leaves a pane running that nothing references —
|
||||
/// visible to `pane ls --all`, absent from the tree, and removable only by
|
||||
/// `pane close --orphans`. `--ratio nan` did exactly that twice over,
|
||||
/// because a NaN cannot be serialised onto the control connection at all:
|
||||
/// the link dropped, so the daemon's own "must be a finite number" never
|
||||
/// came back, and two orphans were left behind.
|
||||
#[test]
|
||||
fn a_ratio_that_cannot_be_used_spawns_nothing() {
|
||||
let mut backend = mock();
|
||||
let err = execute(
|
||||
cli(&["tty7", "split", "%1", "--horizontal", "--ratio", "nan"]),
|
||||
&Context::default(),
|
||||
&mut backend,
|
||||
)
|
||||
.expect_err("a split needs a real ratio");
|
||||
assert!(err.to_string().contains("finite"), "{err}");
|
||||
assert!(
|
||||
backend.spawned.is_empty(),
|
||||
"the refusal has to come before the spawn, or it leaves an orphan"
|
||||
);
|
||||
}
|
||||
|
||||
/// `tty7 new <path>` refuses a path it cannot root the workspace at, and
|
||||
/// refuses it before creating anything.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user