Merge pull request #192 from itsmunzir/fix/167-connect-hint

CLI: the connect hint names the port it actually tried (#167)

apps/lightcraft-cli/src/main.rs

@@ -416,39 +416,39 @@ fn run(args: &[String]) -> Result<(), String> {
     if steps.is_empty() {
         return Err("run: no command given (see `lightcraft-cli commands`)".into());
     }
-    let mut backend: Box<dyn Backend> = match connect {
-        Some(addr) => {
-            if demo || library.is_some() || !imports.is_empty() {
-                return Err("--demo, --library and --import apply to headless mode only".into());
-            }
-            Box::new(
-                Remote::connect(&addr)
-                    .map_err(|e| format!("LightCraft is not reachable at {addr} ({e}); start it with `lightcraft --control 7980`"))?,
-            )
-        }
-        None => {
-            let mut h = match &library {
-                Some(dir) => {
-                    let mut h = Headless::default();
-                    h.session.open_library(dir, demo).map_err(|e| library_error(dir, e))?;
-                    h
-                }
-                None if demo => Headless::demo(),
-                None => Headless::default(),
-            };
-            if !imports.is_empty() {
-                let paths = expand_paths(&imports);
-                let r = h.session.execute("library.import", &json!({"paths": paths})).map_err(|e| e.to_string())?;
-                // what follows acts on the imported photos (already-known files are reported as duplicates)
-                let mut ids: Vec<Value> = r["imported"].as_array().cloned().unwrap_or_default();
-                ids.extend(r["duplicates"].as_array().into_iter().flatten().filter_map(|d| d.get("existing").filter(|v| v.is_u64()).cloned()));
-                if let Some(first) = ids.first().cloned() {
-                    h.session.execute("library.select", &json!({"ids": ids, "active": first})).map_err(|e| e.to_string())?;
+    let mut backend: Box<dyn Backend> =
+        match connect {
+            Some(addr) => {
+                if demo || library.is_some() || !imports.is_empty() {
+                    return Err("--demo, --library and --import apply to headless mode only".into());
                 }
+                Box::new(Remote::connect(&addr).map_err(|e| {
+                    format!("LightCraft is not reachable at {addr} ({e}); start it with `lightcraft --control {}`", connect_port(&addr))
+                })?)
             }
-            Box::new(h)
-        }
-    };
+            None => {
+                let mut h = match &library {
+                    Some(dir) => {
+                        let mut h = Headless::default();
+                        h.session.open_library(dir, demo).map_err(|e| library_error(dir, e))?;
+                        h
+                    }
+                    None if demo => Headless::demo(),
+                    None => Headless::default(),
+                };
+                if !imports.is_empty() {
+                    let paths = expand_paths(&imports);
+                    let r = h.session.execute("library.import", &json!({"paths": paths})).map_err(|e| e.to_string())?;
+                    // what follows acts on the imported photos (already-known files are reported as duplicates)
+                    let mut ids: Vec<Value> = r["imported"].as_array().cloned().unwrap_or_default();
+                    ids.extend(r["duplicates"].as_array().into_iter().flatten().filter_map(|d| d.get("existing").filter(|v| v.is_u64()).cloned()));
+                    if let Some(first) = ids.first().cloned() {
+                        h.session.execute("library.select", &json!({"ids": ids, "active": first})).map_err(|e| e.to_string())?;
+                    }
+                }
+                Box::new(h)
+            }
+        };
     let mut out = std::io::stdout().lock();
     let mut failed = 0;
     for s in &steps {
@@ -474,6 +474,12 @@ fn take_value<'a>(args: &'a [String], i: &mut usize, flag: &str) -> Result<&'a s
     args.get(*i).map(String::as_str).ok_or_else(|| format!("{flag} needs a value"))
 }
 
+/// The port an `--connect` address names, for the recovery hint. The whole address when it has
+/// no port, so a hint is never built from a guess.
+fn connect_port(addr: &str) -> &str {
+    addr.rsplit_once(':').map_or(addr, |(_, port)| port)
+}
+
 fn render(args: &[String]) -> Result<(), String> {
     let mut input = None;
     let mut output = None;

apps/lightcraft-cli/tests/cli.rs

@@ -324,3 +324,13 @@ fn a_library_open_in_another_process_is_refused() {
     assert_eq!(lines[0]["ok"], true);
     let _ = std::fs::remove_dir_all(&lib);
 }
+
+/// Issue #167: when `--connect ADDR` cannot reach the app, the recovery hint must name the port
+/// ADDR actually names — following the old fixed "7980" left the retry connecting elsewhere.
+#[test]
+fn connect_failure_hint_names_the_attempted_port() {
+    let (ok, _lines, err) = run_cli(&["--connect", "127.0.0.1:18437", "ui.inspect"], None);
+    assert!(!ok);
+    assert!(err.contains("--control 18437"), "the hint must name the port it tried: {err}");
+    assert!(!err.contains("--control 7980"), "{err}");
+}