4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| a2b05c56c9 |
feat(makepad-table): opt-in capabilities feature, and raise the matrix_client defect
The two caveats from the dependency investigation. ## The capabilities feature Camera and location attachments are now available behind `features = ["capabilities"]`, which pulls `nigig-uikit` and supplies `UikitAttachmentProvider`. Measured: 89 crates by default, 275 with the feature on. That cost is real and it is inherent, not packaging waste. `camera_widget` imports `send_geocode_request` and `request_map_tile` from `nigig-core`, both of which call `spawn_async` — the shared Tokio runtime — and the first makes an HTTPS call to Nominatim. A camera that geocodes needs an async runtime and an HTTP client; there is no lighter honest version. It is affordable because it is opt-in, and because any app enabling it already depends on `nigig-core`, so that app's own tree grows by nothing. Everything touching `nigig-uikit` is in one module, so the boundary is a file rather than `#[cfg]` scattered through the widget. The provider holds no widgets of its own: the host owns the `CameraWidget` already in its tree and this asks it to open, because a provider that instantiated a second camera would fight the first for the device. A second request while one is outstanding is refused rather than overwriting. The table turns that refusal into `AttachmentUnavailable`, so the user is told the camera is busy instead of watching their first request vanish. File picking is deliberately declined here — `robius-file-picker` already ships unconditionally and costs nothing, and two paths for one job is one too many. Two CI gates, both verified to fail when they should: the opt-in build must keep compiling, and the default build must pull none of `tokio`, `reqwest`, `hyper`, `clap`, `csv`, `image`, `nigig-uikit` or `nigig-core`. The second checks the resolved `cargo tree` rather than the manifest, because feature unification can switch an optional dependency on from a sibling crate. Tests 99 default, 105 with the feature. Both clippy-clean. ## The matrix_client defect Raised in REVIEWS/MATRIX_CLIENT_FEATURE_GATE.md rather than fixed. It is not my crate, nothing depends on the broken combination, and a blind fix could change behaviour someone relies on. `matrix_client` declares `native = ["dep:tokio", "dep:reqwest", "dep:rusqlite"]` but its source gates on `#[cfg(not(target_arch = "wasm32"))]`. Two switches for the same modules, so on a native target with the feature off the modules compile and their dependencies do not — 19 errors, 26 ungated uses across 7 files. There is no CI job for the crate, which is why it rotted unnoticed. The note corrects an overstatement I made while arguing for the trait hook. I said fixing this would unblock wasm. It would not: `matrix_client` already builds clean for wasm32 with `--no-default-features`, and `nigig-core` has 8 wasm errors of its own (`crate::platform::spawn` missing) that have nothing to do with it. The only broken combination is native-target-with-feature-off, which nothing builds. I also said earlier that `matrix_client` was heavy — it is a 7-dependency local crate, not matrix-sdk. That was wrong and it inflated the case for the trait hook; the note records the measured numbers instead. |
|||
| 7737096858 |
refactor(makepad-table): adopt Robrix's image decode path
Replaces the hand-rolled try-PNG-then-JPEG with `pageflipnav/src/utils.rs::load_png_or_jpg`, the pattern the Robrix-derived app in this repo already uses. Two things it does better: - It sniffs the header with `imghdr` and calls the matching loader directly, so a JPEG does not decode-and-fail as a PNG first on every cold cache. - It still falls back to trying both when the sniff names something unexpected or nothing at all. `imghdr` is not perfect, and a mislabelled file is more useful decoded than refused. `imghdr` has no transitive dependencies — it reads a header and names a format. It is already a dependency of `pageflipnav` at the same version. The upstream version logs the failure and dumps the bad bytes to disk. That is right for a chat client receiving untrusted media and wrong here: this runs from the draw path for every attached cell, so a broken file would log once per frame. The caller already caches the failure and draws a labelled chip naming the file, which tells the user more than a log line would. Tests 94 -> 99. Verified by removing the sniff and by removing the fallback; each fails the ordering test. `TextOrImage`, the other candidate for reuse, is referenced in `room_screen.rs` but not defined anywhere in this checkout — it is upstream Robrix only, so there was no baseline here to adopt. |
|||
| ea98d4d95c |
fix(makepad-table): exclude from the workspace, pin makepad, fix money defects
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-build (CAD) / cad-engine-coverage (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
email.yml / fix(makepad-table): exclude from the workspace, pin makepad, fix money defects (push) Failing after 0s
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-map / test (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
`crates/apps/makepad_table` is a nested workspace that has never been compiled — its README says as much: "written without a local cargo/rust toolchain, so the first compile on your machine is the verification step." This is that step. Four crates, all of which build, and three defects in the money code that only a compiler and a test runner could have found. Workspace containment, which is what was asked for: - The root manifest now names `crates/apps/makepad_table` in `exclude`. Cargo already declined to absorb it, because the crate carries its own `[workspace]` table — but that made the isolation a property of someone else's manifest. Deleting that table would have pulled four crates and a second makepad checkout into every workspace-wide build. Verified: the root workspace resolves 58 members and none of them are these. - `examples/table_demo` belonged to no workspace at all and had no `[workspace]` table of its own, so `cargo metadata` failed outright in that directory. It is now a member of the nested workspace. Kept rather than deleted: it is the template the README's "drop into makepad" section refers to. - All three manifests pinned to the fork revision the rest of the repo uses (`gitdab.com/andodeki/makepad` @ ecf5a57) instead of tracking `github.com/makepad/makepad` branch `dev`. A floating branch means the same commit of this repo builds against a different makepad from one day to the next, and against a different makepad from every other crate here. All four crates verified to compile against the pin. The defects, in the order they surfaced — each was hidden by the one before it: 1. `format_with_thousands` computed `(i - first_group_len)` before the `i >= first_group_len` guard that protects it. `&&` short-circuits left to right, so the check never ran in time. Any number whose leading group is short of three digits — 2, 3, 5, 6, 8, 9, 11, 12 digits wide — underflowed a usize: a panic in debug, silent wrapping and misplaced commas in release. Every currency string in the application went through it. The two existing tests used 1234 and 1234567, the two widths that happen to work. 2. With the panic gone, `Currency::format` was visibly wrong on negatives. The symbol was emitted before a signed whole part, giving "$-12.34" instead of "-$12.34"; and `whole` truncated toward zero while `frac` used `rem_euclid`, so the two disagreed below zero. -1234 formatted as "$-12.66" and -1 as "$0.99" — the wrong sign, the wrong place, and the wrong amount. 3. `invoice_totals_arithmetic` asserted `1_840_00` where the sample data totals 1_840_000 minor units. The prose in the same comment said 18,400.00, which is right; the literals were a factor of ten low. The arithmetic was never wrong, the expectations were. The two loose range assertions on tax and grand total are now exact equalities. Tests 12 -> 16 across the two crates, and all 16 pass; previously 6 of 12 failed. Each fix was verified by reintroducing the defect on its own: the guard-order bug fails 5 tests, the sign bug fails 4 with the overflow fix left in place, and dropping the per-line discount from the tax calculation fails the arithmetic test by 71.32 — an error the old range assertions were wide enough to have accepted. |
|||
| 258fa3259e | Merge origin/main: resolve xref/document conflicts, add makepad_table |