Merge pull request #1268 from hannobraun/error

Improve error message when failing to load model

crates/fj-app/src/main.rs

@@ -14,14 +14,13 @@
 
 mod args;
 mod config;
+mod path;
 
-use std::path::PathBuf;
-
-use anyhow::{anyhow, Context as _};
 use fj_export::export;
-use fj_host::{Model, Parameters};
+use fj_host::Parameters;
 use fj_operations::shape_processor::ShapeProcessor;
 use fj_window::run::run;
+use path::ModelPath;
 use tracing_subscriber::fmt::format;
 use tracing_subscriber::EnvFilter;
 
@@ -43,37 +42,13 @@ fn main() -> anyhow::Result<()> {
 
     let args = Args::parse();
     let config = Config::load()?;
-    let path = config.default_path.unwrap_or_else(|| PathBuf::from(""));
+    let model_path = ModelPath::from_args_and_config(&args, &config)?;
     let parameters = args.parameters.unwrap_or_else(Parameters::empty);
     let shape_processor = ShapeProcessor {
         tolerance: args.tolerance,
     };
 
-    let path_of_model = path.canonicalize().unwrap_or_default();
-
-    let model = if let Some(model) =
-        args.model.or(config.default_model).as_ref()
-    {
-        let mut model_path = path;
-        model_path.push(model);
-        Model::new(model_path.clone(), parameters).with_context(|| {
-            if path_of_model.as_os_str().is_empty() {
-                format!(
-                    "Model is not defined, can't find model defined inside the default-model also, add model like \n cargo run -- -m {}", model.display()
-                )
-            } else {
-                format!(
-                "Failed to load model: {0}\ninside default models directory: '{1}'\nCan mainly caused by: \n1. Model '{2}' can not be found inside '{1}'\n2.'{2}' can be mis-typed see inside '{1}' for a match\n3. Define model is '{2}' couldn\'t be found ((defined in command-line arguments))", model_path.display(), path_of_model.display(), model.display()
-            )
-        }
-        })?
-    } else {
-        return Err(anyhow!(
-            "You must specify a model to start Fornjot.\n\
-            - Pass a model as a command-line argument. See `fj-app --help`.\n\
-            - Specify a default model in the configuration file."
-        ));
-    };
+    let model = model_path.load_model(parameters)?;
 
     if let Some(export_path) = args.export {
         // export only mode. just load model, process, export and exit

crates/fj-app/src/path.rs

@@ -0,0 +1,152 @@
+use std::{
+    fmt::{self, Write},
+    path::{Path, PathBuf},
+};
+
+use anyhow::{anyhow, Context};
+use fj_host::{Model, Parameters};
+
+use crate::{args::Args, config::Config};
+
+pub struct ModelPath {
+    default_path: Option<PathBuf>,
+    model_path: ModelPathSource,
+}
+
+impl ModelPath {
+    pub fn from_args_and_config(
+        args: &Args,
+        config: &Config,
+    ) -> anyhow::Result<Self> {
+        let default_path = config.default_path.clone();
+
+        let model_path_from_args = args
+            .model
+            .as_ref()
+            .map(|model| ModelPathSource::Args(model.clone()));
+        let model_path_from_config = config
+            .default_model
+            .as_ref()
+            .map(|model| ModelPathSource::Config(model.clone()));
+        let model_path = model_path_from_args
+            .or(model_path_from_config)
+            .ok_or_else(no_model_error)?;
+
+        Ok(Self {
+            default_path,
+            model_path,
+        })
+    }
+
+    pub fn load_model(&self, parameters: Parameters) -> anyhow::Result<Model> {
+        let default_path = self
+            .default_path
+            .as_ref()
+            .map(|path| -> anyhow::Result<_> {
+                let rel = path;
+                let abs = path.canonicalize().with_context(|| {
+                    format!(
+                        "Converting `default-path` from `fj.toml` (`{}`) into \
+                        absolute path",
+                        path.display(),
+                    )
+                })?;
+                Ok((rel, abs))
+            })
+            .transpose()?;
+
+        let path = default_path
+            .clone()
+            .map(|(_, abs)| abs)
+            .unwrap_or_else(PathBuf::new)
+            .join(self.model_path.path());
+
+        let model = Model::new(&path, parameters).with_context(|| {
+            load_error_context(default_path, &self.model_path, path)
+        })?;
+        Ok(model)
+    }
+}
+
+enum ModelPathSource {
+    Args(PathBuf),
+    Config(PathBuf),
+}
+
+impl ModelPathSource {
+    fn path(&self) -> &Path {
+        match self {
+            ModelPathSource::Args(path) => path,
+            ModelPathSource::Config(path) => path,
+        }
+    }
+}
+
+fn load_error_context(
+    default_path: Option<(&PathBuf, PathBuf)>,
+    model_path: &ModelPathSource,
+    path: PathBuf,
+) -> String {
+    load_error_context_inner(default_path, model_path, path)
+        .expect("Expected `write!` to `String` to never fail")
+}
+
+fn load_error_context_inner(
+    default_path: Option<(&PathBuf, PathBuf)>,
+    model_path: &ModelPathSource,
+    path: PathBuf,
+) -> Result<String, fmt::Error> {
+    let mut error = String::new();
+    write!(
+        error,
+        "Failed to load model: `{}`",
+        model_path.path().display()
+    )?;
+    match model_path {
+        ModelPathSource::Args(_) => {
+            write!(error, "\n- Passed via command-line argument")?
+        }
+        ModelPathSource::Config(_) => {
+            write!(error, "\n- Specified as default model in configuration")?
+        }
+    }
+    write!(error, "\n- Path of model: {}", path.display())?;
+
+    let mut suggestions = String::new();
+    write!(suggestions, "Suggestions:")?;
+    write!(
+        suggestions,
+        "\n- Did you mis-type the model path `{}`?",
+        model_path.path().display()
+    )?;
+
+    if let Some((default_path_rel, default_path_abs)) = &default_path {
+        write!(
+            error,
+            "\n- Searching inside default path from configuration: {}",
+            default_path_abs.display(),
+        )?;
+
+        write!(
+            suggestions,
+            "\n- Did you mis-type the default path `{}`?",
+            default_path_rel.display()
+        )?;
+        write!(
+            suggestions,
+            "\n- Did you accidentally pick up a local configuration file?"
+        )?;
+    }
+
+    let context = format!("{error}\n\n{suggestions}");
+
+    Ok(context)
+}
+
+fn no_model_error() -> anyhow::Error {
+    anyhow!(
+        "You must specify a model to start Fornjot.\n\
+        - Pass a model as a command-line argument. See `fj-app --help`.\n\
+        - Specify a default model in the configuration file."
+    )
+}

crates/fj-host/src/model.rs

@@ -22,7 +22,12 @@ impl Model {
     ///
     /// The path expected here is the root directory of the model's Cargo
     /// package, that is the folder containing `Cargo.toml`.
-    pub fn new(path: PathBuf, parameters: Parameters) -> Result<Self, Error> {
+    pub fn new(
+        path: impl AsRef<Path>,
+        parameters: Parameters,
+    ) -> Result<Self, Error> {
+        let path = path.as_ref();
+
         let crate_dir = path.canonicalize()?;
 
         let metadata = cargo_metadata::MetadataCommand::new()