config: collapse registration into one parallax call
One call cannot half-register, so exactly-once needs one check instead of three and the slot no longer has to live Lua-side. Assisted-by: GLM-5.3 via pi
diff --git a/src/error.rs b/src/error.rs
index c4378a6..4170f51 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -14,7 +14,7 @@ pub enum Error {
#[error("{} must call ({what} ...)", .path)]
#[diagnostic(
code(parallax::config_registration),
- help("a config registers with (setup {{...}}), (server fn), and (flow fn)")
+ help("a config registers with (parallax {{...}})")
)]
ConfigRegistration {
path: Utf8PathBuf,
diff --git a/src/parallax.rs b/src/parallax.rs
index f6e3333..9e700b0 100644
--- a/src/parallax.rs
+++ b/src/parallax.rs
@@ -1,15 +1,14 @@
use camino::Utf8Path;
use camino::Utf8PathBuf;
-use std::mem;
+use std::sync::Arc;
+use std::sync::Mutex;
-use mlua::AnyUserData;
use mlua::ErrorContext as _;
use mlua::FromLua;
use mlua::Function;
use mlua::Lua;
use mlua::LuaSerdeExt as _;
use mlua::Table;
-use mlua::UserData;
use mlua::Value;
use crate::Error;
@@ -18,59 +17,50 @@ use crate::spec::Variant;
const FENNEL_LUA: &str = include_str!("../vendor/fennel.lua");
-/// What a config has registered so far, filled in as the config
-/// runs. It lives Lua-side as the `parallax` module itself: configs
-/// call its methods, and Rust borrows it back out once the config
-/// finishes.
-#[derive(Default)]
-struct Registrations {
- setup: Option<Setup>,
- server: Option<Function>,
- flow: Option<Function>,
+/// The table passed to `(parallax {...})`: the setup spec plus the
+/// registered server and flow functions. Converted where the config's
+/// main chunk frame is still alive, so conversion errors carry the
+/// call site.
+struct Config {
+ server: Function,
+ flow: Function,
+ setup: Setup,
}
-impl Registrations {
- fn setup(&mut self, lua: &Lua, setup: Setup) -> Result<(), mlua::Error> {
- if self.setup.is_some() {
- return Err(error_at(lua, "setup must be called exactly once"));
- }
+impl FromLua for Config {
+ fn from_lua(value: Value, lua: &Lua) -> Result<Config, mlua::Error> {
+ let Value::Table(table) = value else {
+ return Err(error_at(lua, "parallax takes a table"));
+ };
+ let server = pop_function(&table, "server", lua)?;
+ let flow = pop_function(&table, "flow", lua)?;
+ let setup: Setup = lua
+ .from_value(Value::Table(table))
+ .map_err(|error| error_at(lua, &error_message(&error)))?;
if setup.variants.is_empty() {
return Err(error_at(lua, "variants must not be empty"));
}
- self.setup = Some(setup);
- Ok(())
- }
-
- fn server(&mut self, lua: &Lua, call: Function) -> Result<(), mlua::Error> {
- if self.server.is_some() {
- return Err(error_at(lua, "server must be called exactly once"));
- }
- self.server = Some(call);
- Ok(())
- }
-
- fn flow(&mut self, lua: &Lua, call: Function) -> Result<(), mlua::Error> {
- if self.flow.is_some() {
- return Err(error_at(lua, "flow must be called exactly once"));
- }
- self.flow = Some(call);
- Ok(())
+ Ok(Config {
+ server,
+ flow,
+ setup,
+ })
}
}
-/// Storage only. The module exposes plain functions, not methods,
-/// because configs destructure it and call them bare — `(setup {...})`
-/// — with no self to bind a userdata method to.
-impl UserData for Registrations {}
-
-/// Converts `(setup {...})`'s argument, keeping the config call site in
-/// the error: `from_lua` runs inside the callback trampoline, so the
-/// main chunk's frame is still on the Lua stack for caller_position.
-impl FromLua for Setup {
- fn from_lua(value: Value, lua: &Lua) -> Result<Setup, mlua::Error> {
- lua.from_value(value)
- .map_err(|error| error_at(lua, &error_message(&error)))
- }
+/// Takes `key` out of the table as a function, so the serde pass
+/// never sees it and unknown-field validation still catches typos.
+fn pop_function(table: &Table, key: &str, lua: &Lua) -> Result<Function, mlua::Error> {
+ let call = table
+ .get::<Value>(key)
+ .map_err(|error| error_at(lua, &error_message(&error)))?;
+ let Value::Function(call) = call else {
+ return Err(error_at(lua, &format!("{key} must be a function")));
+ };
+ table
+ .set(key, Value::Nil)
+ .map_err(|error| error_at(lua, &error_message(&error)))?;
+ Ok(call)
}
/// A loaded parallax: the executable form of a config. The server
@@ -80,10 +70,10 @@ impl FromLua for Setup {
pub struct Parallax {
// Consumed once running variants lands.
#[allow(dead_code)]
- flow: Function,
+ server: Function,
// Consumed once running variants lands.
#[allow(dead_code)]
- server: Function,
+ flow: Function,
setup: Setup,
name: String,
#[allow(dead_code)]
@@ -101,30 +91,22 @@ impl Parallax {
let lua = unsafe { Lua::unsafe_new() };
let filename = path.as_str().to_owned();
- let Registrations {
- setup,
- server,
- flow,
- } = run_config(&lua, &source, &filename).with_context(|_| format!("loading {path}"))?;
-
- let setup = setup.ok_or_else(|| Error::ConfigRegistration {
- path: path.to_owned(),
- what: "setup",
- })?;
- let server = server.ok_or_else(|| Error::ConfigRegistration {
- path: path.to_owned(),
- what: "server",
- })?;
- let flow = flow.ok_or_else(|| Error::ConfigRegistration {
- path: path.to_owned(),
- what: "flow",
- })?;
- let name = setup.name.clone().unwrap_or_else(|| default_name(path));
+ let config = run_config(&lua, &source, &filename)
+ .with_context(|_| format!("loading {path}"))?
+ .ok_or_else(|| Error::ConfigRegistration {
+ path: path.to_owned(),
+ what: "parallax",
+ })?;
+ let name = config
+ .setup
+ .name
+ .clone()
+ .unwrap_or_else(|| default_name(path));
Ok(Parallax {
- flow,
- server,
- setup,
+ server: config.server,
+ flow: config.flow,
+ setup: config.setup,
name,
path: path.to_owned(),
lua,
@@ -153,44 +135,32 @@ fn default_name(path: &Utf8Path) -> String {
.to_owned()
}
-/// Builds the parallax module in Rust and registers it as the
-/// Builds the `parallax` module in package.preload — a table of
-/// registration functions backed by the Registrations userdata — and
-/// returns the handle so the caller can borrow the registrations back
-/// out after the config runs. The functions raise errors while the
-/// config's stack frame is alive, so failures point at the call site.
-fn install_module(lua: &Lua) -> Result<AnyUserData, mlua::Error> {
- let module = lua.create_userdata(Registrations::default())?;
-
- let setup = {
- let module = module.clone();
- lua.create_function(move |lua, setup: Setup| {
- module.borrow_mut::<Registrations>()?.setup(lua, setup)
+/// Builds the `parallax` module in package.preload — a single function
+/// that converts its argument to a Config and stores it in the slot
+/// the caller keeps — and returns that slot. The function raises
+/// errors while the config's stack frame is alive, so failures point
+/// at the call site.
+fn install_module(lua: &Lua) -> Result<Arc<Mutex<Option<Config>>>, mlua::Error> {
+ let slot: Arc<Mutex<Option<Config>>> = Arc::new(Mutex::new(None));
+ let parallax = {
+ let slot = slot.clone();
+ lua.create_function(move |lua, config: Config| {
+ // Poisoning only follows a panic mid-store; the slot is
+ // still coherent, so take it rather than fail.
+ let mut slot = slot.lock().unwrap_or_else(|poisoned| poisoned.into_inner());
+ if slot.is_some() {
+ return Err(error_at(lua, "parallax must be called exactly once"));
+ }
+ *slot = Some(config);
+ Ok(())
})?
};
- let server = {
- let module = module.clone();
- lua.create_function(move |lua, call: Function| {
- module.borrow_mut::<Registrations>()?.server(lua, call)
- })?
- };
- let flow = {
- let module = module.clone();
- lua.create_function(move |lua, call: Function| {
- module.borrow_mut::<Registrations>()?.flow(lua, call)
- })?
- };
-
- let table = lua.create_table()?;
- table.set("setup", setup)?;
- table.set("server", server)?;
- table.set("flow", flow)?;
- let lookup = lua.create_function(move |_, _name: String| Ok(table.clone()))?;
+ let lookup = lua.create_function(move |_, _name: String| Ok(parallax.clone()))?;
let package: Table = lua.globals().get("package")?;
let preload: Table = package.get("preload")?;
preload.set("parallax", lookup)?;
- Ok(module)
+ Ok(slot)
}
fn error_at(lua: &Lua, message: &str) -> mlua::Error {
@@ -226,12 +196,13 @@ fn caller_position(lua: &Lua) -> Option<String> {
}
}
-/// Loads the Fennel compiler, runs the config, and returns everything
-/// it registered. Every failure surfaces as an mlua error; the caller
-/// annotates it with the config's path.
-fn run_config(lua: &Lua, source: &str, filename: &str) -> Result<Registrations, mlua::Error> {
+/// Loads the Fennel compiler, runs the config, and returns the config
+/// it registered — None when the config never called `(parallax ...)`.
+/// Every failure surfaces as an mlua error; the caller annotates it
+/// with the config's path.
+fn run_config(lua: &Lua, source: &str, filename: &str) -> Result<Option<Config>, mlua::Error> {
let fennel: Table = lua.load(FENNEL_LUA).set_name("fennel.lua").eval()?;
- let module = install_module(lua)?;
+ let slot = install_module(lua)?;
// Configs end with a registration call, which compiles to a tail
// call; tail elimination destroys the config's stack frame, hiding
@@ -253,13 +224,14 @@ fn run_config(lua: &Lua, source: &str, filename: &str) -> Result<Registrations,
.set_name(format!("@{filename}"))
.eval::<()>()?;
- // A failed borrow means a config re-entered registration while it
- // was already being mutated — surface it instead of hanging.
- Ok(mem::take(&mut *module.borrow_mut::<Registrations>()?))
+ Ok(slot
+ .lock()
+ .unwrap_or_else(|poisoned| poisoned.into_inner())
+ .take())
}
-// The index arithmetic in this helper is safe because `index` comes from
-// `rfind` on `compiled`: it's in bounds, non-zero only after the
+// The index arithmetic in this helper is safe because `index` comes
+// from `rfind` on `compiled`: it's in bounds, non-zero only after the
// `index == 0` check, and always followed by the "return " literal.
#[allow(clippy::arithmetic_side_effects, clippy::indexing_slicing)]
fn protect_tail_call(compiled: &str) -> String {
diff --git a/tests/cli.rs b/tests/cli.rs
index 66f26c5..15f1e20 100644
--- a/tests/cli.rs
+++ b/tests/cli.rs
@@ -6,20 +6,15 @@ use assert_cmd::cargo::cargo_bin_cmd;
use predicates::prelude::*;
use tempfile::TempDir;
-const VALID: &str = r#"(local {: setup : flow : server} (require :parallax))
+const VALID: &str = r#"(local parallax (require :parallax))
-(setup
+(parallax
{:variants {:mobile-dark {:viewport [375 812] :scheme :dark}
:desktop-light {:viewport [1280 900]}
:desktop-dark {:viewport [1280 900] :scheme :dark}}
- :baseline {:repo "kejadlen/ketchup"}})
-
-(server
- (fn []
- {:cmd ["bundle" "exec" "rake" "snapshots:serve"]}))
-
-(flow
- (fn [{: goto : wait-for : click : snap : variant}] nil))"#;
+ :baseline {:repo "kejadlen/ketchup"}
+ :server (fn [] {:cmd ["bundle" "exec" "rake" "snapshots:serve"]})
+ :flow (fn [{: goto : wait-for : click : snap : variant}] nil)})"#;
fn config_file(source: &str) -> (TempDir, PathBuf) {
let dir = tempfile::tempdir().expect("tempdir");
@@ -78,14 +73,10 @@ fn syntax_error_points_at_the_config_line() {
}
#[test]
-fn setup_errors_point_at_the_setup_call() {
+fn empty_variants_error_points_at_the_parallax_call() {
let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "\n",
- "(setup {:variants {}})\n",
- "\n",
- "(server (fn [] {:cmd [\"true\"]}))\n",
- "(flow (fn [] nil))\n",
+ "(local parallax (require :parallax))\n",
+ "(parallax {:variants {} :server (fn [] {:cmd [\"true\"]}) :flow (fn [] nil)})\n",
));
cargo_bin_cmd!("parallax")
.arg("list")
@@ -93,21 +84,15 @@ fn setup_errors_point_at_the_setup_call() {
.arg(&path)
.assert()
.failure()
- .stderr(
- predicate::str::contains("config.fnl:3:")
- .and(predicate::str::contains("variants must not be empty")),
- );
+ .stderr(predicate::str::contains("config.fnl:2:").and(predicate::str::contains("empty")));
}
#[test]
-fn setup_conversion_errors_point_at_the_setup_call() {
+fn unknown_field_errors_point_at_the_parallax_call() {
let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "\n",
- "(setup {:variants {:desktop {:viewport [1280 900]}} :extra 1})\n",
- "\n",
- "(server (fn [] {:cmd [\"true\"]}))\n",
- "(flow (fn [] nil))\n",
+ "(local parallax (require :parallax))\n",
+ "(parallax {:variants {:desktop {:viewport [1280 900]}} :extra 1\n",
+ " :server (fn [] {:cmd [\"true\"]}) :flow (fn [] nil)})\n",
));
cargo_bin_cmd!("parallax")
.arg("list")
@@ -119,27 +104,23 @@ fn setup_conversion_errors_point_at_the_setup_call() {
}
#[test]
-fn missing_setup_registration_fails() {
- let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "(server (fn [] {:cmd [\"true\"]}))\n",
- "(flow (fn [] nil))\n",
- ));
+fn non_table_argument_fails() {
+ let (_dir, path) = config_file("(local parallax (require :parallax))\n(parallax \"nope\")\n");
cargo_bin_cmd!("parallax")
.arg("list")
.arg("--config")
.arg(&path)
.assert()
.failure()
- .stderr(predicate::str::contains("must call (setup ...)"));
+ .stderr(predicate::str::contains("config.fnl:2:").and(predicate::str::contains("takes")));
}
#[test]
-fn missing_server_registration_fails() {
+fn server_must_be_a_function() {
let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "(setup {:variants {:desktop {:viewport [1280 900]}}})\n",
- "(flow (fn [] nil))\n",
+ "(local parallax (require :parallax))\n",
+ "(parallax {:variants {:desktop {:viewport [1280 900]}} :server \"not-a-function\"\n",
+ " :flow (fn [] nil)})\n",
));
cargo_bin_cmd!("parallax")
.arg("list")
@@ -147,15 +128,15 @@ fn missing_server_registration_fails() {
.arg(&path)
.assert()
.failure()
- .stderr(predicate::str::contains("must call (server ...)"));
+ .stderr(predicate::str::contains("config.fnl:3:").and(predicate::str::contains("server")));
}
#[test]
-fn missing_flow_registration_fails() {
+fn flow_must_be_a_function() {
let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "(setup {:variants {:desktop {:viewport [1280 900]}}})\n",
- "(server (fn [] {:cmd [\"true\"]}))\n",
+ "(local parallax (require :parallax))\n",
+ "(parallax {:variants {:desktop {:viewport [1280 900]}}\n",
+ " :server (fn [] {:cmd [\"true\"]}) :flow \"not-a-function\"})\n",
));
cargo_bin_cmd!("parallax")
.arg("list")
@@ -163,36 +144,27 @@ fn missing_flow_registration_fails() {
.arg(&path)
.assert()
.failure()
- .stderr(predicate::str::contains("must call (flow ...)"));
+ .stderr(predicate::str::contains("config.fnl:3:").and(predicate::str::contains("flow")));
}
#[test]
-fn duplicate_setup_registration_fails() {
- let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "(setup {:variants {:desktop {:viewport [1280 900]}}})\n",
- "(setup {:variants {:mobile {:viewport [375 812]}}})\n",
- "(server (fn [] {:cmd [\"true\"]}))\n",
- ));
+fn missing_parallax_call_fails() {
+ let (_dir, path) = config_file("(local parallax (require :parallax))\n");
cargo_bin_cmd!("parallax")
.arg("list")
.arg("--config")
.arg(&path)
.assert()
.failure()
- .stderr(
- predicate::str::contains("config.fnl:3:").and(predicate::str::contains("exactly once")),
- );
+ .stderr(predicate::str::contains("must call (parallax ...)"));
}
#[test]
-fn duplicate_flow_registration_fails() {
+fn duplicate_parallax_call_fails() {
let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "(setup {:variants {:desktop {:viewport [1280 900]}}})\n",
- "(server (fn [] {:cmd [\"true\"]}))\n",
- "(flow (fn [] nil))\n",
- "(flow (fn [] nil))\n",
+ "(local parallax (require :parallax))\n",
+ "(parallax {:variants {:desktop {:viewport [1280 900]}} :server (fn [] nil) :flow (fn [] nil)})\n",
+ "(parallax {:variants {:desktop {:viewport [1280 900]}} :server (fn [] nil) :flow (fn [] nil)})\n",
));
cargo_bin_cmd!("parallax")
.arg("list")
@@ -200,27 +172,7 @@ fn duplicate_flow_registration_fails() {
.arg(&path)
.assert()
.failure()
- .stderr(predicate::str::contains("config.fnl:5:").and(predicate::str::contains("exactly")));
-}
-
-#[test]
-fn malformed_flow_registration_fails() {
- let (_dir, path) = config_file(concat!(
- "(local {: setup : flow : server} (require :parallax))\n",
- "(setup {:variants {:desktop {:viewport [1280 900]}}})\n",
- "(server (fn [] {:cmd [\"true\"]}))\n",
- "(flow \"not-a-function\")\n",
- ));
- cargo_bin_cmd!("parallax")
- .arg("list")
- .arg("--config")
- .arg(&path)
- .assert()
- .failure()
- .stderr(
- predicate::str::contains("bad argument #1")
- .and(predicate::str::contains("config.fnl:4:")),
- );
+ .stderr(predicate::str::contains("config.fnl:3:").and(predicate::str::contains("exactly")));
}
#[test]