memoryd, proto, brokerd, bxctl: repeated error lines written once

Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)

crates/brokerd/src/builtin.rs

@@ -25,16 +25,13 @@ pub fn grants() -> Result<Vec<LoadedGrant>, GrantProblem> {
     let sources: &[(&str, &str)] = &[("builtin-memory", MEMORY), ("builtin-recall", RECALL)];
     let mut out: Vec<LoadedGrant> = Vec::new();
     for (id, text) in sources {
-        let grant = toml::from_str::<Grant>(text).map_err(|error| GrantProblem {
+        let problem = |error: String| GrantProblem {
             file: id.to_string(),
             line: None,
-            problem: error.to_string(),
-        })?;
-        let sha256 = sha256(text.as_bytes()).map_err(|error| GrantProblem {
-            file: id.to_string(),
-            line: None,
-            problem: error.to_string(),
-        })?;
+            problem: error,
+        };
+        let grant = toml::from_str::<Grant>(text).map_err(|e| problem(e.to_string()))?;
+        let sha256 = sha256(text.as_bytes()).map_err(|e| problem(e.to_string()))?;
         out.push(LoadedGrant {
             id: id.to_string(),
             grant,

crates/bxctl/src/memory.rs

@@ -98,14 +98,15 @@ fn grouped(n: u64) -> String {
 
 /// Ask `memoryd` and print its answer. `Err` is the line for stderr.
 pub fn run(socket: &Path, ask: &Ask, timeout: Duration, out: &mut dyn Write) -> Result<(), String> {
-    let mut stream = UnixStream::connect(socket)
-        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;
+    let unreachable =
+        |e: &dyn std::fmt::Display| format!("cannot reach memoryd at {}: {e}", socket.display());
+    let mut stream = UnixStream::connect(socket).map_err(|e| unreachable(&e))?;
     stream
         .set_read_timeout(Some(timeout))
-        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;
+        .map_err(|e| unreachable(&e))?;
     stream
         .set_write_timeout(Some(timeout))
-        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;
+        .map_err(|e| unreachable(&e))?;
 
     let msg = match ask {
         Ask::Reindex => Message::Reindex(Empty {}),
@@ -120,8 +121,7 @@ pub fn run(socket: &Path, ask: &Ask, timeout: Duration, out: &mut dyn Write) ->
         r#final: true,
         msg,
     };
-    write_frame(&mut stream, &env)
-        .map_err(|e| format!("cannot reach memoryd at {}: {e}", socket.display()))?;
+    write_frame(&mut stream, &env).map_err(|e| unreachable(&e))?;
 
     let reply = read_frame(&mut stream).map_err(|e| format!("memoryd did not answer: {e}"))?;
     let text = match reply.msg {

crates/memoryd/src/edit.rs

@@ -236,6 +236,25 @@ fn noted(
     })
 }
 
+/// Mark the entry `old` names as named: stale if `before` has no such entry, an error if it was
+/// named already.
+fn claim(
+    file: MemoryFile,
+    before: &[String],
+    named: &mut HashSet<usize>,
+    old: &str,
+) -> Result<(), String> {
+    let pos = position(before, old).ok_or_else(|| stale(file))?;
+    if named.insert(pos) {
+        Ok(())
+    } else {
+        Err(format!(
+            "consolidate names an entry of {} twice",
+            file.name()
+        ))
+    }
+}
+
 fn consolidate(
     file: MemoryFile,
     before: &[String],
@@ -249,43 +268,19 @@ fn consolidate(
         let action = line.action().map_err(|e| e.to_string())?;
         match action {
             proto::EntryAction::Keep(old) => {
-                let pos = position(before, &old).ok_or_else(|| stale(file))?;
-                if !named.insert(pos) {
-                    return Err(format!(
-                        "consolidate names an entry of {} twice",
-                        file.name()
-                    ));
-                }
+                claim(file, before, &mut named, &old)?;
                 after.push(old.trim().to_string());
             }
             proto::EntryAction::Rewrite { old, text } => {
-                let pos = position(before, &old).ok_or_else(|| stale(file))?;
-                if !named.insert(pos) {
-                    return Err(format!(
-                        "consolidate names an entry of {} twice",
-                        file.name()
-                    ));
-                }
+                claim(file, before, &mut named, &old)?;
                 after.push(checked(&text)?);
             }
             proto::EntryAction::Move(old) => {
-                let pos = position(before, &old).ok_or_else(|| stale(file))?;
-                if !named.insert(pos) {
-                    return Err(format!(
-                        "consolidate names an entry of {} twice",
-                        file.name()
-                    ));
-                }
+                claim(file, before, &mut named, &old)?;
                 out.push(note(ctx, Kind::Moved(file), old.trim()));
             }
             proto::EntryAction::Drop(old) => {
-                let pos = position(before, &old).ok_or_else(|| stale(file))?;
-                if !named.insert(pos) {
-                    return Err(format!(
-                        "consolidate names an entry of {} twice",
-                        file.name()
-                    ));
-                }
+                claim(file, before, &mut named, &old)?;
                 out.push(note(ctx, Kind::Dropped(file), old.trim()));
             }
             proto::EntryAction::New(text) => {

crates/proto/src/memory.rs

@@ -165,6 +165,14 @@ pub fn entry_problem(text: &str, max: usize) -> Option<String> {
     None
 }
 
+/// `entry_problem` as a `Result`, so a check can use `?`.
+fn entry(text: &str, max: usize) -> Result<(), String> {
+    match entry_problem(text, max) {
+        Some(problem) => Err(problem),
+        None => Ok(()),
+    }
+}
+
 impl RecallArgs {
     /// The shape rules of `recall` (M5a spec, section 5).
     pub fn check(&self) -> Result<(), String> {
@@ -183,61 +191,32 @@ impl MemoryArgs {
     /// every `consolidate` line has one action and the list is not empty.
     pub fn check(&self) -> Result<(), String> {
         match self {
-            MemoryArgs::Add { text, .. } => {
-                if let Some(problem) = entry_problem(text, MAX_ENTRY) {
-                    return Err(problem);
-                }
-            }
+            MemoryArgs::Add { text, .. } => entry(text, MAX_ENTRY),
             MemoryArgs::Replace { old, text, .. } => {
-                if let Some(problem) = entry_problem(old, MAX_ENTRY) {
-                    return Err(problem);
-                }
-                if let Some(problem) = entry_problem(text, MAX_ENTRY) {
-                    return Err(problem);
-                }
-            }
-            MemoryArgs::Remove { old, .. } => {
-                if let Some(problem) = entry_problem(old, MAX_ENTRY) {
-                    return Err(problem);
-                }
-            }
-            MemoryArgs::Note { text } => {
-                if let Some(problem) = entry_problem(text, MAX_NOTE) {
-                    return Err(problem);
-                }
+                entry(old, MAX_ENTRY)?;
+                entry(text, MAX_ENTRY)
             }
+            MemoryArgs::Remove { old, .. } => entry(old, MAX_ENTRY),
+            MemoryArgs::Note { text } => entry(text, MAX_NOTE),
             MemoryArgs::Consolidate { entries, .. } => {
                 if entries.is_empty() {
                     return Err("consolidate needs every entry of the file".to_string());
                 }
                 for line in entries {
-                    let action = line.action().map_err(|problem| problem.to_string())?;
-                    match action {
+                    match line.action().map_err(str::to_string)? {
                         EntryAction::Keep(old)
                         | EntryAction::Move(old)
-                        | EntryAction::Drop(old) => {
-                            if let Some(problem) = entry_problem(&old, MAX_ENTRY) {
-                                return Err(problem);
-                            }
-                        }
+                        | EntryAction::Drop(old) => entry(&old, MAX_ENTRY)?,
                         EntryAction::Rewrite { old, text } => {
-                            if let Some(problem) = entry_problem(&old, MAX_ENTRY) {
-                                return Err(problem);
-                            }
-                            if let Some(problem) = entry_problem(&text, MAX_ENTRY) {
-                                return Err(problem);
-                            }
-                        }
-                        EntryAction::New(text) => {
-                            if let Some(problem) = entry_problem(&text, MAX_ENTRY) {
-                                return Err(problem);
-                            }
+                            entry(&old, MAX_ENTRY)?;
+                            entry(&text, MAX_ENTRY)?;
                         }
+                        EntryAction::New(text) => entry(&text, MAX_ENTRY)?,
                     }
                 }
+                Ok(())
             }
         }
-        Ok(())
     }
 }