WIP: coordinate rename on the wire and in the browser (improvise-fdk1)

This commit is contained in:
Edward Langley
2026-09-22 23:25:12 -07:00
parent dd51f4316c
commit 5d9dcceaa0
14 changed files with 212 additions and 25 deletions
@@ -636,7 +636,13 @@ fn commit_plain_records_edit(
return; return;
}; };
match col { match col {
RecordCol::Category(_) | RecordCol::Measure(_) => { // The one door the wire's `RenameRecordCoordinate` also takes (improvise-fdk1).
RecordCol::Category(category) => effects.push(Box::new(effect::RenameRecordCoordinate {
record: row.coords.clone(),
category: category.clone(),
item: value.to_string(),
})),
RecordCol::Measure(_) => {
effects.extend(effect::RecordRewrite::of(ctx.model, row, [(col, value)]).effects()); effects.extend(effect::RecordRewrite::of(ctx.model, row, [(col, value)]).effects());
} }
RecordCol::NewMeasure => add_measure(value, effects), RecordCol::NewMeasure => add_measure(value, effects),
+23
View File
@@ -686,6 +686,29 @@ impl Effect for ApplyAndClearDrill {
} }
} }
/// Move the record at `record` to `category = item`: the key path's and the wire's
/// one door, through the same [`RecordRewrite`] a drill uses (improvise-fdk1).
#[derive(Debug)]
pub struct RenameRecordCoordinate {
pub record: CellKey,
pub category: String,
pub item: String,
}
impl Effect for RenameRecordCoordinate {
fn apply(&self, app: &mut App) {
let model = &app.model_state.workbook.model;
let row = crate::view::record_row(model, &self.record);
let col = RecordCol::Category(self.category.clone());
let rewrite = RecordRewrite::of(model, &row, [(&col, self.item.as_str())]);
for effect in rewrite.effects() {
effect.apply(app);
}
}
fn kind(&self) -> EffectKind {
EffectKind::Model(ModelChange::CellData)
}
}
/// A records row's edits composed into the one record they mean, read off the model /// A records row's edits composed into the one record they mean, read off the model
/// before anything is written. Why composed: `context/code/command.md`, `RecordRewrite`. /// before anything is written. Why composed: `context/code/command.md`, `RecordRewrite`.
#[derive(Debug, Default, PartialEq)] #[derive(Debug, Default, PartialEq)]
+17 -9
View File
@@ -322,8 +322,13 @@ fn cell_id(app: &App, layout: &GridLayout, row: usize, col: usize) -> Option<Cel
RecordCol::NewMeasure => return Some(CellId::Create(CreateKind::Measure)), RecordCol::NewMeasure => return Some(CellId::Create(CreateKind::Measure)),
RecordCol::NewCategory => return Some(CellId::Create(CreateKind::Category)), RecordCol::NewCategory => return Some(CellId::Create(CreateKind::Category)),
// Renaming a coordinate moves every measure cell in the row, which is a // Renaming a coordinate moves every measure cell in the row, which is a
// different intent from `SetCell` and still the host's (gotcha 1). // different intent from `SetCell`: its own operands (gotcha 1, fdk1).
RecordCol::Category(_) => return Some(CellId::Coordinate), RecordCol::Category(category) => {
return Some(CellId::Coordinate {
record: RecordHash::of(record),
category: category.clone(),
});
}
}; };
return Some(CellId::Record(RecordCell { return Some(CellId::Record(RecordCell {
record: RecordHash::of(record), record: RecordHash::of(record),
@@ -491,11 +496,10 @@ pub fn published_basis(app: &App, grid: &GridCache) -> Basis {
.iter() .iter()
.flatten() .flatten()
.filter_map(|cell| match cell.id.as_ref()? { .filter_map(|cell| match cell.id.as_ref()? {
CellId::Record(rc) => Some((rc.record, app.record_version_of(rc.record))), CellId::Record(RecordCell { record, .. }) | CellId::Coordinate { record, .. } => {
CellId::Aggregate(_) Some((*record, app.record_version_of(*record)))
| CellId::Derived { .. } }
| CellId::Create(_) CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
| CellId::Coordinate => None,
}) })
.collect(); .collect();
basis::published(app.document_generation(), app.layout_version(), records) basis::published(app.document_generation(), app.layout_version(), records)
@@ -5158,10 +5162,14 @@ mod cell_identity {
let region = column_named(&app, "Region"); let region = column_named(&app, "Region");
let north = grid.cell(1, region).expect("the second row's coordinate"); let north = grid.cell(1, region).expect("the second row's coordinate");
assert_eq!(north.text, "North", "setup: a visible coordinate cell"); assert_eq!(north.text, "North", "setup: a visible coordinate cell");
let record = app.layout.record_row(1).expect("row 1").coords.clone();
assert_eq!( assert_eq!(
north.id, north.id,
Some(CellId::Coordinate), Some(CellId::Coordinate {
"a drawn coordinate is named" record: RecordHash::of(&record),
category: "Region".into(),
}),
"a drawn coordinate is named, with its own record and category (fdk1)"
); );
let nameless: Vec<(usize, usize)> = (0..app.layout.row_count()) let nameless: Vec<(usize, usize)> = (0..app.layout.row_count())
.flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c))) .flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
+20
View File
@@ -147,6 +147,20 @@ pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
}; };
Box::new(effect::ClearCell(key)) Box::new(effect::ClearCell(key))
} }
ModelCommand::RenameRecordCoordinate {
record,
category,
item,
} => {
let Some(record) = app.resolve_record(*record) else {
return vec![];
};
Box::new(effect::RenameRecordCoordinate {
record,
category: category.clone(),
item: item.clone(),
})
}
ModelCommand::AddItem { category, item } => Box::new(effect::AddItem { ModelCommand::AddItem { category, item } => Box::new(effect::AddItem {
category: category.clone(), category: category.clone(),
item: item.clone(), item: item.clone(),
@@ -664,6 +678,11 @@ pub(crate) mod tests {
value: CellValue::Number(1.0), value: CellValue::Number(1.0),
}, },
ModelCommand::ClearCell(record_cell(&a_stored_cell())), ModelCommand::ClearCell(record_cell(&a_stored_cell())),
ModelCommand::RenameRecordCoordinate {
record: record_cell(&a_stored_cell()).record,
category: "Region".into(),
item: "West".into(),
},
ModelCommand::AddItem { ModelCommand::AddItem {
category: "Region".into(), category: "Region".into(),
item: "South".into(), item: "South".into(),
@@ -744,6 +763,7 @@ pub(crate) mod tests {
match cmd { match cmd {
ModelCommand::SetCell { .. } => "SetCell", ModelCommand::SetCell { .. } => "SetCell",
ModelCommand::ClearCell(_) => "ClearCell", ModelCommand::ClearCell(_) => "ClearCell",
ModelCommand::RenameRecordCoordinate { .. } => "RenameRecordCoordinate",
ModelCommand::AddItem { .. } => "AddItem", ModelCommand::AddItem { .. } => "AddItem",
ModelCommand::AddItemInGroup { .. } => "AddItemInGroup", ModelCommand::AddItemInGroup { .. } => "AddItemInGroup",
ModelCommand::RemoveItem { .. } => "RemoveItem", ModelCommand::RemoveItem { .. } => "RemoveItem",
+12
View File
@@ -104,6 +104,18 @@ pub fn drill_snapshot(model: &Model, constraint: &CellKey) -> Vec<RecordRow> {
RecordRow::group(cells) RecordRow::group(cells)
} }
/// The records row `record` is, as the records grid groups it; empty when no
/// cell stores it. The one lookup a rename by record uses (improvise-fdk1).
pub fn record_row(model: &Model, record: &CellKey) -> RecordRow {
drill_snapshot(model, record)
.into_iter()
.find(|row| &row.coords == record)
.unwrap_or_else(|| RecordRow {
coords: record.clone(),
values: Vec::new(),
})
}
/// One step of a view configuration: **what** a view must be, with no answer /// One step of a view configuration: **what** a view must be, with no answer
/// to how a front end gets there. The model half of drill — core.md §8. /// to how a front end gets there. The model half of drill — core.md §8.
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
+1 -1
View File
@@ -5,7 +5,7 @@ pub mod types;
pub use axis::{Axis, AxisColor}; pub use axis::{Axis, AxisColor};
pub use layout::{ pub use layout::{
AxisEntry, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, RecordCol, RecordRow, ViewStep, AxisEntry, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, RecordCol, RecordRow, ViewStep,
drill_constraint, drill_snapshot, drill_steps, records_spine, records_steps, drill_constraint, drill_snapshot, drill_steps, record_row, records_spine, records_steps,
synthetic_record_info, synthetic_record_info,
}; };
pub use types::{ pub use types::{
+16
View File
@@ -93,6 +93,22 @@ impl BasisCache {
} }
} }
impl BasisCache {
/// A basis for records whose counters were read EARLIER, such as when an
/// editor opened (improvise-0j0f). The document tiers are current.
pub fn claim_as_read(
&self,
read: impl IntoIterator<Item = (RecordHash, u64)>,
) -> Option<Basis> {
let current = self.current.as_ref()?;
Some(Basis {
document_generation: current.document_generation,
layout_version: current.layout_version,
records: read.into_iter().collect(),
})
}
}
impl WidgetCache for BasisCache { impl WidgetCache for BasisCache {
type Update = Basis; type Update = Basis;
+38
View File
@@ -6,6 +6,7 @@ use crate::{
Rejection, Snapshot, TileBarCache, ViewListCache, ViewState, WidgetCache, reduce_view, Rejection, Snapshot, TileBarCache, ViewListCache, ViewState, WidgetCache, reduce_view,
settled_mode, settled_mode,
}; };
use improvise_core::model::record::RecordHash;
use serde::Serialize; use serde::Serialize;
/// One client's whole state: the portable session slice plus one cache per /// One client's whole state: the portable session slice plus one cache per
@@ -43,6 +44,19 @@ pub struct ClientState {
/// replay the rest. Why a queue: `context/code/clients.md`. /// replay the rest. Why a queue: `context/code/clients.md`.
#[serde(skip)] #[serde(skip)]
pub in_flight: std::collections::VecDeque<Sent>, pub in_flight: std::collections::VecDeque<Sent>,
/// The coordinate an open editor renames, captured as the user saw it.
/// When it is taken, and when not yet: `context/code/clients.md`.
#[serde(skip)]
pub opened: Option<OpenedCoordinate>,
}
/// A coordinate editor's target: the record, the category, and the record's
/// write counter when the editor opened (improvise-fdk1, improvise-0j0f).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct OpenedCoordinate {
pub record: RecordHash,
pub category: String,
pub version: u64,
} }
/// One frame this client has sent. `awaited` is true exactly when the host /// One frame this client has sent. `awaited` is true exactly when the host
@@ -75,6 +89,7 @@ impl ClientState {
} }
self.settle_mode(); self.settle_mode();
self.settle_editor(); self.settle_editor();
self.settle_opened();
self.revision += 1; self.revision += 1;
} }
@@ -87,6 +102,7 @@ impl ClientState {
} }
self.settle_mode(); self.settle_mode();
self.settle_editor(); self.settle_editor();
self.settle_opened();
self.revision += 1; self.revision += 1;
} }
@@ -113,6 +129,7 @@ impl ClientState {
} }
} }
self.settle_sent(); self.settle_sent();
self.settle_opened();
} }
/// The host applied **none** of the oldest awaited frame (ADR 001 clause /// The host applied **none** of the oldest awaited frame (ADR 001 clause
@@ -127,6 +144,7 @@ impl ClientState {
self.last_rejection = Some(rejection); self.last_rejection = Some(rejection);
self.settle_mode(); self.settle_mode();
self.settle_editor(); self.settle_editor();
self.settle_opened();
self.revision += 1; self.revision += 1;
} }
@@ -161,6 +179,26 @@ impl ClientState {
self.view.mode = settled_mode(&self.view.mode, self.grid.records); self.view.mode = settled_mode(&self.view.mode, self.grid.records);
} }
/// Capture the coordinate an open editor renames, but only from a cache that
/// shows the host's answer to this client's own writes (improvise-fdk1).
fn settle_opened(&mut self) {
let answered = !self.in_flight.iter().any(|sent| sent.awaited);
self.opened = match (self.view.mode.is_editing(), self.opened.take()) {
(false, _) => None,
(true, Some(opened)) => Some(opened),
(true, None) if answered => crate::resolve::coordinate_at_cursor(self)
.and_then(|(record, category)| {
let version = self.basis.current()?.record(record)?;
Some(OpenedCoordinate {
record,
category,
version,
})
}),
(true, None) => None,
};
}
/// An editor that arrived without its buffer is seeded here, so what is /// An editor that arrived without its buffer is seeded here, so what is
/// drawn and what Enter commits read one buffer (improvise-ktlk). /// drawn and what Enter commits read one buffer (improvise-ktlk).
fn settle_editor(&mut self) { fn settle_editor(&mut self) {
+13
View File
@@ -86,6 +86,13 @@ pub enum ModelCommand {
value: CellValue, value: CellValue,
}, },
ClearCell(RecordCell), ClearCell(RecordCell),
/// Move a whole record to a new coordinate, creating the item if need be.
/// Position-free: it names the record, never a row (improvise-fdk1).
RenameRecordCoordinate {
record: RecordHash,
category: String,
item: String,
},
AddItem { AddItem {
category: String, category: String,
item: String, item: String,
@@ -170,6 +177,7 @@ pub enum ModelCommand {
pub fn records_named(cmd: &ModelCommand) -> Vec<RecordHash> { pub fn records_named(cmd: &ModelCommand) -> Vec<RecordHash> {
match cmd { match cmd {
ModelCommand::SetCell { cell, .. } | ModelCommand::ClearCell(cell) => vec![cell.record], ModelCommand::SetCell { cell, .. } | ModelCommand::ClearCell(cell) => vec![cell.record],
ModelCommand::RenameRecordCoordinate { record, .. } => vec![*record],
ModelCommand::AddItem { .. } ModelCommand::AddItem { .. }
| ModelCommand::AddItemInGroup { .. } | ModelCommand::AddItemInGroup { .. }
@@ -606,6 +614,11 @@ mod tests {
match cmd { match cmd {
ModelCommand::SetCell { cell, value: v } => record(cell) + value(v), ModelCommand::SetCell { cell, value: v } => record(cell) + value(v),
ModelCommand::ClearCell(cell) => record(cell), ModelCommand::ClearCell(cell) => record(cell),
ModelCommand::RenameRecordCoordinate {
record: r,
category,
item,
} => surrogate(r) + name(category) + name(item),
ModelCommand::AddItem { category, item } ModelCommand::AddItem { category, item }
| ModelCommand::RemoveItem { category, item } => name(category) + name(item), | ModelCommand::RemoveItem { category, item } => name(category) + name(item),
ModelCommand::AddItemInGroup { ModelCommand::AddItemInGroup {
+3 -3
View File
@@ -55,9 +55,9 @@ pub enum CellId {
/// A records-mode creating column: typing a **name** into it adds that /// A records-mode creating column: typing a **name** into it adds that
/// measure or category and writes no cell at all (gotcha 1, improvise-poia). /// measure or category and writes no cell at all (gotcha 1, improvise-poia).
Create(CreateKind), Create(CreateKind),
/// A records-mode coordinate. Renaming it moves every measure cell in the /// A records-mode coordinate: the row's record and the category this column
/// row (gotcha 1), which no client composes: `protocol.md` §3 (improvise-exdg). /// names. Renaming it moves the whole record — `protocol.md` §3 (improvise-fdk1).
Coordinate, Coordinate { record: RecordHash, category: String },
} }
/// What a creating column creates. The name typed into it is the whole /// What a creating column creates. The name typed into it is the whole
+55 -7
View File
@@ -736,15 +736,15 @@ fn cell_at_cursor(state: &ClientState) -> Option<&crate::CellDisplay> {
/// declining out loud rather than upstream. See `context/code/clients.md`. /// declining out loud rather than upstream. See `context/code/clients.md`.
fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> { fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
// The grid, not the mode: a records GRID is the observed fact and the mode // The grid, not the mode: a records GRID is the observed fact and the mode
// follows it (improvise-bd9o, `context/code/clients.md`). // follows it (improvise-bd9o). A coordinate is the one records cell it commits.
if state.grid.records { if state.grid.records && coordinate_at_cursor(state).is_none() {
return Some(vec![Command::SetStatus(not_yet("editing in records mode"))]); return Some(vec![Command::SetStatus(not_yet("editing in records mode"))]);
} }
let cell = cell_at_cursor(state)?; let cell = cell_at_cursor(state)?;
match cell.id.as_ref()? { match cell.id.as_ref()? {
// `edit_text`, never `text`: the drawn text has been through // `edit_text`, never `text`: the drawn text has been through
// `format_f64`. `tests/lockstep.rs` found exactly that. // `format_f64`. `tests/lockstep.rs` found exactly that.
CellId::Record(_) => Some(vec![ CellId::Record(_) | CellId::Coordinate { .. } => Some(vec![
Command::SetBuffer { Command::SetBuffer {
name: "edit".to_string(), name: "edit".to_string(),
value: cell.edit_text.clone().unwrap_or_default(), value: cell.edit_text.clone().unwrap_or_default(),
@@ -761,7 +761,17 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
// Both creating columns are records-only, so the guard above takes // Both creating columns are records-only, so the guard above takes
// them; declining rather than `None` keeps that a door, not a hole. // them; declining rather than `None` keeps that a door, not a hole.
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]), CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
CellId::Coordinate => Some(vec![Command::SetStatus(not_yet("renaming a coordinate"))]), }
}
/// The record and category of the coordinate cell under the cursor, as the
/// projection names them — never borrowed from a sibling cell (ADR 002).
pub(crate) fn coordinate_at_cursor(state: &ClientState) -> Option<(crate::RecordHash, String)> {
match cell_at_cursor(state)?.id.as_ref()? {
CellId::Coordinate { record, category } => Some((*record, category.clone())),
CellId::Record(_) | CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => {
None
}
} }
} }
@@ -869,7 +879,7 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
return create_column(state, edit_mode, advance, kind, &text); return create_column(state, edit_mode, advance, kind, &text);
} }
if state.grid.records { if state.grid.records {
return None; return rename_coordinate(state, edit_mode, advance, id, &text);
} }
let mut commands = vec![Command::Model { let mut commands = vec![Command::Model {
basis: state.basis.claim([record_at_cursor(state)?])?, basis: state.basis.claim([record_at_cursor(state)?])?,
@@ -880,6 +890,38 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
Some(commands) Some(commands)
} }
/// `CommitAndAdvance` over a coordinate: rename the record the editor OPENED on,
/// at the version it was shown — `context/code/clients.md` (improvise-fdk1).
fn rename_coordinate(
state: &ClientState,
edit_mode: AppMode,
advance: Advance,
id: &CellId,
text: &str,
) -> Option<Vec<Command>> {
let CellId::Coordinate { .. } = id else {
return None;
};
let Some(opened) = state.opened.as_ref() else {
return Some(vec![Command::SetStatus(
"still waiting for the host to answer your last edit".to_string(),
)]);
};
let mut commands = vec![Command::Model {
basis: state
.basis
.claim_as_read([(opened.record, opened.version)])?,
cmd: ModelCommand::RenameRecordCoordinate {
record: opened.record,
category: opened.category.clone(),
item: text.to_string(),
},
}];
commands.push(Command::ChangeMode(AppMode::Normal));
commands.extend(advanced(state, edit_mode, advance)?);
Some(commands)
}
/// `CommitAndAdvance` over a creating column: a schema change over the typed /// `CommitAndAdvance` over a creating column: a schema change over the typed
/// NAME. Why +cat does not advance: clients.md (improvise-gcvb). /// NAME. Why +cat does not advance: clients.md (improvise-gcvb).
fn create_column( fn create_column(
@@ -955,7 +997,10 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> { fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {
match cell_at_cursor(state)?.id.as_ref()? { match cell_at_cursor(state)?.id.as_ref()? {
CellId::Record(cell) => Some(cell.record), CellId::Record(cell) => Some(cell.record),
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) | CellId::Coordinate => { CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate { .. } => {
None None
} }
} }
@@ -1024,7 +1069,10 @@ fn model(state: &ClientState, cmd: ModelCommand) -> Option<Command> {
fn record_cell_at_cursor(state: &ClientState) -> Option<RecordCell> { fn record_cell_at_cursor(state: &ClientState) -> Option<RecordCell> {
match cell_at_cursor(state)?.id.as_ref()? { match cell_at_cursor(state)?.id.as_ref()? {
CellId::Record(cell) => Some(cell.clone()), CellId::Record(cell) => Some(cell.clone()),
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) | CellId::Coordinate => { CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate { .. } => {
None None
} }
} }
+1 -1
View File
@@ -175,7 +175,7 @@ impl Client {
CellId::Aggregate(_) CellId::Aggregate(_)
| CellId::Derived { .. } | CellId::Derived { .. }
| CellId::Create(_) | CellId::Create(_)
| CellId::Coordinate => None, | CellId::Coordinate { .. } => None,
} }
} }
+5 -2
View File
@@ -150,7 +150,7 @@ impl RenderCell {
CellId::Aggregate(_) CellId::Aggregate(_)
| CellId::Derived { .. } | CellId::Derived { .. }
| CellId::Create(_) | CellId::Create(_)
| CellId::Coordinate, | CellId::Coordinate { .. },
) )
| None => false, | None => false,
}, },
@@ -638,7 +638,10 @@ mod tests {
((1, 0), CellDisplay::text("pending")), ((1, 0), CellDisplay::text("pending")),
( (
(1, 2), (1, 2),
CellDisplay::text("North").with_id(CellId::Coordinate), CellDisplay::text("North").with_id(CellId::Coordinate {
record: surrogate(),
category: "Region".into(),
}),
), ),
])), ])),
], ],
@@ -803,7 +803,7 @@ fn rename_north(racing: bool) -> (Host, Vec<Addressed>) {
deliver(&mut a, &[a_snap]); deliver(&mut a, &[a_snap]);
deliver(&mut b, &[b_snap]); deliver(&mut b, &[b_snap]);
let mut press_a = |host: &mut Host, a: &mut Client, b: &mut Client, key: &str| { let press_a = |host: &mut Host, a: &mut Client, b: &mut Client, key: &str| {
let Some(frame) = a.key(&BrowserKey::new(key)) else { let Some(frame) = a.key(&BrowserKey::new(key)) else {
return Vec::new(); return Vec::new();
}; };