refactor: inject the storage backend into App (improvise-fuj)
The ticket named two effects reading files directly, but that was the visible symptom. App had no storage backend at all, so Save, SaveAs, ExportCsv, LoadModel and CSV import all hardwired the filesystem through persistence and csv_parser. App now holds a Box<dyn Storage> chosen by the host at construction. App::new still supplies PhysicalFs, so every existing caller is untouched; App::with_storage takes any other backend. A browser worker constructs App with an OPFS backend and nothing else changes. The alternatives on the ticket were a parameter on each effect, which spreads per-session config through every signature, or a thread-local, which hides a dependency App is the natural owner of — it already holds the keymap set and terminal size. improvise-io keeps its path-taking wrappers so the native CLI is unchanged, each delegating to a backend- or bytes-taking variant: export_csv gains export_csv_to over a render_csv that produces bytes, and parse_csv_with_warnings gains parse_csv_bytes_with_warnings. Production code in improvise-command now contains no std::fs. The test is the point rather than the refactor: it drives SaveAs, ExportCsv, LoadModel and StartImportWizard over a MemoryFs with no disk touched, then inspects the backend to confirm the bytes landed there. Any of those effects reaching for std::fs fails it — the property a browser backend depends on, verified now instead of discovered later in wasm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
This commit is contained in:
Edward Langley
+23
-21
@@ -139,27 +139,28 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
|
||||
## Gotchas (read before editing)
|
||||
|
||||
1. **Records mode is wide format.** A row is one *coordinate tuple* — the cells sharing everything but `_Measure` — and each raw measure gets its own editable column, followed by a `+` column that creates a new measure (improvise-c5v). Consequences: two measures on one coordinate are ONE row; editing a category column moves **every** measure cell in the row, not just the one under the cursor; `_Dim` in the synthetic key carries the column **index**, not a label, because a category and a measure may share a name — resolve it with `GridLayout::record_col` into a `RecordCol`. Formula targets are not columns (nothing to type into them). Drill pending edits are keyed by `(row, RecordCol)`.
|
||||
2. **Every effect declares an `EffectKind`, and there is no default.** `ViewOnly` never reaches other clients; `Model(CellData | LayoutGeometry | WholeWorkbook)` does (improvise-mae). A new effect that forgets to classify itself fails to compile — that is deliberate, since the failure mode is silent: a mis-declared `ViewOnly` edit would simply never reach connected clients. Watch two traps: the *View* lives inside `model_state.workbook`, so "touches model_state" does NOT mean "is a model change" (axis/page/format are `LayoutGeometry`, cursor and scroll are `ViewOnly` — see improvise-8uz); and `Save`/`SaveAs`/`MarkDirty` touch `model_state` for bookkeeping only and are `ViewOnly`.
|
||||
3. **Commands never mutate.** `&CmdContext` is read-only; return `Vec<Box<dyn Effect>>`. If you want to touch `&mut App`, add or extend an Effect.
|
||||
4. **`CellKey` is always sorted.** Use the constructors; equality and hashing rely on canonical form.
|
||||
5. **No `Model::add_item`.** With a `Workbook` in hand use the facade: `wb.add_item("Region", "East")?` / `add_item_in_group` / `remove_item` (improvise-tvt). With only a `Model`, go through the category: `m.category_mut("Region").unwrap().add_item("East")`.
|
||||
6. **Virtual categories.** `_Measure` items = explicit items ∪ formula targets. Use `measure_item_names()` / `effective_item_names("_Measure")`. `add_formula` does *not* add items to `_Measure`. Formula-target lookup also falls back into `_Measure` via `Model::find_item_category`. Cell aggregation goes through `apply_agg`, formula `SUM(…)`/`AVG(…)` through `eval_agg` — they differ on empty input (absent vs. `0` for Sum/Count, error for Avg/Min/Max).
|
||||
7. **Display rounding is view-only.** `format_f64` (half-away-from-zero) is only called in rendering. Formula eval uses full `f64`. Never feed a rounded value back into eval.
|
||||
8. **Formula eval is fixed-point.** `recompute_formulas(none_cats)` iterates until stable, bounded by `MAX_EVAL_DEPTH`; circular refs converge to `CellValue::Error("circular")`. `App::new` runs it before the first frame so formula cells render on startup.
|
||||
9. **Drill into formula cells** strips the `_Measure` coordinate when it names a formula target, so `matching_cells` returns the raw records that feed the formula instead of returning empty.
|
||||
10. **Minibuffer keymaps come from `TextEntry`.** The seven text-entry modes are built by the `TextEntry` builder in `keymap.rs`, which binds the shared spine (AnyChar→`append-char <buf>`, Backspace→`pop-char <buf>`, Esc→`[clear-buffer <buf>, enter-mode <exit>]`) and welds every commit key to its `clear-buffer` via `.commit(key, cmd, args)` (improvise-w3q). `TextEntry::new` takes the `AppMode` and reads `<buf>` off its `MinibufferConfig`, so the buffer the prompt names and the buffer the keys edit are the same string by construction (improvise-k8h) — the keymap never spells a buffer name. `.rebind` exists only for CommandMode's Backspace. `SearchMode` is hand-written on purpose — no commit key, and `exit-search-mode` owns the query.
|
||||
11. **Panel keymaps come from `PanelKeymap`.** The three panel modes share a spine (Esc/Tab→Normal, arrows and j/k→`move-panel-cursor <panel>`, every panel's toggle key, `:`→command mode) built by `PanelKeymap::new(panel)`; the keys that act on the panel's contents arrive through `.bind`/`.bind_args`/`.delete` because they vary in shape, not just command name (improvise-4pg). `Panel::ALL`/`name()`/`key()` in `ui/app.rs` drive the toggle keys for Normal and all three panels, so a fourth panel reaches every keymap from one `ALL` entry. `name()` must spell what `parse_panel` accepts.
|
||||
12. **Prefix keys are stored as an address, not a keymap.** `ViewState::transient_prefix` holds the `(KeyCode, KeyModifiers)` that opened a prefix; `KeymapSet::prefix_keymap` resolves it back to the sub-keymap, walking parents so `RecordsNormal` reaches Normal's `g`/`y`/`Z` (improvise-jc8). `ViewState` must stay free of `improvise-command` types — it is destined for the protocol crate, which `improvise-command` will depend on. Resolution assumes prefixes are **flat**; `prefix_keymaps_are_flat` fails the build if a nested one is added.
|
||||
13. **Keybindings are per-mode.** `ModeKey::from_app_mode()` resolves the mode; Normal + `search_mode=true` maps to `SearchMode`. Minibuffer modes bind Enter/Esc as `Binding::Sequence` so `clear-buffer` fires alongside commit/cancel.
|
||||
14. **Registry names come from prototypes, not strings.** Every `register*` call reads the name off a prototype `Cmd` instance, so a typo is impossible (improvise-9cn) — there is no name literal in `registry.rs`. What types can't reach: four commands name themselves off a field (`ViewNavigate`, `TileAxisOp`, `ToggleGroupAtCursor`, `CommitAndAdvance`), so a prototype can name the wrong *variant* and still compile. That shows up as a **duplicate** registration, not a mismatch — the wrong-variant entry collides with its sibling and the sibling's name vanishes. `registered_names_are_unique` catches it; `every_bound_command_name_is_registered` catches the keymap side.
|
||||
15. **`effect_cmd!` macro** wraps a pre-built effect as a parseable command. Use it for simple wrappers; reach for a hand-written `Cmd` once there's real decision logic.
|
||||
16. **Float equality inside formula eval is currently mixed** — `=`/`!=` use a `1e-10` epsilon but the div-by-zero guard checks `rv == 0.0` exactly. Don't assume one or the other; tracked for cleanup.
|
||||
17. **`IndexMap`** preserves insertion order for categories, views, and items. Persistence relies on this.
|
||||
18. **Commit paths for cell edits** split across `commit_cell_value` → `commit_regular_cell_value` / `commit_plain_records_edit` in `command/cmd/commit.rs`. Keep the synthetic-records branch in sync with the plain branch when changing behavior.
|
||||
19. **Minibuffer commits go through one command.** `commit-buffer <formula|category|item>` (`CommitBuffer { kind: CommitKind }`) owns buffer→trim→bail-on-empty→dirty→status for all three; the varying parts are methods on `CommitKind`. The add effect is emitted **last** so its failure message wins over the command's optimistic status (improvise-6t3). `CommitKind::Item` carries its target category, resolved from `AppMode::ItemAdd` by `parse_commit_kind` at registry time — so `commit-buffer item` is not constructible from a script line.
|
||||
20. **A failed `Save` aborts the batch.** The `Save` effect sets `abort_effects` when there's no file path or the write fails (improvise-8tk), so anything chained after it — the `Quit` in `wq`/`ZZ` — is skipped and the error stays on screen. A `Binding::Sequence` flattens into one effect vec, so the abort carries across sequence steps too.
|
||||
21. **The import wizard's back path is a history stack, not a reverse table.** `advance` pushes the step it leaves onto `ImportWizard::history`; `previous` pops it, so back always retraces the path actually taken through the conditional transitions (improvise-907). Preconditions live in `validation_error()`, checked on the step where the user can still fix them — never deferred to `build_model`. `NameModel` is terminal for `advance`: Enter there calls `build()`, which parks the workbook in `built` on `Done`, and `AcceptWizardModel` takes it from there. Going back from `Done` discards `built`.
|
||||
22. **`app.layout` goes stale mid-batch.** `apply_effects` rebuilds only *after* the whole batch, so an effect that reads the layout after an earlier effect changed the grid needs a `RebuildLayout` effect in front of it — the command decides the order (see `EnterEditAtCursorCmd`, `CommitAndAdvance`). Read the cursor's cell text via `App::display_value_at_cursor()`; don't build a `CmdContext` inside `apply` (it recomputes column widths).
|
||||
2. **Effects never touch `std::fs`.** Every document read or write goes through `App::storage: Box<dyn Storage>` (improvise-fuj). The host picks the backend: `App::new` supplies `PhysicalFs`, `App::with_storage` takes any other — a browser worker will supply OPFS (improvise-i34), and tests use `MemoryFs`. `improvise-io` keeps path-taking wrappers (`save`, `load`, `export_csv`, `parse_csv_with_warnings`) for the native CLI, each delegating to a `_to`/`_from`/`_bytes` variant that takes the backend or the bytes.
|
||||
3. **Every effect declares an `EffectKind`, and there is no default.** `ViewOnly` never reaches other clients; `Model(CellData | LayoutGeometry | WholeWorkbook)` does (improvise-mae). A new effect that forgets to classify itself fails to compile — that is deliberate, since the failure mode is silent: a mis-declared `ViewOnly` edit would simply never reach connected clients. Watch two traps: the *View* lives inside `model_state.workbook`, so "touches model_state" does NOT mean "is a model change" (axis/page/format are `LayoutGeometry`, cursor and scroll are `ViewOnly` — see improvise-8uz); and `Save`/`SaveAs`/`MarkDirty` touch `model_state` for bookkeeping only and are `ViewOnly`.
|
||||
4. **Commands never mutate.** `&CmdContext` is read-only; return `Vec<Box<dyn Effect>>`. If you want to touch `&mut App`, add or extend an Effect.
|
||||
5. **`CellKey` is always sorted.** Use the constructors; equality and hashing rely on canonical form.
|
||||
6. **No `Model::add_item`.** With a `Workbook` in hand use the facade: `wb.add_item("Region", "East")?` / `add_item_in_group` / `remove_item` (improvise-tvt). With only a `Model`, go through the category: `m.category_mut("Region").unwrap().add_item("East")`.
|
||||
7. **Virtual categories.** `_Measure` items = explicit items ∪ formula targets. Use `measure_item_names()` / `effective_item_names("_Measure")`. `add_formula` does *not* add items to `_Measure`. Formula-target lookup also falls back into `_Measure` via `Model::find_item_category`. Cell aggregation goes through `apply_agg`, formula `SUM(…)`/`AVG(…)` through `eval_agg` — they differ on empty input (absent vs. `0` for Sum/Count, error for Avg/Min/Max).
|
||||
8. **Display rounding is view-only.** `format_f64` (half-away-from-zero) is only called in rendering. Formula eval uses full `f64`. Never feed a rounded value back into eval.
|
||||
9. **Formula eval is fixed-point.** `recompute_formulas(none_cats)` iterates until stable, bounded by `MAX_EVAL_DEPTH`; circular refs converge to `CellValue::Error("circular")`. `App::new` runs it before the first frame so formula cells render on startup.
|
||||
10. **Drill into formula cells** strips the `_Measure` coordinate when it names a formula target, so `matching_cells` returns the raw records that feed the formula instead of returning empty.
|
||||
11. **Minibuffer keymaps come from `TextEntry`.** The seven text-entry modes are built by the `TextEntry` builder in `keymap.rs`, which binds the shared spine (AnyChar→`append-char <buf>`, Backspace→`pop-char <buf>`, Esc→`[clear-buffer <buf>, enter-mode <exit>]`) and welds every commit key to its `clear-buffer` via `.commit(key, cmd, args)` (improvise-w3q). `TextEntry::new` takes the `AppMode` and reads `<buf>` off its `MinibufferConfig`, so the buffer the prompt names and the buffer the keys edit are the same string by construction (improvise-k8h) — the keymap never spells a buffer name. `.rebind` exists only for CommandMode's Backspace. `SearchMode` is hand-written on purpose — no commit key, and `exit-search-mode` owns the query.
|
||||
12. **Panel keymaps come from `PanelKeymap`.** The three panel modes share a spine (Esc/Tab→Normal, arrows and j/k→`move-panel-cursor <panel>`, every panel's toggle key, `:`→command mode) built by `PanelKeymap::new(panel)`; the keys that act on the panel's contents arrive through `.bind`/`.bind_args`/`.delete` because they vary in shape, not just command name (improvise-4pg). `Panel::ALL`/`name()`/`key()` in `ui/app.rs` drive the toggle keys for Normal and all three panels, so a fourth panel reaches every keymap from one `ALL` entry. `name()` must spell what `parse_panel` accepts.
|
||||
13. **Prefix keys are stored as an address, not a keymap.** `ViewState::transient_prefix` holds the `(KeyCode, KeyModifiers)` that opened a prefix; `KeymapSet::prefix_keymap` resolves it back to the sub-keymap, walking parents so `RecordsNormal` reaches Normal's `g`/`y`/`Z` (improvise-jc8). `ViewState` must stay free of `improvise-command` types — it is destined for the protocol crate, which `improvise-command` will depend on. Resolution assumes prefixes are **flat**; `prefix_keymaps_are_flat` fails the build if a nested one is added.
|
||||
14. **Keybindings are per-mode.** `ModeKey::from_app_mode()` resolves the mode; Normal + `search_mode=true` maps to `SearchMode`. Minibuffer modes bind Enter/Esc as `Binding::Sequence` so `clear-buffer` fires alongside commit/cancel.
|
||||
15. **Registry names come from prototypes, not strings.** Every `register*` call reads the name off a prototype `Cmd` instance, so a typo is impossible (improvise-9cn) — there is no name literal in `registry.rs`. What types can't reach: four commands name themselves off a field (`ViewNavigate`, `TileAxisOp`, `ToggleGroupAtCursor`, `CommitAndAdvance`), so a prototype can name the wrong *variant* and still compile. That shows up as a **duplicate** registration, not a mismatch — the wrong-variant entry collides with its sibling and the sibling's name vanishes. `registered_names_are_unique` catches it; `every_bound_command_name_is_registered` catches the keymap side.
|
||||
16. **`effect_cmd!` macro** wraps a pre-built effect as a parseable command. Use it for simple wrappers; reach for a hand-written `Cmd` once there's real decision logic.
|
||||
17. **Float equality inside formula eval is currently mixed** — `=`/`!=` use a `1e-10` epsilon but the div-by-zero guard checks `rv == 0.0` exactly. Don't assume one or the other; tracked for cleanup.
|
||||
18. **`IndexMap`** preserves insertion order for categories, views, and items. Persistence relies on this.
|
||||
19. **Commit paths for cell edits** split across `commit_cell_value` → `commit_regular_cell_value` / `commit_plain_records_edit` in `command/cmd/commit.rs`. Keep the synthetic-records branch in sync with the plain branch when changing behavior.
|
||||
20. **Minibuffer commits go through one command.** `commit-buffer <formula|category|item>` (`CommitBuffer { kind: CommitKind }`) owns buffer→trim→bail-on-empty→dirty→status for all three; the varying parts are methods on `CommitKind`. The add effect is emitted **last** so its failure message wins over the command's optimistic status (improvise-6t3). `CommitKind::Item` carries its target category, resolved from `AppMode::ItemAdd` by `parse_commit_kind` at registry time — so `commit-buffer item` is not constructible from a script line.
|
||||
21. **A failed `Save` aborts the batch.** The `Save` effect sets `abort_effects` when there's no file path or the write fails (improvise-8tk), so anything chained after it — the `Quit` in `wq`/`ZZ` — is skipped and the error stays on screen. A `Binding::Sequence` flattens into one effect vec, so the abort carries across sequence steps too.
|
||||
22. **The import wizard's back path is a history stack, not a reverse table.** `advance` pushes the step it leaves onto `ImportWizard::history`; `previous` pops it, so back always retraces the path actually taken through the conditional transitions (improvise-907). Preconditions live in `validation_error()`, checked on the step where the user can still fix them — never deferred to `build_model`. `NameModel` is terminal for `advance`: Enter there calls `build()`, which parks the workbook in `built` on `Done`, and `AcceptWizardModel` takes it from there. Going back from `Done` discards `built`.
|
||||
23. **`app.layout` goes stale mid-batch.** `apply_effects` rebuilds only *after* the whole batch, so an effect that reads the layout after an earlier effect changed the grid needs a `RebuildLayout` effect in front of it — the command decides the order (see `EnterEditAtCursorCmd`, `CommitAndAdvance`). Read the cursor's cell text via `App::display_value_at_cursor()`; don't build a `CmdContext` inside `apply` (it recomputes column widths).
|
||||
|
||||
---
|
||||
|
||||
@@ -189,6 +190,7 @@ ast.rs 77 Expr, BinOp, AggFunc, Formula, Filter
|
||||
### `improvise-io` (`crates/improvise-io/src/`)
|
||||
```
|
||||
storage.rs 130 / 5t Storage trait, PhysicalFs, MemoryFs (byte-level I/O)
|
||||
injected into App as `storage`; effects use it, never std::fs
|
||||
persistence/improv.pest 124 PEG grammar — single source of truth
|
||||
persistence/mod.rs 2402 / 83t save/load/gzip/legacy-JSON, CSV export
|
||||
import/wizard.rs 1121 / 38t ImportPipeline + ImportWizard
|
||||
|
||||
@@ -10,4 +10,4 @@ pub mod ui;
|
||||
// resolving unchanged inside the moved modules.
|
||||
pub use improvise_core::{format, model, view, workbook};
|
||||
pub use improvise_formula as formula;
|
||||
pub use improvise_io::{import, persistence};
|
||||
pub use improvise_io::{import, persistence, storage};
|
||||
|
||||
@@ -459,10 +459,26 @@ pub struct App {
|
||||
/// `KeymapSet::default_keymaps()`. Read every keypress but never
|
||||
/// mutated during a session.
|
||||
keymap_set: KeymapSet,
|
||||
/// **Config**: where documents are read and written. The host chooses:
|
||||
/// the native binary supplies `PhysicalFs`, a browser worker will supply
|
||||
/// an OPFS backend, and tests can supply `MemoryFs`. Every effect that
|
||||
/// touches a document goes through this rather than `std::fs`, so the
|
||||
/// command layer has no filesystem of its own (improvise-fuj).
|
||||
pub storage: Box<dyn crate::storage::Storage>,
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub fn new(mut workbook: Workbook, file_path: Option<PathBuf>) -> Self {
|
||||
/// Build an app backed by the real filesystem.
|
||||
pub fn new(workbook: Workbook, file_path: Option<PathBuf>) -> Self {
|
||||
Self::with_storage(workbook, file_path, Box::new(crate::storage::PhysicalFs))
|
||||
}
|
||||
|
||||
/// Build an app over an arbitrary storage backend.
|
||||
pub fn with_storage(
|
||||
mut workbook: Workbook,
|
||||
file_path: Option<PathBuf>,
|
||||
storage: Box<dyn crate::storage::Storage>,
|
||||
) -> Self {
|
||||
// Recompute formula cache before building the initial layout so
|
||||
// formula-derived values are available on the first frame. The
|
||||
// cache is keyed by the active view's None-axis categories, so
|
||||
@@ -488,6 +504,7 @@ impl App {
|
||||
layout,
|
||||
abort_effects: false,
|
||||
keymap_set: KeymapSet::default_keymaps(),
|
||||
storage,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -766,7 +766,7 @@ impl Effect for SetDrillPendingEdit {
|
||||
pub struct Save;
|
||||
impl Effect for Save {
|
||||
fn apply(&self, app: &mut App) {
|
||||
match save_to_current_path(&app.model_state) {
|
||||
match save_to_current_path(&*app.storage, &app.model_state) {
|
||||
Ok(msg) => {
|
||||
app.model_state.dirty = false;
|
||||
app.view_state.status_msg = msg;
|
||||
@@ -787,11 +787,14 @@ impl Effect for Save {
|
||||
|
||||
/// Write the workbook to its current file path, returning the status
|
||||
/// message either way. Missing path and I/O failure are both errors.
|
||||
fn save_to_current_path(state: &crate::ui::app::ModelState) -> Result<String, String> {
|
||||
fn save_to_current_path(
|
||||
storage: &dyn crate::storage::Storage,
|
||||
state: &crate::ui::app::ModelState,
|
||||
) -> Result<String, String> {
|
||||
state.file_path.as_ref().map_or_else(
|
||||
|| Err("No file path — use :w <path>".to_string()),
|
||||
|path| {
|
||||
crate::persistence::save(&state.workbook, path)
|
||||
crate::persistence::save_to(storage, &state.workbook, path)
|
||||
.map(|()| format!("Saved to {}", path.display()))
|
||||
.map_err(|e| format!("Save error: {e}"))
|
||||
},
|
||||
@@ -802,7 +805,7 @@ fn save_to_current_path(state: &crate::ui::app::ModelState) -> Result<String, St
|
||||
pub struct SaveAs(pub PathBuf);
|
||||
impl Effect for SaveAs {
|
||||
fn apply(&self, app: &mut App) {
|
||||
match crate::persistence::save(&app.model_state.workbook, &self.0) {
|
||||
match crate::persistence::save_to(&*app.storage, &app.model_state.workbook, &self.0) {
|
||||
Ok(()) => {
|
||||
app.model_state.file_path = Some(self.0.clone());
|
||||
app.model_state.dirty = false;
|
||||
@@ -928,8 +931,12 @@ impl Effect for AcceptWizardModel {
|
||||
pub struct StartImportWizard(pub String);
|
||||
impl Effect for StartImportWizard {
|
||||
fn apply(&self, app: &mut App) {
|
||||
match std::fs::read_to_string(&self.0) {
|
||||
Ok(content) => match serde_json::from_str::<serde_json::Value>(&content) {
|
||||
match app
|
||||
.storage
|
||||
.read(std::path::Path::new(&self.0))
|
||||
.map_err(|e| e.to_string())
|
||||
{
|
||||
Ok(bytes) => match serde_json::from_slice::<serde_json::Value>(&bytes) {
|
||||
Ok(json) => {
|
||||
app.view_state.wizard = Some(crate::import::wizard::ImportWizard::new(json));
|
||||
app.view_state.mode = AppMode::ImportWizard;
|
||||
@@ -953,7 +960,12 @@ pub struct ExportCsv(pub PathBuf);
|
||||
impl Effect for ExportCsv {
|
||||
fn apply(&self, app: &mut App) {
|
||||
let view_name = app.model_state.workbook.active_view.clone();
|
||||
match crate::persistence::export_csv(&app.model_state.workbook, &view_name, &self.0) {
|
||||
match crate::persistence::export_csv_to(
|
||||
&*app.storage,
|
||||
&app.model_state.workbook,
|
||||
&view_name,
|
||||
&self.0,
|
||||
) {
|
||||
Ok(()) => {
|
||||
app.view_state.status_msg = format!("Exported to {}", self.0.display());
|
||||
}
|
||||
@@ -972,7 +984,7 @@ impl Effect for ExportCsv {
|
||||
pub struct LoadModel(pub PathBuf);
|
||||
impl Effect for LoadModel {
|
||||
fn apply(&self, app: &mut App) {
|
||||
match crate::persistence::load(&self.0) {
|
||||
match crate::persistence::load_from(&*app.storage, &self.0) {
|
||||
Ok(mut loaded) => {
|
||||
loaded.normalize_view_state();
|
||||
app.model_state.workbook = loaded;
|
||||
@@ -1037,7 +1049,15 @@ impl Effect for ImportJsonHeadless {
|
||||
// (short rows, improvise-k8i) are threaded into the status message.
|
||||
let mut warnings: Vec<String> = vec![];
|
||||
let (records, raw) = if is_csv {
|
||||
match crate::import::csv_parser::parse_csv_with_warnings(&self.path) {
|
||||
let bytes = match app.storage.read(&self.path) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
app.view_state.status_msg =
|
||||
format!("Cannot read '{}': {e}", self.path.display());
|
||||
return;
|
||||
}
|
||||
};
|
||||
match crate::import::csv_parser::parse_csv_bytes_with_warnings(&bytes) {
|
||||
Ok((recs, w)) => {
|
||||
warnings = w;
|
||||
let raw = serde_json::Value::Array(recs.clone());
|
||||
@@ -1049,7 +1069,7 @@ impl Effect for ImportJsonHeadless {
|
||||
}
|
||||
}
|
||||
} else {
|
||||
let content = match std::fs::read_to_string(&self.path) {
|
||||
let content = match app.storage.read(&self.path) {
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
app.view_state.status_msg =
|
||||
@@ -1057,7 +1077,7 @@ impl Effect for ImportJsonHeadless {
|
||||
return;
|
||||
}
|
||||
};
|
||||
let value: serde_json::Value = match serde_json::from_str(&content) {
|
||||
let value: serde_json::Value = match serde_json::from_slice(&content) {
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
app.view_state.status_msg = format!("JSON parse error: {e}");
|
||||
@@ -1280,6 +1300,72 @@ mod tests {
|
||||
App::new(wb, None)
|
||||
}
|
||||
|
||||
// ── Storage injection (improvise-fuj) ───────────────────────────────
|
||||
|
||||
/// improvise-fuj: every effect that touches a document goes through
|
||||
/// `App::storage`, so the whole save/load/import/export surface works on
|
||||
/// a backend that is not a filesystem. This is what a browser worker
|
||||
/// will supply (improvise-i34); here `MemoryFs` stands in for it, and
|
||||
/// the test would fail if any of these effects reached for `std::fs`.
|
||||
#[test]
|
||||
fn the_document_surface_runs_on_a_non_filesystem_backend() {
|
||||
use crate::storage::{MemoryFs, Storage};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
// A backend the test can inspect afterwards. Arc so the test keeps a
|
||||
// handle while the App owns its own reference.
|
||||
#[derive(Debug, Default)]
|
||||
struct SharedMemoryFs(Arc<MemoryFs>);
|
||||
impl Storage for SharedMemoryFs {
|
||||
fn read(&self, path: &Path) -> anyhow::Result<Vec<u8>> {
|
||||
self.0.read(path)
|
||||
}
|
||||
fn write(&self, path: &Path, bytes: &[u8]) -> anyhow::Result<()> {
|
||||
self.0.write(path, bytes)
|
||||
}
|
||||
}
|
||||
|
||||
let fs = Arc::new(MemoryFs::new());
|
||||
let mut wb = Workbook::new("Injected");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "East").unwrap();
|
||||
let mut app = App::with_storage(wb, None, Box::new(SharedMemoryFs(fs.clone())));
|
||||
|
||||
// Save: writes the model through the injected backend, no disk.
|
||||
let doc = PathBuf::from("m.improv");
|
||||
SaveAs(doc.clone()).apply(&mut app);
|
||||
assert_eq!(fs.paths(), vec![doc.clone()], "SaveAs wrote to the backend");
|
||||
assert!(!app.model_state.dirty);
|
||||
|
||||
// Export: CSV also goes through the backend.
|
||||
ExportCsv(PathBuf::from("m.csv")).apply(&mut app);
|
||||
assert!(
|
||||
fs.paths().contains(&PathBuf::from("m.csv")),
|
||||
"ExportCsv wrote to the backend, got {:?}",
|
||||
fs.paths()
|
||||
);
|
||||
|
||||
// Load: reads it back out of the backend.
|
||||
app.model_state.workbook = Workbook::new("Replaced");
|
||||
LoadModel(doc).apply(&mut app);
|
||||
assert_eq!(
|
||||
app.model_state.workbook.model.name, "Injected",
|
||||
"LoadModel read through the backend; got status {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
|
||||
// Import wizard: seeds from a JSON document in the backend.
|
||||
fs.write(Path::new("d.json"), br#"[{"region":"East","rev":1}]"#)
|
||||
.unwrap();
|
||||
StartImportWizard("d.json".to_string()).apply(&mut app);
|
||||
assert!(
|
||||
app.view_state.wizard.is_some(),
|
||||
"StartImportWizard read through the backend; got status {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
}
|
||||
|
||||
// ── Effect classification (improvise-mae) ───────────────────────────
|
||||
|
||||
/// Serialized model data — everything a connected client renders.
|
||||
|
||||
@@ -25,12 +25,20 @@ pub fn parse_csv(path: &Path) -> Result<Vec<Value>> {
|
||||
/// fields are filled with `Value::Null` — and each affected column is
|
||||
/// reported as a warning with the number of rows it was missing from.
|
||||
pub fn parse_csv_with_warnings(path: &Path) -> Result<(Vec<Value>, Vec<String>)> {
|
||||
let bytes = std::fs::read(path)
|
||||
.with_context(|| format!("Failed to open CSV file: {}", path.display()))?;
|
||||
parse_csv_bytes_with_warnings(&bytes)
|
||||
}
|
||||
|
||||
/// As [`parse_csv_with_warnings`], but over bytes already in hand — so CSV
|
||||
/// import can come from any [`Storage`](crate::storage::Storage) backend
|
||||
/// rather than only a file (improvise-fuj).
|
||||
pub fn parse_csv_bytes_with_warnings(bytes: &[u8]) -> Result<(Vec<Value>, Vec<String>)> {
|
||||
let mut reader = ReaderBuilder::new()
|
||||
.has_headers(true)
|
||||
.flexible(true)
|
||||
.trim(csv::Trim::All)
|
||||
.from_path(path)
|
||||
.with_context(|| format!("Failed to open CSV file: {}", path.display()))?;
|
||||
.from_reader(bytes);
|
||||
|
||||
// Detect if first row looks like headers (strings) or data (mixed)
|
||||
let has_headers = reader.headers().is_ok();
|
||||
|
||||
@@ -631,7 +631,24 @@ fn coord_str(key: &CellKey) -> String {
|
||||
.join(", ")
|
||||
}
|
||||
|
||||
/// Export a view as CSV through the real filesystem. The native entry point.
|
||||
pub fn export_csv(workbook: &Workbook, view_name: &str, path: &Path) -> Result<()> {
|
||||
export_csv_to(&PhysicalFs, workbook, view_name, path)
|
||||
}
|
||||
|
||||
/// Export a view as CSV into `storage`.
|
||||
pub fn export_csv_to(
|
||||
storage: &dyn Storage,
|
||||
workbook: &Workbook,
|
||||
view_name: &str,
|
||||
path: &Path,
|
||||
) -> Result<()> {
|
||||
storage.write(path, &render_csv(workbook, view_name)?)
|
||||
}
|
||||
|
||||
/// Render a view as CSV bytes. Separated from writing so the bytes can go to
|
||||
/// any [`Storage`] backend, not just a file (improvise-fuj).
|
||||
fn render_csv(workbook: &Workbook, view_name: &str) -> Result<Vec<u8>> {
|
||||
let view = workbook
|
||||
.views
|
||||
.get(view_name)
|
||||
@@ -641,7 +658,9 @@ pub fn export_csv(workbook: &Workbook, view_name: &str, path: &Path) -> Result<(
|
||||
let model = &workbook.model;
|
||||
let (fmt_comma, fmt_decimals) = crate::format::parse_number_format(&view.number_format);
|
||||
|
||||
let mut wtr = csv::WriterBuilder::new().flexible(true).from_path(path)?;
|
||||
let mut wtr = csv::WriterBuilder::new()
|
||||
.flexible(true)
|
||||
.from_writer(Vec::new());
|
||||
|
||||
// Header row
|
||||
let row_header = layout.row_cats.join("/");
|
||||
@@ -677,7 +696,7 @@ pub fn export_csv(workbook: &Workbook, view_name: &str, path: &Path) -> Result<(
|
||||
}
|
||||
|
||||
wtr.flush()?;
|
||||
Ok(())
|
||||
Ok(wtr.into_inner()?)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -206,3 +206,16 @@ wasm_non_hazard(flate2, 'compiles for wasm as-is').
|
||||
invariant(compile_check_is_not_runtime_proof,'the wasm CI gate is cargo check, so it catches nothing that merely panics at runtime: Instant and std::fs both COMPILE for wasm32-unknown-unknown').
|
||||
work_done('improvise-ywd',audit,'added wasm32-unknown-unknown to the flake toolchain, which was the real blocker; all four bundle crates then checked clean with zero source changes. Swapped Instant for web_time and added the CI wasm job.').
|
||||
location(wasm_ci,'.github/workflows/ci.yml','wasm job').
|
||||
|
||||
% ── Storage injection (improvise-fuj) ───────────────────────────────────────
|
||||
storage_injection_point('App::storage', 'Box<dyn Storage>', 'chosen by the host at construction').
|
||||
storage_host(native_cli, physical_fs).
|
||||
storage_host(tests, memory_fs).
|
||||
storage_host(browser_worker, opfs, planned('improvise-i34')).
|
||||
invariant(effects_never_touch_fs,'every document read or write in the command layer goes through App::storage; production code in improvise-command contains no std::fs').
|
||||
invariant(io_keeps_path_wrappers,'improvise-io keeps path-taking wrappers (save, load, export_csv, parse_csv_with_warnings) delegating to _to/_from/_bytes variants, so the native CLI is unchanged while the byte-level path is backend-agnostic').
|
||||
% Why the sync trait survives contact with OPFS.
|
||||
opfs_api(promise_based, async, 'getDirectory, getFileHandle, createWritable').
|
||||
opfs_api(sync_access_handle, sync, 'createSyncAccessHandle; worker-only; read/write/flush/close are blocking').
|
||||
decision('improvise-i34',keep_storage_sync,'improvise-9x6 hosts App and persistence inside a browser worker, where FileSystemSyncAccessHandle is available. Async handle acquisition happens once at worker init; read/write are then synchronous, so the two-method sync trait stands and no async migration is needed. Caveat: asserted from API knowledge, untested here; the IndexedDB fallback IS async-only and would reopen the question.').
|
||||
work_done('improvise-fuj',refactor,'App gained a storage backend; Save/SaveAs/ExportCsv/LoadModel/StartImportWizard and CSV import all route through it. Test drives save, export, load and wizard-seed over MemoryFs with no disk.').
|
||||
|
||||
Reference in New Issue
Block a user