Make the UI load the newly imported map, scraping its name from the other tool. #523

For this to work, RunCommand has to be much more careful about getting
all output when the command is done.

importer/src/bin/one_step_import.rs

@@ -50,14 +50,13 @@ fn main() -> Result<()> {
     };
 
     // Name the temporary map based on the Geofabrik region.
-    let name = CityName::new("zz", "oneshot");
+    let city = CityName::new("zz", "oneshot");
-    let pbf = name.input_path(format!("osm/{}.pbf", abstutil::basename(&url)));
+    let name = abstutil::basename(&url)
-    let osm = name.input_path(format!(
+        .strip_suffix("-latest.osm")
-        "osm/{}.osm",
+        .unwrap()
-        abstutil::basename(&url)
+        .to_string();
-            .strip_suffix("-latest.osm")
+    let pbf = city.input_path(format!("osm/{}.pbf", abstutil::basename(&url)));
-            .unwrap()
+    let osm = city.input_path(format!("osm/{}.osm", name));
-    ));
     std::fs::create_dir_all(std::path::Path::new(&osm).parent().unwrap())
         .expect("Creating parent dir failed");
 
@@ -97,5 +96,8 @@ fn main() -> Result<()> {
     // debugging.
     abstio::delete_file("boundary0.poly");
 
+    // For the sake of the UI, print the name of the new map as the last line of output.
+    println!("{}", name);
+
     Ok(())
 }

map_gui/src/tools/command.rs

@@ -13,13 +13,15 @@ use crate::AppLike;
 /// native, of course.
 pub struct RunCommand<A: AppLike> {
     p: Popen,
-    comm: Communicator,
+    // Only wrapped in an Option so we can modify it when we're almost done.
+    comm: Option<Communicator>,
     panel: Panel,
     lines: VecDeque<String>,
     max_capacity: usize,
     started: Instant,
-    // Wrapped in an Option just to make calling from event() work. The bool is success.
+    // Wrapped in an Option just to make calling from event() work. The bool is success, and the
-    on_load: Option<Box<dyn FnOnce(&mut EventCtx, &mut A, bool) -> Transition<A>>>,
+    // strings are the last lines of output.
+    on_load: Option<Box<dyn FnOnce(&mut EventCtx, &mut A, bool, Vec<String>) -> Transition<A>>>,
 }
 
 impl<A: AppLike + 'static> RunCommand<A> {
@@ -27,8 +29,9 @@ impl<A: AppLike + 'static> RunCommand<A> {
         ctx: &mut EventCtx,
         _: &A,
         args: Vec<&str>,
-        on_load: Box<dyn FnOnce(&mut EventCtx, &mut A, bool) -> Transition<A>>,
+        on_load: Box<dyn FnOnce(&mut EventCtx, &mut A, bool, Vec<String>) -> Transition<A>>,
     ) -> Box<dyn State<A>> {
+        info!("RunCommand: {}", args.join(" "));
         match subprocess::Popen::create(
             &args,
             subprocess::PopenConfig {
@@ -38,9 +41,10 @@ impl<A: AppLike + 'static> RunCommand<A> {
             },
         ) {
             Ok(mut p) => {
-                let comm = p
+                let comm = Some(
-                    .communicate_start(None)
+                    p.communicate_start(None)
-                    .limit_time(Duration::from_millis(0));
+                        .limit_time(Duration::from_millis(0)),
+                );
                 let panel = ctx.make_loading_screen(Text::from(Line("Starting command...")));
                 let max_capacity =
                     (0.8 * ctx.canvas.window_height / ctx.default_line_height()) as usize;
@@ -61,17 +65,10 @@ impl<A: AppLike + 'static> RunCommand<A> {
             ),
         }
     }
-}
-
-impl<A: AppLike + 'static> State<A> for RunCommand<A> {
-    fn event(&mut self, ctx: &mut EventCtx, app: &mut A) -> Transition<A> {
-        ctx.request_update(UpdateType::Game);
-        if ctx.input.nonblocking_is_update_event().is_none() {
-            return Transition::Keep;
-        }
 
+    fn read_output(&mut self) {
         let mut new_lines = Vec::new();
-        let (stdout, stderr) = match self.comm.read() {
+        let (stdout, stderr) = match self.comm.as_mut().unwrap().read() {
             Ok(pair) => pair,
             // This is almost always a timeout.
             Err(err) => err.capture,
@@ -95,6 +92,17 @@ impl<A: AppLike + 'static> State<A> for RunCommand<A> {
                 self.lines.push_back(line);
             }
         }
+    }
+}
+
+impl<A: AppLike + 'static> State<A> for RunCommand<A> {
+    fn event(&mut self, ctx: &mut EventCtx, app: &mut A) -> Transition<A> {
+        ctx.request_update(UpdateType::Game);
+        if ctx.input.nonblocking_is_update_event().is_none() {
+            return Transition::Keep;
+        }
+
+        self.read_output();
 
         // Throttle rerendering?
         let mut txt = Text::from(
@@ -110,14 +118,27 @@ impl<A: AppLike + 'static> State<A> for RunCommand<A> {
         self.panel = ctx.make_loading_screen(txt);
 
         if let Some(status) = self.p.poll() {
+            // Make sure to grab all remaining output.
+            let comm = self.comm.take().unwrap();
+            self.comm = Some(comm.limit_time(Duration::from_secs(10)));
+            self.read_output();
+            // TODO Possible hack -- why is this last line empty?
+            if self.lines.back().map(|x| x.is_empty()).unwrap_or(false) {
+                self.lines.pop_back();
+            }
+
             let success = status.success();
+            let mut lines: Vec<String> = self.lines.drain(..).collect();
+            if !success {
+                lines.push(format!("Command failed: {:?}", status));
+            }
             return Transition::Multi(vec![
                 Transition::Pop,
-                (self.on_load.take().unwrap())(ctx, app, success),
+                (self.on_load.take().unwrap())(ctx, app, success, lines.clone()),
                 Transition::Push(PopupMsg::new(
                     ctx,
                     if success { "Success!" } else { "Failure!" },
-                    self.lines.drain(..).collect(),
+                    lines,
                 )),
             ]);
         }

map_gui/src/tools/importer.rs

@@ -91,7 +91,7 @@ impl<A: AppLike + 'static> State<A> for ImportCity<A> {
                             ctx,
                             app,
                             args,
-                            Box::new(|_, _, success| {
+                            Box::new(|_, _, success, mut lines| {
                                 if success {
                                     abstio::delete_file("boundary.geojson");
 
@@ -99,13 +99,11 @@ impl<A: AppLike + 'static> State<A> for ImportCity<A> {
                                         let mut import =
                                             state.downcast::<ImportCity<A>>().ok().unwrap();
                                         let on_load = import.on_load.take().unwrap();
-                                        vec![MapLoader::new(
+                                        // one_step_import prints the name of the map as the last
-                                            ctx,
+                                        // line.
-                                            app,
+                                        let name =
-                                            // TODO The name is hardcoded for now
+                                            MapName::new("zz", "oneshot", &lines.pop().unwrap());
-                                            MapName::new("zz", "oneshot", "clipped"),
+                                        vec![MapLoader::new(ctx, app, name, on_load)]
-                                            on_load,
-                                        )]
                                     }))
                                 } else {
                                     // The popup already explained the failure