feat: put server, import and lsp behind cargo features

[dubadub]

Addresses the dependency-count half of #366.

Why

cargo install cookcli compiles 412 crates on aarch64. Only self-update and sync were gated, so someone who wants cook recipe and cook shopping-list still compiled an HTTP server, a TLS stack, an HTML scraper and a language server.

Result

Defaults are unchanged — cargo install cookcli still gets every command, so this is not a breaking change.

configuration	crates	binary
default (today, and still)	412	23 MB
--no-default-features	225	9 MB
--no-default-features --features server	278

Always compiled: parsing, scaling, shopping lists, search, pantry, reports, cook build, seed.

Two things worth reviewing carefully

1. src/server/ had to be split first (separate commit, pure code movement). It held both the axum layer and the rendering layer — askama templates, view-model builders, i18n, embedded static assets — and cook build depends on the latter. They could not be gated apart, so rendering moved to src/web/ and the sync client to src/sync/ (login and logout need it without the server). find_todays_menu moved out of an axum handler module into web, since it is a pure function returning a template type.

2. This surfaced a latent bug. The bundled_units feature of cooklang — the unit database behind quantity scaling and unit normalisation — was never declared by cookcli. It was arriving purely through Cargo feature unification, because cooklang-language-server and cooklang-reports depend on cooklang with default features. Gating lsp off dropped it, and cook recipe output silently changed:

   "quantity": {
-    "scalable": false,
-    "unit": "c",
+    "scalable": true,
+    "unit": "cups",

Caught by the snapshot tests. Fixed by declaring bundled_units in our own cooklang dependency, where it is actually used. Output is now identical across every feature combination.

This was latent on main: any dependency change that dropped those crates from the graph would have silently altered recipe output.

Also: sync implies server, because the sync loop only ever runs inside the server (start_sync is unreachable from cook login and cook logout).

Verification

Build, clippy and tests across every combination — --no-default-features, each feature alone, server,import,lsp, --all-features, and default:

• cargo build — 0 errors, 0 warnings in all 9 combinations
• cargo clippy — 0 warnings in all combinations
• cargo test — 13 suites pass under default, --no-default-features, and --all-features
• cargo fmt --check — clean

The test_help_output snapshot is now gated on server/import/lsp, since it asserts the full subcommand list.

Independent of #367 (thin LTO) and #368 (install docs).

Refs #366

[claude]

Review: feat: put server, import and lsp behind cargo features

Nice work overall — the split of src/server/ (axum layer) from src/web/ (rendering layer used by both cook server and cook build) is clean, and the bundled_units fix is a genuinely valuable catch (a latent bug where cook recipe's unit output silently depended on feature unification from cooklang-language-server/cooklang-reports rather than a declared dependency). I checked every renamed crate::server::* reference across src/ and templates/ and found no stale paths — the mechanical move looks complete and consistent with the new #[cfg(feature = ...)] gating.

Possible bug / stray test artifact

tests/snapshots/snapshot_test__help_output_no_update.snap.new is checked into this PR (added, not gitignored). A .snap.new file is cargo-insta's "pending, unaccepted" output — its presence means that at some point the actual --help output didn't match the already-accepted snapshot_test__help_output_no_update.snap, and that mismatch was never resolved (accepted or investigated), just committed alongside the .snap it disagrees with.

Concretely: test_help_output is now gated #[cfg(all(feature = "server", feature = "import", feature = "lsp"))], and the _no_update variant additionally requires self-update off — i.e. it only runs under something like --no-default-features --features server,import,lsp. That's exactly the kind of intermediate combination mentioned as manually verified in the PR description, but .github/workflows/test.yml only runs cargo test (all default features) and cargo test --no-default-features — it never builds/tests any of the in-between combos (server alone, server,import,lsp, etc.). So this specific test path isn't covered by CI at all, and the leftover .snap.new is the only evidence that something didn't match during manual testing.

Suggest:

1. Regenerate and diff the two files to see what's actually different, and either update the accepted .snap or delete the stray .snap.new before merging.
2. Add *.snap.new to .gitignore (this repo's CI already uploads them as artifacts on failure at line 78 of test.yml, so they clearly aren't meant to be committed).
3. Consider adding at least one CI job for a partial feature combo (e.g. --no-default-features --features server,import,lsp) so this class of "help output only breaks for one specific feature set" bug can't slip through silently again — this is the same class of bug the bundled_units fix addressed.

Things that look solid on inspection

• sync = ["server", ...] correctly forces server on, and the reasoning (the sync loop only runs inside cook server, so login/logout without a server would store credentials nothing ever consumes) is sound — worth knowing this means "sync without the full HTTP stack" isn't achievable, but that seems like an intentional, reasonable tradeoff rather than an oversight.
• self-update = ["dep:self_update", "dep:reqwest"] explicitly adding dep:reqwest looks redundant at first glance (nothing in update.rs calls reqwest directly), but self_update itself depends on reqwest transitively (confirmed in Cargo.lock), and this repo pins reqwest to default-features = false, features = ["json", "rustls-tls"]. Declaring it explicitly forces Cargo's feature unification to pick rustls instead of whatever TLS backend self_update's own reqwest dependency would default to — this avoids pulling in an OpenSSL-based build, which matters given the PR is specifically fighting low-memory/aarch64 build pain. Good catch, worth calling out in the PR description since it's non-obvious.
• All #[cfg(feature = "server")] additions in src/web/templates.rs (the HTML-page-only templates: ErrorTemplate, ShoppingListTemplate, PreferencesTemplate, PantryTemplate, EditTemplate, NewTemplate, PantrySection/PantryItem) are consistent with what cook build's static-site renderer actually uses (RecipeTemplate/RecipesTemplate/MenuTemplate, which stay ungated) — I didn't find a template referenced by cook build that got accidentally gated behind server.
• Module/command gating in args.rs, lib.rs, main.rs is exhaustive and matches — no leftover unconditional use/mod that would break --no-default-features.

Minor

• README's new feature table is a nice addition; might be worth a one-line callout that sync implies server there too (currently only mentioned in the Cargo.toml comment and commit message), since a reader skimming just the table could reasonably expect sync alone to be lean.

Didn't run a local build in this environment (sandboxed), so I relied on static review of the diff plus grepping for stale references — worth having CI actually confirm the no-default-features/test suite passes clean before merge, per this repo's own pre-PR checklist.

[dubadub]

Housekeeping: the bundled_units fix in this PR is now also split out on its own as #371, so it can land independently — it fixes a latent correctness bug that exists on main today and has nothing to do with the refactor here.

That means the two touch the same Cargo.toml line. Merge order:

1. fix: satisfy clippy::useless_borrows_in_formatting (unbreaks CI) #370 — unbreaks CI (clippy is red on main from the Rust 1.97 toolchain bump; that is why this PR shows Test fail despite passing locally)
2. fix(deps): declare cooklang's bundled_units feature explicitly #371 — bundled_units, a verified no-op today
3. this PR — I will rebase onto main afterwards and drop the now-duplicate hunk

This PR still needs bundled_units: without it, --no-default-features drops cooklang-language-server from the graph and recipe output silently changes. That is exactly how the bug was found.

[dubadub]

Rebased onto main now that #370, #371 and #368 have landed.

Changes since the last review pass:

• Dropped the duplicate bundled_units hunk (it is on main via fix(deps): declare cooklang's bundled_units feature explicitly #371).
• Removed a stray tests/snapshots/*.snap.new file that a git add -A had swept into the commit, and added *.snap.new to .gitignore so insta rejects cannot slip in again.
• Gated the insta::with_settings import alongside test_help_output, which uses it — clippy 1.97 flagged it as unused under --no-default-features.

Re-verified against the same toolchain CI uses (1.97), not just my local one:

• cargo +1.97 clippy --all-targets -- -D warnings — 0 issues on default, --no-default-features, --all-features, and --features server
• cargo +1.97 fmt --check — clean
• cargo +1.97 test — 13 suites pass on default and --no-default-features

[claude]

Review

Nice piece of work — the src/server → src/web split (rendering/templates/i18n vs. axum transport) is clean, and moving find_todays_menu's tests along with the function into src/web/menus.rs means no test coverage was actually lost, contrary to what the diff stat suggests at a glance.

Bug: cook server built without lsp breaks the in-browser LSP editor at runtime

Cargo.toml's server feature does not include lsp:

server = ["dep:axum", "dep:askama_axum", "dep:tower", "dep:tower-http", "dep:notify", "dep:notify-debouncer-full", "dep:tokio-stream", "dep:futures-util", "dep:open"]

but src/server/mod.rs unconditionally registers .route("/ws/lsp", get(lsp_bridge::lsp_websocket)), and src/server/lsp_bridge.rs::spawn_lsp_process shells out to the same binary with Command::new(exe_path).arg("lsp").

If someone builds the exact combo the updated README now recommends —

cargo build --release --no-default-features --features server

— the lsp subcommand doesn't exist in that binary. spawn() itself still succeeds (it's spawning a real, existing executable), so spawn_lsp_process's Err branch is never hit; the child process just starts, clap rejects the unrecognized lsp subcommand, and it exits immediately. The websocket bridge then silently breaks (stdin/stdout pipes go dead) rather than failing loudly. This is a runtime issue, not a compile-time one, so the "0 errors/warnings across 9 build combinations" verification in the PR description wouldn't have caught it — and CI (.github/workflows/test.yml) only exercises the "all features" and "no default features" extremes, never --features server alone, so this gap isn't covered there either.

Suggest either:

• adding "lsp" to the server feature list (mirroring how sync already implies server), or
• gating the /ws/lsp route and lsp_bridge module behind #[cfg(feature = "lsp")] so a server-only build simply omits that endpoint instead of registering a route that's guaranteed to fail.

Minor: stray license header on a new, non-vendored file

src/web/mod.rs adds a full MIT license block attributed to Copyright (c) 2024 cooklang. Elsewhere in the repo, that header pattern is reserved for files adapted from Francisco J. Sanchez's original CLI (main.rs, args.rs, shopping_list.rs, server/mod.rs, util/cooklang_to_human.rs, recipe/read.rs, all 2023-attributed, all with the explanatory "the original code is licensed under..." preamble). Genuinely new project files (pantry.rs, search.rs, report.rs, and the other new/renamed files in this PR — web/menus.rs, web/builders.rs, web/templates.rs, sync/mod.rs) carry no such header. This looks like a copy-paste artifact from splitting server/mod.rs rather than an intentional choice — worth double-checking with whoever owns the licensing convention here.

Nit: reqwest under import / self-update features

Worth a second look, lower confidence than the above: neither src/import.rs nor src/update.rs reference reqwest directly (only src/sync/* and src/login.rs do), yet both the import and self-update feature entries in Cargo.toml pull in dep:reqwest. If that's just to force feature unification onto rustls-tls (so cooklang-import's / self_update's own transitive reqwest doesn't drag in a native-TLS/OpenSSL stack alongside it), it's a reasonable and probably intentional pattern — but if not, it's dependency weight the "trim cargo install cookcli" goal of this PR would want to avoid for a standalone --features import or --features self-update build.

Everything else

• The cfg-gating on Command enum variants, mod declarations, and match arms in args.rs / main.rs / lib.rs is consistent and follows the standard "cfg resolves before derive expansion" pattern — should be sound.
• Splitting the always-needed rendering layer (web::builders, web::templates, web::i18n, web::language, StaticFiles) from the axum-specific pieces (kept #[cfg(feature = "server")] inside web::templates.rs / web::language.rs at the item level rather than the module level) correctly keeps cook build working without the server feature.
• sync implying server is explicitly called out and justified in the PR description (the sync loop only runs inside the server); worth flagging as a tradeoff for anyone who'd want a lightweight cook login/cook logout-only build, but not a defect.
• No security concerns — the changes are purely structural/feature-gating; no new attack surface, and CORS/body-limit config is untouched.