feat(command): the record tier and the dirty flag read the cell delta (improvise-5r8c)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
committed by
Edward Langley
parent
5c7a12fd3d
commit
9dd78a1a9e
Generated
+2
@@ -907,6 +907,7 @@ dependencies = [
|
||||
"improvise-core",
|
||||
"improvise-formula",
|
||||
"improvise-gp",
|
||||
"improvise-incremental",
|
||||
"improvise-io",
|
||||
"improvise-protocol",
|
||||
"serde",
|
||||
@@ -959,6 +960,7 @@ dependencies = [
|
||||
"improvise-core",
|
||||
"improvise-io",
|
||||
"proptest",
|
||||
"zset",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
+19
-18
@@ -1267,26 +1267,27 @@ gotcha 1: renaming a category coordinate in wide records mode moves *every*
|
||||
measure cell in the row, so one legal edit spans a whole record. **A unit finer
|
||||
than the coarsest legal write cannot be maintained.**
|
||||
|
||||
### `record_fingerprints` — a process-local XOR fold
|
||||
### The record tier and the dirty flag read the cell delta (improvise-5r8c)
|
||||
|
||||
A fingerprint per record of the raw cells it holds, keyed by the same surrogate
|
||||
the index and the version counter are keyed by. One `u64` folded over each cell's
|
||||
key and value. The fold is XOR, so it does not care what order `iter_cells`
|
||||
yields — and it cannot cancel a pair against itself, because a key appears at
|
||||
most once in the store.
|
||||
`record_fingerprints` (a per-record XOR of cell hashes) and `document_content` (a hash of
|
||||
every cell) are gone: both walked the whole store per batch. `apply_effects` now turns on
|
||||
`DataStore::record_changes` when a batch holds a `CellData` effect, drains the log after
|
||||
each `CellData` effect into a `CellDelta` (`improvise-incremental`, ADR 009 clause 3), and
|
||||
at the end of the batch drains what is left. The tail matters: a geometry effect such as
|
||||
`RemoveCategory` removes cells, and no later `CellData` effect may be there to drain them.
|
||||
|
||||
This hasher is **not** the surrogate's: it is process-local, compared only against
|
||||
a fingerprint taken moments earlier in the same process, and never crosses a wire.
|
||||
`RecordHash` is the opposite case and is specified accordingly
|
||||
(`improvise_core::model::record`).
|
||||
|
||||
The value hash is exhaustive and by **bits** rather than by `Debug`: two values
|
||||
that print alike must not fingerprint alike, and it runs once per cell per editing
|
||||
batch.
|
||||
|
||||
Only *raw* data is fingerprinted. Formula results live in the model's separate
|
||||
`formula_cache`, and a record whose derived value moved because someone edited a
|
||||
different record has not itself been written.
|
||||
- **Record tier.** The batch delta, consolidated across effects, names the records to bump
|
||||
(`moved_records`). A cell set and set back inside one batch cancels, as the XOR did.
|
||||
- **Dirty flag.** A `CellData` effect dirties the document when its delta is non-zero or
|
||||
`document_shape` moved. The shape is the formulas, `document_config` and
|
||||
`DataStore::reorders`. ADR 009 names only the formula fingerprint; two more are needed.
|
||||
`AddItem` writes no cell and moves the config. `SortData` writes no cell either and
|
||||
moves the saved order, which no delta carries, so the store counts the sorts that moved
|
||||
a cell.
|
||||
- **Float rules.** `0.0` over `-0.0`, and one NaN over another, are no longer writes: the
|
||||
old fingerprint hashed bits and counted them.
|
||||
- **Cost.** A cell write is O(delta) plus `document_shape`, which is O(formulas and
|
||||
config), not O(cells).
|
||||
|
||||
### `document_generation`, `layout_version`, `cursor_epoch`
|
||||
|
||||
|
||||
@@ -55,8 +55,38 @@ fold of the cells that match a partial key") and nothing else. These are the rea
|
||||
6. **Values agree by float rules with every NaN one value** (`same_value`), the rule clause 3
|
||||
states for cell deltas. Signed zero compares equal, so a later `ExactSum` cannot be told
|
||||
from eager's sum by the sign of a zero here; clause 8 has its own property test for that.
|
||||
7. **No `zset` dependency yet.** Nothing in this step uses a Z-set. It arrives with the first
|
||||
backend, as the cell delta of step 3.
|
||||
7. **The `zset` dependency arrived with step 3**, as the cell delta (below).
|
||||
|
||||
## Step 3: the cell delta (improvise-5r8c)
|
||||
|
||||
`delta.rs` holds `DeltaValue`, `CellDelta = ZSet<(CellKey, DeltaValue)>`, `cell_delta` and
|
||||
`moved_records`. `improvise-command`'s `apply_effects` reads them for the record tier and the
|
||||
dirty flag (`context/code/command.md`). Evidence: `context/test-evidence/improvise-incremental.md`.
|
||||
These are the readings taken, where ADR 009 clause 3 leaves a choice.
|
||||
|
||||
1. **`DataStore` logs, `improvise-incremental` builds the Z-set.** The ADR says the store
|
||||
"emits a `ZSet`", but core never depends on `zset` (clause 2). `DataStore::record_changes`
|
||||
turns on a `Vec<CellChange>` log (`None` while off, so an unwatched store keeps nothing),
|
||||
`drain_changes` empties it and `cell_delta` folds it. The log lives in the store, not in
|
||||
`Model::set_cell`, so every route that writes a cell is seen, including `RemoveCategory`'s
|
||||
sweep.
|
||||
2. **`DeltaValue` canonicalises on construction**: every NaN becomes `f64::NAN`, `-0.0` becomes
|
||||
`+0.0`, and the order is `total_cmp` on the result, with numbers before text before errors.
|
||||
The inner value is private, so a non-canonical one cannot be built. Equality is the order's
|
||||
`Equal`, so `==` and `Ord` agree.
|
||||
3. **`CellKey` is `Ord` by derivation**: the sorted coordinate pairs, compared as strings.
|
||||
Clause 3 calls this "the natural order of ids", which ADR 007's id form will change; the
|
||||
derive then moves with the key type.
|
||||
4. **The record tier is the set of records a consolidated batch delta names.** A cell written
|
||||
and written back inside one batch cancels, as the old XOR did.
|
||||
5. **The dirty flag needs two inputs the ADR does not name.** Besides a non-empty delta and
|
||||
the formula set, `document_config` stays (an `AddItem` writes no cell and the existing test
|
||||
requires it dirty) and `DataStore::reorders` joins (`SortData` reorders the saved cells and
|
||||
no delta carries order).
|
||||
6. **`document_generation` and the wire are untouched**, and `Eager` and the `Derived` trait
|
||||
are not used: no backend consumes the delta yet. The differential for this step is the
|
||||
delta against the store it describes (`the_cell_delta_of_an_edit_is_...`), since there is
|
||||
nothing to compare with `Eager`.
|
||||
|
||||
## What the harness checks
|
||||
|
||||
|
||||
@@ -7227,3 +7227,37 @@ group per level, where the two keyings coincide.
|
||||
`z` through a closed Year > Quarter > Month hierarchy, cursor at row 1. Rival:
|
||||
`toggle_group_collapse` never removing a fold; the final assertion (no fold left
|
||||
behind once the quarter is closed again) fails.
|
||||
|
||||
### basis_tiers::a_write_that_compares_equal_by_the_float_rules_changes_nothing
|
||||
|
||||
improvise-5r8c. `0.0` over `-0.0`, `-0.0` over `0.0` and three NaN spellings move no record
|
||||
version and leave the document clean. It failed on the bit fingerprint (`0 -> -0` bumped the
|
||||
version to 2), the bug the ADR names. Faults: dirty set on every `CellData` effect fails it
|
||||
and two more.
|
||||
|
||||
### basis_tiers::a_write_that_differs_by_the_float_rules_still_changes_something
|
||||
|
||||
`0.0` over NaN bumps the version and dirties. Fault: dirty read from the shape alone ignores
|
||||
the delta and fails this and four tests elsewhere.
|
||||
|
||||
### basis_tiers::cells_removed_by_a_later_geometry_effect_still_move_their_records
|
||||
|
||||
`[SetCell, RemoveCategory]`: the removal comes after the last `CellData` drain. Fault: not
|
||||
reading the log at the end of the batch survived the whole suite until this test, and then
|
||||
fails it. A fault dropping the per-effect delta from the batch fails ten tests.
|
||||
|
||||
### effect::tests::a_cell_data_effect_no_raw_cell_records_still_dirties_the_document
|
||||
|
||||
Predates this step. `SortData` moves the saved order and no cell delta reports it, so the
|
||||
first cell-delta version of the dirty flag failed this test. `DataStore::reorders` fixes it,
|
||||
and removing it from `document_shape` fails this test alone.
|
||||
|
||||
### model::cell::data_store::sorting_counts_as_a_reorder_only_when_it_moves_a_cell
|
||||
|
||||
A counter bumped on every sort, and one never bumped, each fail it: the second sort of an
|
||||
already sorted store is the case that separates them from the first.
|
||||
|
||||
### model::cell::data_store::a_recording_store_reports_a_new_cell_an_overwrite_and_a_removal
|
||||
|
||||
An overwrite logging `old: None` fails it. Removing an absent cell must log nothing, and a
|
||||
second drain is empty.
|
||||
|
||||
@@ -192,3 +192,26 @@ Clause 2 asks for a property test over a random edit sequence. A proptest (256 c
|
||||
edits of five kinds over the hand-built model) runs `differential` with a candidate that rebuilds
|
||||
an `Eager` from scratch on every edit. It fails on the `Eager::apply`-skips-recompute fault, which
|
||||
is the property: maintaining in place equals building afresh.
|
||||
|
||||
## delta::tests::overwriting_a_value_with_one_that_compares_equal_is_the_zero_delta
|
||||
|
||||
Every pair of NaN spellings (positive, negative, two payloads), `0.0` over `-0.0` and back,
|
||||
and equal text each give the zero delta. Faults: `DeltaValue::new` not mapping NaN to the
|
||||
canonical one, and not mapping `-0.0` to `+0.0`; each fails this test and two others.
|
||||
|
||||
## delta::tests::numbers_order_by_total_cmp_with_the_canonical_nan_last
|
||||
|
||||
Comparing by bits instead of `f64::total_cmp` passes every equality test, since the values
|
||||
are canonical, and fails only here, where negatives must sort below positives.
|
||||
|
||||
## delta::tests::values_that_differ_by_the_float_rules_stay_different
|
||||
|
||||
Guards the other direction: a `cmp` returning `Equal` across variants (`0` against `"0"`)
|
||||
fails this and the totality test. A weight of `+1` for the old value fails four tests.
|
||||
|
||||
## the_cell_delta_of_an_edit_is_the_store_after_minus_the_store_before
|
||||
|
||||
The differential for step 3. A proptest (256 cases, up to 12 edits drawn from sets, clears,
|
||||
both zeros and three NaN spellings) checks that the delta `DataStore` logs for an edit equals
|
||||
the store as a Z-set after it minus the store before. No `Derived` backend exists yet, so
|
||||
this is the harness's step-3 form: the delta against the store it describes.
|
||||
|
||||
@@ -12,6 +12,8 @@ improvise-protocol = { path = "../improvise-protocol" }
|
||||
improvise-formula = { path = "../improvise-formula" }
|
||||
improvise-io = { path = "../improvise-io" }
|
||||
improvise-gp = { path = "../improvise-gp" }
|
||||
# The cell delta behind the record tier and the dirty flag (ADR 009 step 3).
|
||||
improvise-incremental = { path = "../improvise-incremental" }
|
||||
anyhow = "1"
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
# Parses JSON a user's numbers cross, so it must read back the f64 written: per crate,
|
||||
|
||||
@@ -19,7 +19,7 @@ use crate::command::cmd::CmdContext;
|
||||
use crate::command::keymap::{Keymap, KeymapSet};
|
||||
use crate::format::FormatPlan;
|
||||
use crate::import::wizard::ImportWizard;
|
||||
use crate::model::cell::{CellKey, CellValue, DataStore};
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::persistence;
|
||||
use crate::ui::effect::{EffectKind, ModelChange, ViewTarget};
|
||||
use crate::ui::project::Dirty;
|
||||
@@ -29,6 +29,7 @@ use crate::ui::viewport::{
|
||||
use crate::view::{GridLayout, View};
|
||||
use crate::workbook::Workbook;
|
||||
use improvise_core::model::record::{AggregateHash, RecordHash};
|
||||
use improvise_incremental::{CellDelta, cell_delta, moved_records};
|
||||
use improvise_protocol::{CellId, RecordCell};
|
||||
|
||||
/// A cell's coordinate tuple with `_Measure` dropped: the unit of atomicity
|
||||
@@ -165,15 +166,12 @@ fn view_shapes(workbook: &Workbook) -> std::collections::BTreeMap<String, u64> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// `document_config` plus the cells and the formulas. `CellData` is wider than its
|
||||
/// name (improvise-748f), and only that arm pays the walk — context/code/command.md.
|
||||
fn document_content(workbook: &Workbook) -> u64 {
|
||||
/// `document_config` plus the formulas: everything `CellData` can move besides the
|
||||
/// cells, which the cell delta reports (improvise-5r8c, context/code/command.md).
|
||||
fn document_shape(workbook: &Workbook) -> u64 {
|
||||
use std::hash::{Hash, Hasher};
|
||||
let mut h = std::collections::hash_map::DefaultHasher::new();
|
||||
for (key, value) in workbook.model.data.iter_cells() {
|
||||
key.hash(&mut h);
|
||||
hash_value(value, &mut h);
|
||||
}
|
||||
workbook.model.data.reorders().hash(&mut h);
|
||||
for formula in workbook.model.formulas() {
|
||||
formula.raw.hash(&mut h);
|
||||
formula.target.hash(&mut h);
|
||||
@@ -243,40 +241,6 @@ fn sorted_pairs(
|
||||
out
|
||||
}
|
||||
|
||||
/// Exhaustive, and by bits rather than by `Debug`: two values that print alike
|
||||
/// must not fingerprint alike.
|
||||
fn hash_value(value: &CellValue, hasher: &mut impl std::hash::Hasher) {
|
||||
use std::hash::Hash;
|
||||
match value {
|
||||
CellValue::Number(n) => {
|
||||
0u8.hash(hasher);
|
||||
n.to_bits().hash(hasher);
|
||||
}
|
||||
CellValue::Text(t) => {
|
||||
1u8.hash(hasher);
|
||||
t.hash(hasher);
|
||||
}
|
||||
CellValue::Error(e) => {
|
||||
2u8.hash(hasher);
|
||||
e.hash(hasher);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A per-record XOR fingerprint of the **raw** cells it holds. Process-local and
|
||||
/// never on a wire, unlike [`RecordHash`] — see `context/code/command.md`.
|
||||
fn record_fingerprints(data: &DataStore) -> std::collections::HashMap<RecordHash, u64> {
|
||||
use std::hash::{Hash, Hasher};
|
||||
let mut out: std::collections::HashMap<RecordHash, u64> = std::collections::HashMap::new();
|
||||
for (key, value) in data.iter_cells() {
|
||||
let mut hasher = std::collections::hash_map::DefaultHasher::new();
|
||||
key.hash(&mut hasher);
|
||||
hash_value(value, &mut hasher);
|
||||
*out.entry(RecordHash::of(&key)).or_insert(0) ^= hasher.finish();
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// Resolution against the grid each view is drawn, each derived at most once per
|
||||
/// borrow of the `App` (improvise-kimw) — `context/code/command.md`.
|
||||
pub struct Grids<'a> {
|
||||
@@ -1083,12 +1047,14 @@ impl App {
|
||||
let moves_geometry = effects
|
||||
.iter()
|
||||
.any(|e| matches!(e.kind(), EffectKind::Model(ModelChange::LayoutGeometry)));
|
||||
let records_before =
|
||||
writes_cells.then(|| record_fingerprints(&self.model_state.workbook.model.data));
|
||||
if writes_cells {
|
||||
self.model_state.workbook.model.data.record_changes();
|
||||
}
|
||||
let mut batch_delta = CellDelta::new();
|
||||
let shapes_before = touches_model.then(|| view_shapes(&self.model_state.workbook));
|
||||
// Two bases, because the cheap one is the held-key path: `[`/`]`, axis
|
||||
// cycle and hide/show must not walk 18,000 cells (improvise-748f).
|
||||
let mut content_basis = writes_cells.then(|| document_content(&self.model_state.workbook));
|
||||
let mut shape_basis = writes_cells.then(|| document_shape(&self.model_state.workbook));
|
||||
let mut config_basis = moves_geometry.then(|| document_config(&self.model_state.workbook));
|
||||
// The cursor's own basis. The cheap one-view form on the held-key path,
|
||||
// and every view where a cell write can prune one — `context/code/command.md`.
|
||||
@@ -1119,11 +1085,13 @@ impl App {
|
||||
// The observed half, read **inside the loop** so `Save` still clears it
|
||||
// in loop order (gotcha 45) — the position is the property.
|
||||
if matches!(kind, EffectKind::Model(ModelChange::CellData))
|
||||
&& let Some(basis) = content_basis
|
||||
&& let Some(basis) = shape_basis
|
||||
{
|
||||
let now = document_content(&self.model_state.workbook);
|
||||
self.model_state.dirty |= now != basis;
|
||||
content_basis = Some(now);
|
||||
let delta = cell_delta(&self.model_state.workbook.model.data.drain_changes());
|
||||
let now = document_shape(&self.model_state.workbook);
|
||||
self.model_state.dirty |= !delta.is_zero() || now != basis;
|
||||
batch_delta += delta;
|
||||
shape_basis = Some(now);
|
||||
}
|
||||
if matches!(kind, EffectKind::Model(ModelChange::LayoutGeometry))
|
||||
&& let Some(basis) = config_basis
|
||||
@@ -1177,8 +1145,11 @@ impl App {
|
||||
break;
|
||||
}
|
||||
}
|
||||
if let Some(before) = records_before {
|
||||
self.bump_changed_records(&before);
|
||||
if writes_cells {
|
||||
let data = &mut self.model_state.workbook.model.data;
|
||||
batch_delta += cell_delta(&data.drain_changes());
|
||||
data.stop_recording_changes();
|
||||
self.bump_moved_records(&batch_delta);
|
||||
}
|
||||
self.observe_written_view_frames();
|
||||
self.observe_reshape(shapes_before);
|
||||
@@ -1206,20 +1177,11 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
/// Bump every record whose raw cells read differently than at `before`. Gained,
|
||||
/// lost and rewritten all count — each moves under an older basis.
|
||||
fn bump_changed_records(&mut self, before: &std::collections::HashMap<RecordHash, u64>) {
|
||||
/// Bump every record the batch's cell delta touches. Gained, lost and rewritten
|
||||
/// all count, each moves under an older basis; a record is bumped once.
|
||||
fn bump_moved_records(&mut self, delta: &CellDelta) {
|
||||
let data = &mut self.model_state.workbook.model.data;
|
||||
let after = record_fingerprints(data);
|
||||
// A set, not a list: a record present in both maps is named twice by the
|
||||
// chain, and a rewritten record would otherwise count as two writes.
|
||||
let moved: std::collections::HashSet<RecordHash> = before
|
||||
.keys()
|
||||
.chain(after.keys())
|
||||
.filter(|record| before.get(*record) != after.get(*record))
|
||||
.copied()
|
||||
.collect();
|
||||
for record in moved {
|
||||
for record in moved_records(delta) {
|
||||
data.bump_record(record);
|
||||
}
|
||||
}
|
||||
@@ -5231,6 +5193,71 @@ mod basis_tiers {
|
||||
assert_eq!(app.record_version(&record("North")), 0);
|
||||
}
|
||||
|
||||
/// Clause 3 of ADR 009: values compare by the float rules, so `-0.0` over
|
||||
/// `0.0` and one NaN over another are not changes (the old scan hashed bits).
|
||||
#[test]
|
||||
fn a_write_that_compares_equal_by_the_float_rules_changes_nothing() {
|
||||
let same = [
|
||||
(0.0, -0.0),
|
||||
(-0.0, 0.0),
|
||||
(f64::NAN, -f64::NAN),
|
||||
(f64::NAN, f64::from_bits(0x7ff0_0000_0000_0001)),
|
||||
];
|
||||
for (was, now) in same {
|
||||
let mut app = app_with_two_records();
|
||||
let north = cell("North", "Revenue");
|
||||
app.apply_effects(vec![Box::new(SetCell(
|
||||
north.clone(),
|
||||
CellValue::Number(was),
|
||||
))]);
|
||||
app.model_state.dirty = false;
|
||||
let version = app.record_version(&record("North"));
|
||||
assert_eq!(version, 1, "the fixture write must register");
|
||||
|
||||
app.apply_effects(vec![Box::new(SetCell(north, CellValue::Number(now)))]);
|
||||
|
||||
assert_eq!(
|
||||
app.record_version(&record("North")),
|
||||
version,
|
||||
"{was} -> {now}"
|
||||
);
|
||||
assert!(!app.model_state.dirty, "{was} -> {now}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Cells a non-`CellData` effect removes, in a batch that also writes cells, are
|
||||
/// read at the end of the batch: no later `CellData` effect drains them.
|
||||
#[test]
|
||||
fn cells_removed_by_a_later_geometry_effect_still_move_their_records() {
|
||||
let mut app = app_with_two_records();
|
||||
|
||||
app.apply_effects(vec![
|
||||
Box::new(SetCell(cell("North", "Revenue"), CellValue::Number(99.0))),
|
||||
Box::new(crate::ui::effect::RemoveCategory("Month".into())),
|
||||
]);
|
||||
|
||||
assert_eq!(app.model_state.workbook.model.data.iter_cells().count(), 0);
|
||||
assert_eq!(app.record_version(&record("South")), 1, "lost both cells");
|
||||
}
|
||||
|
||||
/// The other direction, which the equal-value test cannot see: a real
|
||||
/// change still dirties the document and moves the record.
|
||||
#[test]
|
||||
fn a_write_that_differs_by_the_float_rules_still_changes_something() {
|
||||
let mut app = app_with_two_records();
|
||||
let north = cell("North", "Revenue");
|
||||
app.apply_effects(vec![Box::new(SetCell(
|
||||
north.clone(),
|
||||
CellValue::Number(0.0),
|
||||
))]);
|
||||
app.model_state.dirty = false;
|
||||
|
||||
app.apply_effects(vec![Box::new(SetCell(north, CellValue::Number(f64::NAN)))]);
|
||||
|
||||
assert_eq!(app.record_version(&record("North")), 2);
|
||||
assert!(app.model_state.dirty);
|
||||
}
|
||||
|
||||
/// The counters are shared document state on `App`, never on the portable
|
||||
/// session slice (gotcha 30).
|
||||
#[test]
|
||||
|
||||
@@ -227,6 +227,7 @@ pub struct DataStore {
|
||||
/// The changes since `record_changes`, or `None` while nobody listens, so an
|
||||
/// unwatched store keeps no log.
|
||||
changes: Option<Vec<CellChange>>,
|
||||
reorders: u64,
|
||||
}
|
||||
|
||||
/// Resolved through the symbol table and sorted wherever the storage is a
|
||||
@@ -240,6 +241,7 @@ impl std::fmt::Debug for DataStore {
|
||||
records,
|
||||
arities: _,
|
||||
changes: _,
|
||||
reorders: _,
|
||||
} = self;
|
||||
let cells: Vec<_> = cells
|
||||
.iter()
|
||||
@@ -290,6 +292,18 @@ impl DataStore {
|
||||
/// Sort cells by `CellKey`. Called once on entry into records mode;
|
||||
/// subsequent inserts append at the end.
|
||||
pub fn sort_by_key(&mut self) {
|
||||
let before: Vec<InternedKey> = self.cells.keys().cloned().collect();
|
||||
self.sort_cells();
|
||||
self.reorders += u64::from(self.cells.keys().ne(before.iter()));
|
||||
}
|
||||
|
||||
/// How many times `sort_by_key` has moved a cell. Order is saved, so a reorder
|
||||
/// is a change no [`CellChange`] reports.
|
||||
pub fn reorders(&self) -> u64 {
|
||||
self.reorders
|
||||
}
|
||||
|
||||
fn sort_cells(&mut self) {
|
||||
let symbols = &self.symbols;
|
||||
self.cells.sort_by(|a, _, b, _| {
|
||||
let resolve = |k: &InternedKey| -> Vec<(String, String)> {
|
||||
@@ -851,6 +865,18 @@ mod data_store {
|
||||
assert!(store.drain_changes().is_empty(), "a drain empties the log");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sorting_counts_as_a_reorder_only_when_it_moves_a_cell() {
|
||||
let mut store = DataStore::new();
|
||||
store.set(key(&[("Region", "West")]), CellValue::Number(1.0));
|
||||
store.set(key(&[("Region", "East")]), CellValue::Number(2.0));
|
||||
assert_eq!(store.reorders(), 0);
|
||||
store.sort_by_key();
|
||||
assert_eq!(store.reorders(), 1, "West, East became East, West");
|
||||
store.sort_by_key();
|
||||
assert_eq!(store.reorders(), 1, "already sorted: nothing moved");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_store_that_stopped_recording_reports_nothing_more() {
|
||||
let mut store = DataStore::new();
|
||||
|
||||
Reference in New Issue
Block a user