Refactor command dispatch and state handling.

Cargo.lock

@@ -1415,6 +1415,7 @@ dependencies = [
 name = "worldwideportal"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "console_error_panic_hook",
  "im",
  "itertools 0.13.0",

Cargo.toml

@@ -19,3 +19,4 @@ minicrossterm = { git = "https://git.blastmud.org/blasthavers/minicrossterm.git"
 thiserror = "1.0.63"
 console_error_panic_hook = "0.1.7"
 ouroboros = "0.18.4"
+anyhow = "1.0.86"

src/command_handler.rs

@@ -1,28 +1,25 @@
 use itertools::join;
-use std::{cell::RefCell, rc::Rc};
-use yew::Callback;
 
 use crate::{
-    echo_to_term_frame, lua_state::LuaState, parsing::parse_commands, RegisteredTermFrameLens,
+    echo_to_term_frame, lua_state::LuaState, parsing::parse_commands, GlobalCell, TermFrame,
-    TermFrame,
 };
 
 fn reentrant_command_handler(
     lua_state: &mut LuaState,
-    frames: &RegisteredTermFrameLens,
+    globals: &GlobalCell,
     term_frame: &TermFrame,
     command_in: &str,
 ) {
     web_sys::console::log_1(&"Inside command handler".into());
-    echo_to_term_frame(frames, term_frame, "Hello World!\n");
+    echo_to_term_frame(globals, term_frame, "Hello World!\n").unwrap_or(());
     for command in parse_commands(command_in).commands {
         match command.split_out_command() {
             None => (),
             Some((cmd, rest)) => {
                 if cmd == "##" {
                     match lua_state.execute(&join(rest.arguments.iter(), " ")) {
-                        Ok(msg) => echo_to_term_frame(frames, term_frame, &msg).unwrap_or(()),
+                        Ok(msg) => echo_to_term_frame(globals, term_frame, &msg).unwrap_or(()),
-                        Err(msg) => echo_to_term_frame(frames, term_frame, &msg).unwrap_or(()),
+                        Err(msg) => echo_to_term_frame(globals, term_frame, &msg).unwrap_or(()),
                     }
                 }
             }
@@ -30,28 +27,16 @@ fn reentrant_command_handler(
     }
 }
 
-fn command_handler(
+pub fn command_handler(globals: &GlobalCell, term_frame: &TermFrame, command_in: &str) {
-    lua_state: &RefCell<LuaState>,
+    match globals.lua_engine.try_borrow_mut() {
-    frames: &RegisteredTermFrameLens,
-    term_frame: &TermFrame,
-    command_in: String,
-) {
-    match lua_state.try_borrow_mut() {
         Err(_) => echo_to_term_frame(
-            frames,
+            globals,
             term_frame,
             "Attempt to re-enter command handler during processing.\n",
         )
         .unwrap_or(()), // Ignore error handling error.
         Ok(mut lua_state_m) => {
-            reentrant_command_handler(&mut lua_state_m, frames, term_frame, &command_in)
+            reentrant_command_handler(&mut lua_state_m, globals, term_frame, command_in)
         }
     }
 }
-
-pub fn make_command_handler_callback(
-    lua_state: Rc<RefCell<LuaState>>,
-    frames: RegisteredTermFrameLens,
-) -> Callback<(TermFrame, String), ()> {
-    Callback::from(move |(term, cmd)| command_handler(&lua_state, &frames, &term, cmd))
-}

src/lua_state.rs

@@ -1,6 +1,7 @@
+use anyhow::Error;
 use piccolo::{Callback, Closure, Context, Executor, IntoValue, Lua, StashedExecutor, Table};
 
-use crate::{echo_to_term_frame, RegisteredTermFrameLens, TermFrame};
+use crate::{echo_to_term_frame, GlobalCell, TermFrame};
 use std::str;
 
 pub struct LuaState {
@@ -9,22 +10,10 @@ pub struct LuaState {
 }
 
 impl LuaState {
-    pub fn setup(frames: RegisteredTermFrameLens) -> Result<LuaState, String> {
+    pub fn setup() -> Result<LuaState, String> {
         let mut interp = Lua::core();
-        let exec: StashedExecutor = interp.enter(|ctx| {
+        let exec: StashedExecutor =
-            let cmd_table = Table::new(&ctx);
+            interp.enter(|ctx| Ok::<StashedExecutor, String>(ctx.stash(Executor::new(ctx))))?;
-            cmd_table
-                .set(
-                    ctx,
-                    ctx.intern_static(b"echo_frame"),
-                    echo_frame(ctx, frames),
-                )
-                .map_err(|_| "Can't add command")?;
-            ctx.set_global(ctx.intern_static(b"commands").into_value(ctx), cmd_table)
-                .map(|_| ())
-                .map_err(|_| "Can't set commands key".to_owned())?;
-            Ok::<StashedExecutor, String>(ctx.stash(Executor::new(ctx)))
-        })?;
 
         Ok(LuaState { interp, exec })
     }
@@ -42,13 +31,37 @@ impl LuaState {
     }
 }
 
-fn echo_frame(ctx: Context, frames: RegisteredTermFrameLens) -> Callback {
+pub fn install_lua_globals(global: &GlobalCell) -> Result<(), String> {
+    global
+        .lua_engine
+        .borrow_mut()
+        .interp
+        .try_enter(|ctx| {
+            let cmd_table = Table::new(&ctx);
+            cmd_table
+                .set(
+                    ctx,
+                    ctx.intern_static(b"echo_frame"),
+                    echo_frame(ctx, global.clone()),
+                )
+                .map_err(|_| Error::msg("Can't add command"))?;
+            ctx.set_global(ctx.intern_static(b"commands").into_value(ctx), cmd_table)
+                .map(|_| ())
+                .map_err(|_| Error::msg("Can't set commands key"))?;
+            Ok(())
+        })
+        .map_err(|e| e.to_string())?;
+
+    Ok(())
+}
+
+fn echo_frame(ctx: Context, global: GlobalCell) -> Callback {
     Callback::from_fn(&ctx, move |ctx, _ex, mut stack| {
         let frame_no: u64 = stack.consume(ctx)?;
         let message: piccolo::String = stack.consume(ctx)?;
         let message_str = str::from_utf8(message.as_bytes())
             .map_err(|_| "Expected message to echo to be UTF-8.".into_value(ctx))?;
-        echo_to_term_frame(&frames, &TermFrame(frame_no), message_str)
+        echo_to_term_frame(&global, &TermFrame(frame_no), message_str)
             .map_err(|m| m.into_value(ctx))?;
         Ok(piccolo::CallbackReturn::Return)
     })

src/main.rs

@@ -8,15 +8,16 @@ pub mod lineengine;
 pub mod lua_state;
 pub mod parsing;
 pub mod term_view;
-use crate::command_handler::*;
+use crate::lua_state::{install_lua_globals, LuaState};
-use crate::lua_state::LuaState;
 use crate::term_view::*;
 
 #[derive(Properties)]
-struct GlobalState {
+pub struct GlobalState {
-    frame_registry: RegisteredTermFrames,
+    // No strong references allowed between each of these groups of state.
-    lua_engine: LuaState,
+    frame_registry: RefCell<RegisteredTermFrames>,
+    lua_engine: RefCell<LuaState>,
 }
+type GlobalCell = Rc<GlobalState>;
 
 // Used only for yew. Always equal since we don't want it to
 // actually look into GlobalState, changes there should never
@@ -29,15 +30,16 @@ impl PartialEq for GlobalState {
 
 #[function_component(App)]
 fn app() -> Html {
-    let global = use_mut_ref(|| Global {
+    let global = use_memo((), |_| GlobalState {
         frame_registry: RegisteredTermFrames::new().into(),
-        lua_engine: LuaState::setup(frames.clone()).expect("Can create interpreter"),
+        lua_engine: LuaState::setup().expect("Can create interpreter").into(),
     });
+    install_lua_globals(&global).expect("Couldn't install Lua globals");
 
     html! {
         <div class="vpane toplevel">
-            <TermView terminal={TermFrame(0)} frames={frames.clone()} handler={command_handler.clone()}/>
+            <TermView terminal={TermFrame(0)} global={global.clone()}/>
-            <TermView terminal={TermFrame(1)} frames={frames.clone()} handler={command_handler.clone()}/>
+            <TermView terminal={TermFrame(1)} global={global.clone()}/>
         </div>
     }
 }

src/term_view.rs

@@ -1,14 +1,14 @@
-use std::{
+use std::collections::HashMap;
-    cell::RefCell,
-    rc::{Rc, Weak},
-};
 
-use im::hashmap::*;
 use wasm_bindgen::prelude::*;
 use web_sys::{Element, Node};
 use yew::prelude::*;
 
-use crate::lineengine::line::{Readline, ReadlineEvent};
+use crate::{
+    command_handler::command_handler,
+    lineengine::line::{Readline, ReadlineEvent},
+    GlobalCell,
+};
 
 #[wasm_bindgen]
 extern "C" {
@@ -73,8 +73,8 @@ pub struct TermFrameData {
     pub term: Terminal,
     pub fit: FitAddon,
     pub node: Node,
-    pub readline: RefCell<Readline>,
+    pub readline: Readline,
-    pub retained_closures: RefCell<Option<(Closure<dyn FnMut(String)>, Closure<dyn FnMut(Dims)>)>>,
+    pub retained_closures: Option<(Closure<dyn FnMut(String)>, Closure<dyn FnMut(Dims)>)>,
 }
 
 impl PartialEq for TermFrameData {
@@ -84,109 +84,114 @@ impl PartialEq for TermFrameData {
     }
 }
 
-pub type RegisteredTermFrames = HashMap<TermFrame, Rc<TermFrameData>>;
+pub type RegisteredTermFrames = HashMap<TermFrame, TermFrameData>;
-#[derive(Properties, PartialEq, Clone)]
-pub struct RegisteredTermFrameLens {
-    pub get: Callback<(), Rc<RegisteredTermFrames>>,
-    pub set: Callback<Rc<RegisteredTermFrames>, ()>,
-}
 
-fn get_or_make_term_frame(
+fn get_or_make_term_frame<'a>(
     frame: &TermFrame,
-    frames: &RegisteredTermFrameLens,
+    frames: &'a mut RegisteredTermFrames,
-    handler: &Callback<(TermFrame, String), ()>,
+    // Only used for callbacks, expected frames already borrowed mut!
-) -> Rc<TermFrameData> {
+    globals: &GlobalCell,
-    if let Some(tfd) = frames.get.emit(()).get(frame) {
+) -> &'a TermFrameData {
-        return tfd.clone();
+    frames.entry(frame.clone()).or_insert_with(|| {
-    }
+        let term = Terminal::new();
+        let fit = FitAddon::new();
+        let element = web_sys::window()
+            .and_then(|w| w.document())
+            .and_then(|d| d.create_element("div").ok())
+            .expect("Can create element for term");
+        element.set_class_name("hterminal");
+        term.open(&element);
+        term.loadAddon(&fit);
+        fit.fit();
+        for i in 0..100 {
+            term.write(&format!("{} Hello world\r\n", i));
+        }
+        let term_for_readline: Terminal = Terminal { obj: term.clone() };
+        let initial_size = (term.cols(), term.rows());
+        let mut new_data: TermFrameData = TermFrameData {
+            id: frame.clone(),
+            term: Terminal { obj: term.clone() },
+            fit,
+            node: element.into(),
+            readline: Readline::new(
+                "".to_owned(),
+                Box::new(move |dat| {
+                    term_for_readline.write(
+                        std::str::from_utf8(dat).expect("readline tried to emit invalid UTF-8"),
+                    )
+                }),
+                initial_size,
+            )
+            .into(),
+            retained_closures: None,
+        };
 
-    let mut new_frames: RegisteredTermFrames = (*frames.get.emit(())).clone();
+        let frame_for_resize: TermFrame = frame.clone();
-    let term = Terminal::new();
+        let globals_for_resize: GlobalCell = globals.clone();
-    let fit = FitAddon::new();
+        let resize_closure = Closure::new(move |dims: Dims| {
-    let element = web_sys::window()
+            globals_for_resize
-        .and_then(|w| w.document())
+                .frame_registry
-        .and_then(|d| d.create_element("div").ok())
+                .try_borrow_mut()
-        .expect("Can create element for term");
+                .ok()
-    element.set_class_name("hterminal");
+                .and_then(|mut frame_reg| {
-    term.open(&element);
+                    frame_reg.get_mut(&frame_for_resize).and_then(|frame| {
-    term.loadAddon(&fit);
+                        frame
-    fit.fit();
+                            .readline
-    for i in 0..100 {
+                            .handle_resize((dims.cols(), dims.rows()))
-        term.write(&format!("{} Hello world\r\n", i));
+                            .ok()
-    }
+                    })
-    let term_for_readline: Terminal = Terminal { obj: term.clone() };
+                })
-    let initial_size = (term.cols(), term.rows());
+                .unwrap_or(())
-    let new_data: Rc<TermFrameData> = TermFrameData {
+        });
-        id: frame.clone(),
+        term.onResize(&resize_closure);
-        term: Terminal { obj: term.clone() },
-        fit,
-        node: element.into(),
-        readline: Readline::new(
-            "".to_owned(),
-            Box::new(move |dat| {
-                term_for_readline
-                    .write(std::str::from_utf8(dat).expect("readline tried to emit invalid UTF-8"))
-            }),
-            initial_size,
-        )
-        .into(),
-        retained_closures: RefCell::new(None),
-    }
-    .into();
 
-    let data_for_resize: Weak<TermFrameData> = Rc::downgrade(&new_data);
+        let frame_for_ondata: TermFrame = frame.clone();
-    let resize_closure = Closure::new(move |dims: Dims| match Weak::upgrade(&data_for_resize) {
+        let globals_for_ondata: GlobalCell = globals.clone();
-        None => {}
+        let data_closure = Closure::new(move |d: String| {
-        Some(r) => match r.readline.try_borrow_mut() {
+            let new_lines: Vec<String> = match globals_for_ondata
-            Err(_) => {}
+                .frame_registry
-            Ok(mut v) => v.handle_resize((dims.cols(), dims.rows())).unwrap_or(()),
+                .try_borrow_mut()
-        },
+                .expect("Unexpected failure to borrow frame registry in onData")
-    });
+                .get_mut(&frame_for_ondata)
-    term.onResize(&resize_closure);
+            {
-
+                None => vec![],
-    let cloned_handler = (*handler).clone();
+                Some(frame) => frame
-    let emit_frame = frame.clone();
+                    .readline
-    let data_for_on_data: Weak<TermFrameData> = Rc::downgrade(&new_data);
+                    .readline(d.as_bytes())
-
+                    .expect("Readline failed")
-    let data_closure = Closure::new(move |d: String| match Weak::upgrade(&data_for_on_data) {
+                    .into_iter()
-        None => {}
+                    .filter_map(|ev| match ev {
-        Some(r) => match r.readline.try_borrow_mut() {
+                        ReadlineEvent::Line(l) => Some(l),
-            Err(_) => {}
+                        _ => None,
-            Ok(mut v) => {
+                    })
-                for ev in v.readline(d.as_bytes()).expect("Readline failed") {
+                    .collect(),
-                    match ev {
+            };
-                        ReadlineEvent::Line(l) => cloned_handler.emit((emit_frame.clone(), l)),
+            // We deliberately stop borrowing anything from RefCells here since command_handler has
-                        _ => {}
+            // lots of its own borrows, and we don't want them to conflict.
-                    }
+            for cmd in &new_lines {
-                }
+                command_handler(&globals_for_ondata, &frame_for_ondata, cmd);
             }
-        },
+        });
-    });
 
-    term.onData(&data_closure);
+        term.onData(&data_closure);
-    new_data
+        new_data
-        .retained_closures
+            .retained_closures
-        .replace(Some((data_closure, resize_closure)));
+            .replace((data_closure, resize_closure));
 
-    new_frames.insert(frame.clone(), new_data.clone());
+        new_data
-    web_sys::console::log_2(
+    })
-        &"Setting frames to have length: ".into(),
-        &new_frames.iter().count().into(),
-    );
-    frames.set.emit(new_frames.into());
-    new_data
 }
 
 #[derive(Properties, PartialEq)]
 pub struct TermViewProps {
     pub terminal: TermFrame,
-    pub frames: RegisteredTermFrameLens,
+    pub global: GlobalCell,
-    pub handler: Callback<(TermFrame, String), ()>,
 }
 
 #[function_component(TermView)]
 pub fn term_view(props: &TermViewProps) -> Html {
-    let term = get_or_make_term_frame(&props.terminal, &props.frames, &props.handler);
+    let mut frame_reg = props.global.frame_registry.borrow_mut();
+    let term = get_or_make_term_frame(&props.terminal, &mut frame_reg, &props.global);
     term.fit.fit();
     html! {
         {Html::VRef(term.node.clone())}
@@ -194,19 +199,22 @@ pub fn term_view(props: &TermViewProps) -> Html {
 }
 
 pub fn echo_to_term_frame(
-    frames: &RegisteredTermFrameLens,
+    global: &GlobalCell,
     frame_id: &TermFrame,
     message: &str,
 ) -> Result<(), &'static str> {
-    let frame_val = frames.get.emit(());
+    global
-    let frame: &TermFrameData = frame_val.get(frame_id).ok_or_else(|| {
+        .frame_registry
-        web_sys::console::log_3(
+        .borrow()
-            &"Attempt to echo to frame that doesn't exist.".into(),
+        .get(frame_id)
-            &frame_id.0.into(),
+        .ok_or_else(|| {
-            &frame_val.iter().count().into(),
+            web_sys::console::log_2(
-        );
+                &"Attempt to echo to frame that doesn't exist.".into(),
-        "Attempt to echo to frame that doesn't exist."
+                &frame_id.0.into(),
-    })?;
+            );
-    frame.term.write(message);
+            "Attempt to echo to frame that doesn't exist."
+        })?
+        .term
+        .write(message);
     Ok(())
 }