GH-195: Cleanup

cli/src/error.rs

@@ -19,7 +19,7 @@ pub enum CliError {
     #[error("Core error: {0}")]
     Core(#[from] CoreError),
 
-    #[error("Cannot infer output format, please specifiy --format")]
+    #[error("Cannot infer output format, please specify --format")]
     UnknownOutputFormat,
 
     #[error("Reqwest error '{0}'")]
@@ -37,9 +37,6 @@ pub enum CliError {
     #[error("Could not get registry source")]
     Registry,
 
-    #[error("Specifier '{0}' not supported")]
-    Specifier(String),
-
     #[error("Could not find local path to '{0}'")]
     Local(String),
 

cli/src/main.rs

@@ -1,4 +1,3 @@
-use std::cell::RefCell;
 use std::{
     collections::HashMap,
     env,
@@ -35,11 +34,11 @@ use warp::{
     Filter, Rejection, Reply,
 };
 
-use crate::file_access::CliAccessManager;
 use error::CliError;
 use modmark_core::{context::CompilationState, eval, Context, CoreError, OutputFormat};
 use parser::{parse, Ast};
 
+use crate::file_access::CliAccessManager;
 use crate::package::PackageManager;
 
 mod error;
@@ -150,6 +149,7 @@ static RECEIVER: OnceCell<Mutex<Receiver<()>>> = OnceCell::new();
 static PREVIEW_PORT: OnceCell<Option<Port>> = OnceCell::new();
 static ABSOLUTE_OUTPUT_PATH: OnceCell<PathBuf> = OnceCell::new();
 static CONNECTION_ID_COUNTER: AtomicUsize = AtomicUsize::new(1);
+static MAX_COMPILATION_TRIES: usize = 3;
 
 type PreviewConnections = Arc<RwLock<HashMap<usize, mpsc::UnboundedSender<Message>>>>;
 type PreviewDoc = Arc<Mutex<String>>;
@@ -160,31 +160,27 @@ type CompilationResult = Result<(String, CompilationState, Ast), Vec<CoreError>>
 async fn compile_file(input_file: &Path, output_format: &OutputFormat) -> CompilationResult {
     let source = fs::read_to_string(input_file).map_err(|e| vec![e.into()])?;
     let ast = parse(&source).map_err(|e| vec![e.into()])?;
-    if let Some((output, state)) = eval(
-        &source,
-        &mut CTX.get().unwrap().lock().unwrap(),
-        output_format,
-    )? {
-        return Ok((output, state, ast));
-    }
 
-    spawn_blocking(|| {
-        RECEIVER.get().unwrap().lock().unwrap().blocking_recv();
-    })
-    .await
-    .unwrap();
+    for i in 1..=MAX_COMPILATION_TRIES {
+        if let Some((output, state)) = eval(
+            &source,
+            &mut CTX.get().unwrap().lock().unwrap(),
+            output_format,
+        )? {
+            return Ok((output, state, ast));
+        }
 
-    if let Some((output, state)) = eval(
-        &source,
-        &mut CTX.get().unwrap().lock().unwrap(),
-        output_format,
-    )? {
-        return Ok((output, state, ast));
+        if i != MAX_COMPILATION_TRIES {
+            spawn_blocking(|| {
+                RECEIVER.get().unwrap().lock().unwrap().blocking_recv();
+            })
+            .await
+            .unwrap();
+        }
     }
 
-    panic!("Something went wrong!!!")
+    Err(vec![])
 }
-
 fn print_compiling_message() -> Result<(), CliError> {
     let mut stdout = stdout();
     stdout.execute(terminal::Clear(terminal::ClearType::All))?;
@@ -202,11 +198,23 @@ fn print_result(result: &CompilationResult, args: &Args) -> Result<(), CliError>
         Err(errors) => {
             stdout.execute(terminal::Clear(terminal::ClearType::All))?;
             stdout.execute(cursor::MoveTo(0, 0))?;
-            stdout.execute(style::PrintStyledContent(
-                format!("{} compilation error(s):\n", errors.len()).red(),
-            ))?;
-            for error in errors {
-                stdout.execute(style::PrintStyledContent(format!("{error:?}\n").red()))?;
+            let num_errors = errors.len();
+            if num_errors == 0 {
+                stdout.execute(style::PrintStyledContent(
+                    "No result retrieved from compiler\n".red(),
+                ))?;
+            } else if num_errors == 1 {
+                let error = errors.first().unwrap();
+                stdout.execute(style::PrintStyledContent(
+                    format!("1 compilation error:\n{error}\n").red(),
+                ))?;
+            } else {
+                stdout.execute(style::PrintStyledContent(
+                    format!("{} compilation errors:\n", num_errors).red(),
+                ))?;
+                for error in errors {
+                    stdout.execute(style::PrintStyledContent(format!("{error:?}\n").red()))?;
+                }
             }
             return Ok(());
         }

cli/src/package.rs

@@ -21,20 +21,20 @@ pub struct PackageManager {
 
 impl Resolve for PackageManager {
     fn resolve_all(&self, paths: Vec<ResolveTask>) {
+        // I don't know how to get rid of all these clones...
         let self_clone = self.clone();
         tokio::spawn(async move {
-            let self_clone2 = self_clone.clone();
             join_all(
                 paths
                     .into_iter()
                     .map(|task| {
-                        let self_clone = self_clone.clone();
-                        tokio::spawn(async move { self_clone.clone().resolve(task).await })
+                        let another_self = self_clone.clone();
+                        tokio::spawn(async move { another_self.resolve(task).await })
                     })
                     .collect::<Vec<_>>(),
             )
             .await;
-            self_clone2.sender.send(()).await.unwrap();
+            self_clone.sender.send(()).await.unwrap();
         });
     }
 }

core/src/context.rs

@@ -80,10 +80,9 @@ impl CompilationState {
 impl<T, U> Debug for Context<T, U> {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         f.debug_struct("Context")
-            //            .field("native packages", &self.native_packages)
-            //            .field("standard packages", &self.standard_packages)
-            //            .field("external packages", &self.external_packages)
-            //            .field("transforms", &self.transforms)
+            .field("package manager", &self.package_manager)
+            .field("compilation state", &self.state)
+            .field("filesystem", &self.filesystem)
             .finish()
     }
 }
@@ -125,6 +124,7 @@ impl TransformVariant {
 }
 
 impl<T, U> Context<T, U> {
+    /// Creates a new Context with the given resolver and policy
     pub fn new(resolver: T, policy: U) -> Result<Self, CoreError>
     where
         T: Resolve,
@@ -178,6 +178,9 @@ where
             lock.expose_transforms(config.try_into()?)?;
             Ok(true)
         } else {
+            // IMPORTANT: It is important that we drop the lock here. If resolve_all were to resolve
+            // the packages in this thread, they would need to acquire the lock, which is impossible
+            // if it isn't dropped here and would result in a dead-lock
             drop(lock);
             self.resolver.resolve_all(missings);
             Ok(false)

core/src/package.rs

@@ -134,7 +134,7 @@ impl ArgType {
             ArgType::Enum(values) => value
                 .as_str()
                 .map(ToString::to_string)
-                .filter(|s| values.contains(&s))
+                .filter(|s| values.contains(s))
                 .map(ArgValue::EnumVariant)
                 .ok_or(CoreError::EnumVariant(values.clone(), value.to_string())),
             ArgType::Primitive(t) => t.try_from_value(value),

core/src/package_store.rs

@@ -179,7 +179,6 @@ impl PackageStore {
             .chain(self.external_packages.values())
             .map(|pkg| pkg.info.clone())
             .collect::<Vec<_>>()
-            .clone()
     }
 
     /// This gets the transform info for a given element and output format. If a native package
@@ -202,7 +201,7 @@ impl PackageStore {
         output_format: &OutputFormat,
     ) -> Result<Vec<ArgInfo>, CoreError> {
         self.get_transform_info(element_name, output_format)
-            .map(|info| info.arguments.clone())
+            .map(|info| info.arguments)
             .ok_or(CoreError::MissingTransform(
                 element_name.to_string(),
                 output_format.0.to_string(),
@@ -254,7 +253,7 @@ impl PackageStore {
             })
             .map(|id| ResolveTask {
                 manager: Arc::clone(&arc_mutex),
-                package: id.clone(),
+                package: id,
                 resolved: false,
             })
             .collect();
@@ -345,8 +344,8 @@ impl PackageStore {
         }
 
         mem::take(&mut config.0)
-            .into_iter()
-            .map(|(id, _)| CoreError::UnusedConfig(id.name))
+            .into_keys()
+            .map(|id| CoreError::UnusedConfig(id.name))
             .for_each(|e| errors.push(e));
 
         if errors.is_empty() {
@@ -401,7 +400,7 @@ impl PackageStore {
         Ok(())
     }
 
-    pub(crate) fn is_missing_packages(&self) -> bool {
+    pub fn is_missing_packages(&self) -> bool {
         !self.awaited_packages.is_empty()
     }
 
@@ -491,10 +490,6 @@ pub trait Resolve {
     fn resolve_all(&self, paths: Vec<ResolveTask>);
 }
 
-pub struct ResolveWrapper {
-    task: ResolveTask,
-}
-
 #[derive(Debug)]
 pub struct ResolveTask {
     manager: Arc<Mutex<PackageStore>>,
@@ -536,11 +531,18 @@ impl Drop for ResolveTask {
         if !self.resolved {
             let mut manager = self.manager.lock().unwrap();
             let package = mem::take(&mut self.package);
-            manager.reject_request(package.clone(), CoreError::DroppedRequest);
+            manager.reject_request(package, CoreError::DroppedRequest);
         }
     }
 }
 
+pub struct DenyAllResolver;
+impl Resolve for DenyAllResolver {
+    fn resolve_all(&self, _paths: Vec<ResolveTask>) {
+        // Dropping the tasks should reject them
+    }
+}
+
 #[derive(Debug, Clone, Hash, PartialEq, Eq, Default)]
 pub struct PackageID {
     pub name: String,

core/src/std_packages.rs

@@ -188,9 +188,9 @@ pub fn native_err<T, U>(
     // Push the issue to errors
     ctx.state.errors.push(Issue {
         source: source.to_string(),
-        target: target.to_string(),
+        target,
         description: body.to_string(),
-        input: input.map(|s| s.to_string()),
+        input,
     });
 
     // Check if we have an __error transform