Improve one-shot map importing UI (partly motivated by #760):

- stop overwriting the one zz/overpass map by naming them differently
- allow a user-specified name too
- move the buttons to search all maps and import a new place to the top
  of the ever-growing country list

cli/src/main.rs

@@ -146,8 +146,12 @@ enum Command {
     /// Imports a one-shot A/B Street map in a single command.
     OneStepImport {
         /// The path to a GeoJSON file with a boundary
-        #[structopt()]
+        #[structopt(long)]
         geojson_path: String,
+        /// What to name the new imported map. The country will always be "zz" (a fake country
+        /// code), with the city as "oneshot." This name shouldn't contain spaces or be empty.
+        #[structopt(long)]
+        map_name: String,
         /// Do people drive on the left side of the road in this map?
         #[structopt(long)]
         drive_on_left: bool,
@@ -206,9 +210,10 @@ async fn main() -> Result<()> {
         }
         Command::OneStepImport {
             geojson_path,
+            map_name,
             drive_on_left,
             use_geofabrik,
-        } => one_step_import::run(geojson_path, drive_on_left, use_geofabrik).await?,
+        } => one_step_import::run(geojson_path, map_name, drive_on_left, use_geofabrik).await?,
         Command::Import { raw_args } => importer::run(raw_args).await,
     }
     Ok(())

cli/src/one_step_import.rs

@@ -5,7 +5,19 @@ use anyhow::Result;
 use abstio::CityName;
 use geom::LonLat;
 
-pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool) -> Result<()> {
+pub async fn run(
+    geojson_path: String,
+    name: String,
+    drive_on_left: bool,
+    use_geofabrik: bool,
+) -> Result<()> {
+    if name.contains(" ") || name.is_empty() {
+        panic!(
+            "--map_name must be non-empty and contain no spaces: {}",
+            name
+        );
+    }
+
     // Convert to a boundary polygon.
     {
         println!("Converting GeoJSON to Osmosis boundary");
@@ -22,11 +34,9 @@ pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool)
     }
 
     let city = CityName::new("zz", "oneshot");
-    let name;
     let osm;
     if !use_geofabrik {
-        // No easy guess on this without looking at the XML file
+        println!("Downloading OSM data from Overpass...");
-        name = "overpass".to_string();
         osm = city.input_path(format!("osm/{}.osm", name));
 
         let geojson = abstio::slurp_file(geojson_path)?;
@@ -48,11 +58,6 @@ pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool)
         println!("Figuring out what Geofabrik file contains your boundary");
         let url = crate::pick_geofabrik::run("boundary0.poly".to_string()).await?;
 
-        // Name the temporary map based on the Geofabrik region.
-        name = abstutil::basename(&url)
-            .strip_suffix("-latest.osm")
-            .unwrap()
-            .to_string();
         let pbf = city.input_path(format!("osm/{}.pbf", abstutil::basename(&url)));
         osm = city.input_path(format!("osm/{}.osm", name));
         std::fs::create_dir_all(std::path::Path::new(&osm).parent().unwrap())
@@ -87,8 +92,5 @@ pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool)
     // 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/city_picker.rs

@@ -4,9 +4,9 @@ use abstio::{CityName, Manifest, MapName};
 use geom::{Distance, Percent};
 use map_model::City;
 use widgetry::{
-    Autocomplete, ClickOutcome, ControlState, DrawBaselayer, DrawWithTooltips, EventCtx, GeomBatch,
+    lctrl, Autocomplete, ClickOutcome, ControlState, DrawBaselayer, DrawWithTooltips, EventCtx,
-    GfxCtx, Image, Key, Line, Outcome, Panel, RewriteColor, State, Text, TextExt, Transition,
+    GeomBatch, GfxCtx, Image, Key, Line, Outcome, Panel, RewriteColor, State, Text, TextExt,
-    Widget,
+    Transition, Widget,
 };
 
 use crate::load::{FileLoader, MapLoader};
@@ -135,13 +135,6 @@ impl<A: AppLike + 'static> CityPicker<A> {
                         );
                     }
                 }
-                other_places.push(
-                    ctx.style()
-                        .btn_outline
-                        .text("Search all maps")
-                        .hotkey(Key::Tab)
-                        .build_def(ctx),
-                );
 
                 Transition::Replace(Box::new(CityPicker {
                     on_load: Some(on_load),
@@ -150,12 +143,6 @@ impl<A: AppLike + 'static> CityPicker<A> {
                             Line("Select a district").small_heading().into_widget(ctx),
                             ctx.style().btn_close_widget(ctx),
                         ]),
-                        Widget::row(vec![
-                            Widget::col(other_places).centered_vert(),
-                            district_picker,
-                            Widget::col(this_city).centered_vert(),
-                        ]),
-                        "Don't see the place you want?".text_widget(ctx),
                         if cfg!(target_arch = "wasm32") {
                             // On web, this is a link, so it's styled appropriately.
                             ctx.style()
@@ -171,6 +158,16 @@ impl<A: AppLike + 'static> CityPicker<A> {
                                 .text("Import a new city into A/B Street")
                                 .build_widget(ctx, "import new city")
                         },
+                        ctx.style()
+                            .btn_outline
+                            .icon_text("system/assets/tools/search.svg", "Search all maps")
+                            .hotkey(lctrl(Key::F))
+                            .build_def(ctx),
+                        Widget::row(vec![
+                            Widget::col(other_places).centered_vert(),
+                            district_picker,
+                            Widget::col(this_city).centered_vert(),
+                        ]),
                     ]))
                     .build(ctx),
                 }))

