mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-21 16:02:20 +00:00
fix(cli): drop tab group, and shore up the workspace deletion path
`tab group` wrote a field the GUI recomputes on every sidebar render from the tab's repository root, so a heading set from the CLI was overwritten before anyone could read it. The command is gone; the read-only GROUP column stays, and the docs now say where its value comes from. Alongside it: - `tty7 new --open` tells a failed request apart from a missing GUI instead of reporting both as "no GUI is running", and its warnings carry the `tty7:` prefix the rest of the CLI uses. Neither is an exit code: the workspace exists by the time we ask. - The switcher measures unclaimed workspaces against the rows already listed rather than against the store that put them there, so this window's own workspace cannot show up twice. - `on_workspace_deleted` gets a test for its destructive half — the one that erases state only this client holds. - The empty-tree write-back says why it covers remote workspaces too, and both unreachable `WorkspaceDeleted` arms say they are unreachable. - `TabView` is no longer serialisable: it is a reading of the machine tree, and both sides that want one have the tree already. - New tests for the `tab ls` table and its JSON.
This commit is contained in:
@@ -391,17 +391,6 @@ pub enum TabCmd {
|
||||
#[arg(value_name = "INDEX")]
|
||||
index: u64,
|
||||
},
|
||||
|
||||
#[command(about = "Put a tab in a sidebar group, or take it out of one")]
|
||||
Group {
|
||||
#[arg(value_name = "@TAB")]
|
||||
tab: String,
|
||||
#[arg(
|
||||
value_name = "GROUP",
|
||||
help = "The group to join; omit to leave whatever group it is in"
|
||||
)]
|
||||
group: Option<String>,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Debug, Subcommand)]
|
||||
|
||||
@@ -85,7 +85,6 @@ pub fn execute(cli: Cli, ctx: &Context, backend: &mut dyn Backend) -> Result<Out
|
||||
Some(Command::Tab(TabCmd::Close { tab })) => tab_close(&tab, backend),
|
||||
Some(Command::Tab(TabCmd::Rename { tab, name })) => tab_rename(&tab, name, backend),
|
||||
Some(Command::Tab(TabCmd::Move { tab, index })) => tab_move(&tab, index, backend),
|
||||
Some(Command::Tab(TabCmd::Group { tab, group })) => tab_group(&tab, group, backend),
|
||||
Some(Command::Pane(PaneCmd::Ls { ws, all })) => pane_ls(ws.as_deref(), all, backend),
|
||||
Some(Command::Pane(PaneCmd::Close { target })) => {
|
||||
pane_close(target.as_deref(), ctx, backend)
|
||||
@@ -352,17 +351,34 @@ fn new_workspace(path: Option<String>, open: bool, backend: &mut dyn Backend) ->
|
||||
})?;
|
||||
// Only when asked: a workspace made from a script has no business
|
||||
// stealing the screen, and the switcher lists it either way.
|
||||
let opened = open
|
||||
&& matches!(
|
||||
backend.control(ControlRequest::GuiOpen {
|
||||
path: None,
|
||||
workspace: Some(ws.id),
|
||||
}),
|
||||
Ok(ReplyOk::Bool(true))
|
||||
);
|
||||
if open && !opened {
|
||||
eprintln!("no GUI is running on this machine; the workspace was made all the same");
|
||||
}
|
||||
let opened = match open {
|
||||
false => false,
|
||||
true => match backend.control(ControlRequest::GuiOpen {
|
||||
path: None,
|
||||
workspace: Some(ws.id),
|
||||
}) {
|
||||
Ok(ReplyOk::Bool(opened)) => {
|
||||
// The workspace exists by the time we ask, so an unreachable
|
||||
// GUI is worth a word and not an exit code: failing here would
|
||||
// read as "nothing was made".
|
||||
if !opened {
|
||||
eprintln!(
|
||||
"tty7: no GUI is running on this machine; \
|
||||
the workspace was made all the same"
|
||||
);
|
||||
}
|
||||
opened
|
||||
}
|
||||
Ok(other) => bail!("the server answered GuiOpen with {other:?}"),
|
||||
Err(error) => {
|
||||
eprintln!(
|
||||
"tty7: could not ask the GUI to open it ({error:#}); \
|
||||
the workspace was made all the same"
|
||||
);
|
||||
false
|
||||
}
|
||||
},
|
||||
};
|
||||
report(
|
||||
ws.id.to_string(),
|
||||
json!({ "id": ws.id.to_string(), "pane": pane, "opened": opened }),
|
||||
@@ -645,24 +661,6 @@ fn tab_move(tab: &str, index: u64, backend: &mut dyn Backend) -> Result<Outcome>
|
||||
report("", json!({ "tab": tab.to_string(), "to": index }))
|
||||
}
|
||||
|
||||
/// The GUI files tabs under headings in its sidebar, and that heading is a
|
||||
/// field on the tab like any other. Until now only the GUI could write it,
|
||||
/// so a tab the CLI made landed in the ungrouped pile with no way out.
|
||||
fn tab_group(tab: &str, group: Option<String>, backend: &mut dyn Backend) -> Result<Outcome> {
|
||||
let addr = address::parse_tab(tab)?;
|
||||
let machine = fetch_machine(backend)?;
|
||||
let (workspace, tab) = resolve::tab(&machine, &addr)?;
|
||||
let group = group
|
||||
.map(|g| g.trim().to_string())
|
||||
.filter(|g| !g.is_empty());
|
||||
backend.control(ControlRequest::TabSetGroup {
|
||||
workspace,
|
||||
tab,
|
||||
group: group.clone(),
|
||||
})?;
|
||||
report("", json!({ "tab": tab.to_string(), "group": group }))
|
||||
}
|
||||
|
||||
fn pane_ls(explicit: Option<&str>, all: bool, backend: &mut dyn Backend) -> Result<Outcome> {
|
||||
if all {
|
||||
return pane_ls_all(backend);
|
||||
@@ -1510,28 +1508,47 @@ mod tests {
|
||||
to: 0,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
backend.control_calls.clear();
|
||||
run_cli(&["tty7", "tab", "group", "@1", " scm "], &ctx, &mut backend);
|
||||
assert_eq!(
|
||||
backend.control_calls[1],
|
||||
ControlRequest::TabSetGroup {
|
||||
workspace: api.id,
|
||||
tab: api.tabs[0].id,
|
||||
group: Some("scm".into()),
|
||||
}
|
||||
#[test]
|
||||
fn tab_ls_names_an_unnamed_tab_and_shows_the_leaf_of_its_group() {
|
||||
let mut backend = mock();
|
||||
backend.machine.workspaces[0].tabs[1].sidebar_group = Some("C:\\proj\\sub".into());
|
||||
|
||||
let out = run_cli(
|
||||
&["tty7", "tab", "ls", "api"],
|
||||
&Context::default(),
|
||||
&mut backend,
|
||||
);
|
||||
|
||||
backend.control_calls.clear();
|
||||
run_cli(&["tty7", "tab", "group", "@1"], &ctx, &mut backend);
|
||||
// @1 was named; @2 was not, so it borrows the leaf of its cwd. The
|
||||
// GROUP column is the heading's last segment, not the whole path.
|
||||
assert_eq!(
|
||||
backend.control_calls[1],
|
||||
ControlRequest::TabSetGroup {
|
||||
workspace: api.id,
|
||||
tab: api.tabs[0].id,
|
||||
group: None,
|
||||
},
|
||||
"no group named means leave the group"
|
||||
human(out),
|
||||
"TAB NAME GROUP PANES\n@1 build - 1\n@2 proj sub 2\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tab_ls_json_keeps_the_literal_name_beside_the_label() {
|
||||
let mut backend = mock();
|
||||
backend.machine.workspaces[0].tabs[1].sidebar_group = Some("C:\\proj\\sub".into());
|
||||
|
||||
let out = run_cli(
|
||||
&["tty7", "tab", "ls", "api"],
|
||||
&Context::default(),
|
||||
&mut backend,
|
||||
);
|
||||
|
||||
let Outcome::Report(report) = out else {
|
||||
panic!("tab ls must report");
|
||||
};
|
||||
let tabs = report.json["tabs"].as_array().expect("tabs").clone();
|
||||
assert_eq!(tabs[1]["name"], Value::Null, "nobody named this tab");
|
||||
assert_eq!(tabs[1]["label"], "proj", "the table's stand-in travels too");
|
||||
assert_eq!(
|
||||
tabs[1]["group"], "C:\\proj\\sub",
|
||||
"the JSON keeps the whole heading the table abbreviates"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -6,12 +6,13 @@
|
||||
//! the machine tree. This is the reading of that tree, kept in one place so
|
||||
//! the CLI and the GUI name a tab the same way.
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::core::cli_agent::{AgentStatus, CLIAgent};
|
||||
use crate::core::machine::{PaneRecord, TabId, Workspace};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
/// Deliberately not serialisable: it is a reading of the machine tree, and
|
||||
/// both sides that want one have the tree already. Putting it on the wire
|
||||
/// would be sending a conclusion where the evidence has already gone.
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct TabView {
|
||||
pub id: TabId,
|
||||
pub name: Option<String>,
|
||||
|
||||
@@ -205,7 +205,11 @@ resolve it immediately before use. A full tab UUID also works: `@<uuid>`.
|
||||
| `tab close @TAB` | close the tab and every pane in it | `{"closed"}` |
|
||||
| `tab rename @TAB NAME` | name or rename | `{"tab","name"}` |
|
||||
| `tab move @TAB INDEX` | reposition within its workspace | `{"tab","to"}` |
|
||||
| `tab group @TAB [GROUP]` | file it under a sidebar heading, or with no GROUP take it out of one | `{"tab","group"}` |
|
||||
|
||||
GROUP is the heading the GUI's sidebar files the tab under, shown by its last
|
||||
segment (`group` in the JSON is the whole value). Read-only from here: with the
|
||||
default repo grouping the GUI recomputes it from the tab's working directory,
|
||||
so anything written from outside would be overwritten on the next render.
|
||||
|
||||
Almost no tab has a `name`: the GUI's tab strip reads OSC titles, which the
|
||||
machine tree never sees. So the NAME column — and `label` in the JSON — falls
|
||||
|
||||
@@ -483,9 +483,18 @@ impl Tty7App {
|
||||
// person who ran `tty7 new` like nothing happened at all. They open
|
||||
// like any other row — the id in the tree is the id a window claims.
|
||||
if let Some(slot) = groups.iter().position(|g| g.key.is_empty()) {
|
||||
// Measured against the rows already listed rather than against the
|
||||
// store, which is what put them there: the block above lists this
|
||||
// window's own workspace before the store has caught up with it,
|
||||
// and two rows under one id would be two ways into one window.
|
||||
let listed: Vec<WorkspaceId> = groups
|
||||
.iter()
|
||||
.flat_map(|g| g.rows.iter().map(|r| r.id))
|
||||
.collect();
|
||||
let app: &App = cx;
|
||||
let rows: Vec<Row> = crate::ui::machine_mirror::unclaimed_local_workspaces(app)
|
||||
.into_iter()
|
||||
.filter(|ws| !listed.contains(&ws.id))
|
||||
.map(|ws| Row {
|
||||
id: ws.id,
|
||||
name: ws.name,
|
||||
|
||||
+75
-4
@@ -1387,6 +1387,15 @@ fn finish_hydration(
|
||||
// A full window over an empty tree has to write itself back, whether
|
||||
// or not an edit was waiting: the machine is missing tabs this window
|
||||
// is showing, and nothing else would ever put them there.
|
||||
//
|
||||
// Deliberately not limited to this machine. An empty tree means one of
|
||||
// two things and the answer is the same either way: locally the
|
||||
// workspace was removed under the window (`ws rm`, or another client),
|
||||
// and remotely the far end lost its records — a re-imaged box, a store
|
||||
// that was wiped. Writing the window back is what a reattach is for.
|
||||
// The panes it names may well be dead; the window already draws them
|
||||
// that way, and a tab the user can close beats a tab that silently
|
||||
// stops existing.
|
||||
if was_dirty || machine_was_empty {
|
||||
app.update(cx, |app, cx| sync_window(app, cx));
|
||||
}
|
||||
@@ -1531,6 +1540,10 @@ pub(crate) fn on_layout_delta(cx: &mut App, host: HostId, key: &str, delta: Layo
|
||||
|
||||
fn apply_to_mirror(mirror: &mut WsMirror, delta: &LayoutDelta) -> bool {
|
||||
match delta {
|
||||
// Nothing here is about a workspace's tab list, so the mirror is
|
||||
// already right. `WorkspaceDeleted` never reaches this far —
|
||||
// `on_layout_delta` hands it to `on_workspace_deleted` and returns —
|
||||
// and is listed only so a new delta cannot join this arm by accident.
|
||||
LayoutDelta::WorkspaceCreated { .. }
|
||||
| LayoutDelta::WorkspaceRenamed { .. }
|
||||
| LayoutDelta::WorkspaceTouched { .. }
|
||||
@@ -1617,10 +1630,10 @@ impl Tty7App {
|
||||
| LayoutDelta::WorkspaceTouched { .. }
|
||||
| LayoutDelta::WorkspaceRenamed { .. }
|
||||
| LayoutDelta::PaneFacts { .. } => true,
|
||||
// Handled before the window is ever reached — a deletion is about
|
||||
// whether this workspace still exists here at all, which is not a
|
||||
// question one window's tab list can answer. See
|
||||
// `on_workspace_deleted`.
|
||||
// Unreachable: `on_layout_delta` hands a deletion to
|
||||
// `on_workspace_deleted` and returns before any window is asked. A
|
||||
// deletion is about whether this workspace still exists here at
|
||||
// all, which is not a question one window's tab list can answer.
|
||||
LayoutDelta::WorkspaceDeleted => true,
|
||||
LayoutDelta::ActiveTabChanged { tab } => {
|
||||
if let Some(index) = index_of(&self.tabs, *tab) {
|
||||
@@ -1912,6 +1925,64 @@ mod tests {
|
||||
});
|
||||
}
|
||||
|
||||
/// The destructive half of a deletion. It erases state only this client
|
||||
/// holds — geometry, the label, a remote binding — so the fence in front of
|
||||
/// it ("no window is showing this workspace") is the whole safety of it.
|
||||
///
|
||||
/// The other half needs a live `Tty7App` in a real window to reach, so it
|
||||
/// is not tested here; what it does is hydrate, which the hydration tests
|
||||
/// cover, and it touches neither the store nor the registry.
|
||||
#[gpui::test]
|
||||
fn a_deletion_nothing_has_open_forgets_the_workspace_here_too(cx: &mut gpui::TestAppContext) {
|
||||
use crate::core::session::{WindowView, WindowViews};
|
||||
|
||||
cx.update(|cx| {
|
||||
// Removing a workspace saves the views, and a test has no business
|
||||
// writing the real ones.
|
||||
let _ = tty7_core::core::config::set_config_dir(
|
||||
std::env::temp_dir().join(format!("tty7-deleted-test-{}", std::process::id())),
|
||||
);
|
||||
crate::ui::windows::WindowRegistry::init(cx);
|
||||
|
||||
let deleted = WindowView::default();
|
||||
let gone = deleted.id;
|
||||
let untouched = WindowView::default();
|
||||
let survivor = untouched.id;
|
||||
WorkspaceStore::install_for_test(
|
||||
cx,
|
||||
WindowViews {
|
||||
views: vec![deleted, untouched],
|
||||
active: Some(gone),
|
||||
},
|
||||
);
|
||||
cx.default_global::<TreeSync>()
|
||||
.windows
|
||||
.entry(gone)
|
||||
.or_default()
|
||||
.sync = SyncPhase::Primed(WsMirror::default());
|
||||
|
||||
on_workspace_deleted(cx, gone);
|
||||
|
||||
assert!(
|
||||
WorkspaceStore::all(cx).get(gone).is_none(),
|
||||
"a row that opens onto nothing is worse than no row at all"
|
||||
);
|
||||
assert_eq!(
|
||||
WorkspaceStore::all(cx).active,
|
||||
None,
|
||||
"the active workspace cannot be one that no longer exists"
|
||||
);
|
||||
assert!(
|
||||
WorkspaceStore::all(cx).get(survivor).is_some(),
|
||||
"a deletion is about one workspace, not about the store"
|
||||
);
|
||||
assert!(
|
||||
!cx.default_global::<TreeSync>().windows.contains_key(&gone),
|
||||
"its sync state has nothing left to be about"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/// The rule that stops a failed rebuild from being read as "empty".
|
||||
///
|
||||
/// A window with no tabs may delete its workspace outright — tree and store
|
||||
|
||||
Reference in New Issue
Block a user