Commit graph

4 commits

Author SHA1 Message Date
a2b05c56c9 feat(makepad-table): opt-in capabilities feature, and raise the matrix_client defect
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
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.
2026-08-18 18:03:26 +00:00
7737096858 refactor(makepad-table): adopt Robrix's image decode path
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
makepad-table / model (push) Has been cancelled
makepad-table / widget (push) Has been cancelled
makepad-table / hygiene (push) Has been cancelled
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.
2026-08-18 16:51:47 +00:00
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.
2026-08-17 04:10:26 +00:00
258fa3259e Merge origin/main: resolve xref/document conflicts, add makepad_table
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
2026-08-16 22:53:23 +03:00