map_gui/src/tools/importer.rs

@@ -5,7 +5,7 @@ use clipboard::{ClipboardContext, ClipboardProvider};
 
 use abstio::MapName;
 use widgetry::{
-    EventCtx, GfxCtx, Line, Outcome, Panel, State, TextExt, Toggle, Transition, Widget,
+    EventCtx, GfxCtx, Line, Outcome, Panel, State, TextBox, TextExt, Toggle, Transition, Widget,
 };
 
 use crate::load::MapLoader;
@@ -49,29 +49,24 @@ impl<A: AppLike + 'static> ImportCity<A> {
                     "Copy the JSON text on the right into your clipboard".text_widget(ctx),
                 ])
                 .margin_below(16),
+                Toggle::choice(
+                    ctx,
+                    "left handed driving",
+                    "drive on the left",
+                    "right",
+                    None,
+                    false,
+                ),
+                Toggle::choice(ctx, "source", "GeoFabrik", "Overpass (faster)", None, false),
                 Widget::row(vec![
-                    "Step 4)".text_widget(ctx).centered_vert(),
+                    "Name the map:".text_widget(ctx).centered_vert(),
-                    Toggle::choice(
+                    TextBox::widget(ctx, "new_map_name", generate_new_map_name(), true, 20),
-                        ctx,
-                        "left handed driving",
-                        "drive on the left",
-                        "right",
-                        None,
-                        false,
-                    ),
                 ]),
-                Widget::row(vec![
+                ctx.style()
-                    "Step 5)".text_widget(ctx).centered_vert(),
+                    .btn_solid_primary
-                    Toggle::choice(ctx, "source", "GeoFabrik", "Overpass (faster)", None, false),
+                    .text("Import the area from your clipboard")
-                ]),
+                    .build_def(ctx)
-                Widget::row(vec![
+                    .margin_below(32),
-                    "Step 6)".text_widget(ctx).centered_vert(),
-                    ctx.style()
-                        .btn_solid_primary
-                        .text("Import the area from your clipboard")
-                        .build_def(ctx),
-                ])
-                .margin_below(32),
                 ctx.style()
                     .btn_plain
                     .btn()
@@ -102,23 +97,26 @@ impl<A: AppLike + 'static> State<A> for ImportCity<A> {
                     Transition::Keep
                 }
                 "Import the area from your clipboard" => {
+                    let name = sanitize_name(self.panel.text_box("new_map_name"));
+
                     let mut args = vec![
                         find_exe("cli"),
                         "one-step-import".to_string(),
-                        "boundary.geojson".to_string(),
+                        "--geojson-path=boundary.geojson".to_string(),
+                        format!("--map-name={}", name),
                     ];
                     if self.panel.is_checked("left handed driving") {
-                        args.push("--drive_on_left".to_string());
+                        args.push("--drive-on-left".to_string());
                     }
                     if self.panel.is_checked("source") {
-                        args.push("--use_geofabrik".to_string());
+                        args.push("--use-geofabrik".to_string());
                     }
                     match grab_geojson_from_clipboard() {
                         Ok(()) => Transition::Push(crate::tools::RunCommand::new_state(
                             ctx,
                             true,
                             args,
-                            Box::new(|_, _, success, mut lines| {
+                            Box::new(|_, _, success, _| {
                                 if success {
                                     abstio::delete_file("boundary.geojson");
 
@@ -126,11 +124,8 @@ impl<A: AppLike + 'static> State<A> for ImportCity<A> {
                                         let mut state =
                                             state.downcast::<ImportCity<A>>().ok().unwrap();
                                         let on_load = state.on_load.take().unwrap();
-                                        // one_step_import prints the name of the map as the last
+                                        let map_name = MapName::new("zz", "oneshot", &name);
-                                        // line.
+                                        vec![MapLoader::new_state(ctx, app, map_name, on_load)]
-                                        let name =
-                                            MapName::new("zz", "oneshot", &lines.pop().unwrap());
-                                        vec![MapLoader::new_state(ctx, app, name, on_load)]
                                     }))
                                 } else {
                                     // The popup already explained the failure
@@ -179,3 +174,18 @@ fn grab_geojson_from_clipboard() -> Result<()> {
     write!(f, "{}", contents)?;
     Ok(())
 }
+
+fn sanitize_name(x: String) -> String {
+    x.replace(" ", "_")
+}
+
+fn generate_new_map_name() -> String {
+    let mut i = 0;
+    loop {
+        let name = format!("imported_{}", i);
+        if !abstio::file_exists(MapName::new("zz", "oneshot", &name).path()) {
+            return name;
+        }
+        i += 1;
+    }
+}