From d52118a3ffbc88546f27662cafef22ee2ea04b52 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 27 Oct 2022 15:06:15 +0200 Subject: [PATCH 1/4] Remove unused code --- crates/fj/src/models/context.rs | 203 -------------------------------- crates/fj/src/models/host.rs | 12 -- crates/fj/src/models/mod.rs | 4 +- 3 files changed, 1 insertion(+), 218 deletions(-) diff --git a/crates/fj/src/models/context.rs b/crates/fj/src/models/context.rs index e64848102..c6cc14558 100644 --- a/crates/fj/src/models/context.rs +++ b/crates/fj/src/models/context.rs @@ -1,11 +1,3 @@ -use std::{ - collections::HashMap, - fmt::{self, Display, Formatter}, - str::FromStr, -}; - -use crate::models::Error; - /// Contextual information passed to a [`Model`][crate::models::Model] when it /// is being initialized. /// @@ -15,201 +7,6 @@ pub trait Context { fn get_argument(&self, name: &str) -> Option<&str>; } -impl Context for &'_ C { - fn get_argument(&self, name: &str) -> Option<&str> { - (**self).get_argument(name) - } -} - -impl Context for Box { - fn get_argument(&self, name: &str) -> Option<&str> { - (**self).get_argument(name) - } -} - -impl Context for std::rc::Rc { - fn get_argument(&self, name: &str) -> Option<&str> { - (**self).get_argument(name) - } -} - -impl Context for std::sync::Arc { - fn get_argument(&self, name: &str) -> Option<&str> { - (**self).get_argument(name) - } -} - -impl Context for HashMap { - fn get_argument(&self, name: &str) -> Option<&str> { - self.get(name).map(|s| s.as_str()) - } -} - -/// Extension methods for the [`Context`] type. -/// -/// By splitting these methods out into a separate trait, [`Context`] can stay -/// object-safe while allowing convenience methods that use generics. -pub trait ContextExt { - /// Get an argument, returning a [`MissingArgument`] error if it doesn't - /// exist. - fn get_required_argument( - &self, - name: &str, - ) -> Result<&str, MissingArgument>; - - /// Parse an argument from its string representation using [`FromStr`]. - fn parse_argument(&self, name: &str) -> Result - where - T: FromStr, - T::Err: std::error::Error + Send + Sync + 'static; - - /// Try to parse an argument, if it is present. - fn parse_optional_argument( - &self, - name: &str, - ) -> Result, ParseFailed> - where - T: FromStr, - T::Err: std::error::Error + Send + Sync + 'static; -} - -impl ContextExt for C { - fn get_required_argument( - &self, - name: &str, - ) -> Result<&str, MissingArgument> { - self.get_argument(name).ok_or_else(|| MissingArgument { - name: name.to_string(), - }) - } - - fn parse_argument(&self, name: &str) -> Result - where - T: FromStr, - T::Err: std::error::Error + Send + Sync + 'static, - { - let value = self.get_required_argument(name)?; - - value - .parse() - .map_err(|e| ParseFailed { - name: name.to_string(), - value: value.to_string(), - error: Box::new(e), - }) - .map_err(ContextError::from) - } - - fn parse_optional_argument( - &self, - name: &str, - ) -> Result, ParseFailed> - where - T: FromStr, - T::Err: std::error::Error + Send + Sync + 'static, - { - let value = match self.get_argument(name) { - Some(value) => value, - None => return Ok(None), - }; - - let parsed = value.parse().map_err(|e| ParseFailed { - name: name.to_string(), - value: value.to_string(), - error: Box::new(e), - })?; - - Ok(Some(parsed)) - } -} - -/// An error that may be returned from a [`Context`] method. -#[derive(Debug)] -pub enum ContextError { - /// An argument was missing. - MissingArgument(MissingArgument), - /// An argument was present, but we were unable to parse it into the final - /// type. - ParseFailed(ParseFailed), -} - -impl From for ContextError { - fn from(m: MissingArgument) -> Self { - ContextError::MissingArgument(m) - } -} - -impl From for ContextError { - fn from(p: ParseFailed) -> Self { - ContextError::ParseFailed(p) - } -} - -impl Display for ContextError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - match self { - ContextError::MissingArgument(_) => { - write!(f, "An argument was missing") - } - ContextError::ParseFailed(_) => { - write!(f, "Unable to parse an argument") - } - } - } -} - -impl std::error::Error for ContextError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - ContextError::MissingArgument(m) => Some(m), - ContextError::ParseFailed(p) => Some(p), - } - } -} - -/// The error returned when a required argument wasn't provided. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct MissingArgument { - /// The argument's name. - pub name: String, -} - -impl Display for MissingArgument { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let MissingArgument { name } = self; - - write!(f, "The \"{name}\" argument was missing") - } -} - -impl std::error::Error for MissingArgument {} - -/// The error returned when [`ContextExt::parse_argument()`] is unable to parse -/// the argument's value. -#[derive(Debug)] -pub struct ParseFailed { - /// The argument's name. - pub name: String, - /// The actual value. - pub value: String, - /// The error that occurred. - pub error: Error, -} - -impl Display for ParseFailed { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let ParseFailed { name, value, .. } = self; - - write!(f, "Unable to parse the \"{name}\" argument (\"{value:?}\")") - } -} - -impl std::error::Error for ParseFailed { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - Some(&*self.error) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/fj/src/models/host.rs b/crates/fj/src/models/host.rs index 19072c881..2f4c2f6c6 100644 --- a/crates/fj/src/models/host.rs +++ b/crates/fj/src/models/host.rs @@ -11,18 +11,6 @@ pub trait Host { fn register_boxed_model(&mut self, model: Box); } -impl Host for &'_ mut H { - fn register_boxed_model(&mut self, model: Box) { - (*self).register_boxed_model(model); - } -} - -impl Host for Box { - fn register_boxed_model(&mut self, model: Box) { - (**self).register_boxed_model(model); - } -} - /// Extension methods to augment the [`Host`] API. pub trait HostExt { /// Register a model with the Fornjot runtime. diff --git a/crates/fj/src/models/mod.rs b/crates/fj/src/models/mod.rs index 03c033a40..904bf53ed 100644 --- a/crates/fj/src/models/mod.rs +++ b/crates/fj/src/models/mod.rs @@ -6,9 +6,7 @@ mod metadata; mod model; pub use self::{ - context::{ - Context, ContextError, ContextExt, MissingArgument, ParseFailed, - }, + context::Context, host::{Host, HostExt}, metadata::{ArgumentMetadata, Metadata, ModelMetadata}, model::Model, From cf7043a7238d42ea2b1df2ecfb89887ae3921cf2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Sat, 5 Nov 2022 09:12:35 +0100 Subject: [PATCH 2/4] Un-hide method in docs --- crates/fj/src/models/host.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/fj/src/models/host.rs b/crates/fj/src/models/host.rs index 2f4c2f6c6..cf9fb4556 100644 --- a/crates/fj/src/models/host.rs +++ b/crates/fj/src/models/host.rs @@ -7,7 +7,6 @@ pub trait Host { /// This is mainly for more advanced use cases (e.g. when you need to close /// over extra state to load the model). For simpler models, you probably /// want to use [`HostExt::register_model()`] instead. - #[doc(hidden)] fn register_boxed_model(&mut self, model: Box); } From 097c57fa5670547eea446554e1b6ad4e143e5ff6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Sat, 5 Nov 2022 09:25:36 +0100 Subject: [PATCH 3/4] Update doc comments --- crates/fj/src/models/context.rs | 2 -- crates/fj/src/models/host.rs | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj/src/models/context.rs b/crates/fj/src/models/context.rs index c6cc14558..332913d96 100644 --- a/crates/fj/src/models/context.rs +++ b/crates/fj/src/models/context.rs @@ -1,7 +1,5 @@ /// Contextual information passed to a [`Model`][crate::models::Model] when it /// is being initialized. -/// -/// Check out the [`ContextExt`] trait for some helper methods. pub trait Context { /// Get an argument that was passed to this model. fn get_argument(&self, name: &str) -> Option<&str>; diff --git a/crates/fj/src/models/host.rs b/crates/fj/src/models/host.rs index cf9fb4556..9af499dc1 100644 --- a/crates/fj/src/models/host.rs +++ b/crates/fj/src/models/host.rs @@ -11,6 +11,8 @@ pub trait Host { } /// Extension methods to augment the [`Host`] API. +/// +/// The purpose of this trait is to keep [`Host`] object-safe. pub trait HostExt { /// Register a model with the Fornjot runtime. fn register_model(&mut self, model: M) From f3c3e6324c0e66168f373f84583e28b5979446c2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Sat, 5 Nov 2022 09:38:43 +0100 Subject: [PATCH 4/4] Improve formatting --- crates/fj/src/models/metadata.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/fj/src/models/metadata.rs b/crates/fj/src/models/metadata.rs index dffa9fa92..398ab3be4 100644 --- a/crates/fj/src/models/metadata.rs +++ b/crates/fj/src/models/metadata.rs @@ -4,16 +4,22 @@ pub struct Metadata { /// A short, human-friendly name used to identify this module. pub name: String, + /// A semver-compliant version number. pub version: String, + /// A short, one-line description. pub short_description: Option, + /// A more elaborate description. pub description: Option, + /// A link to the homepage. pub homepage: Option, + /// A link to the source code. pub repository: Option, + /// The name of the software license(s) this software is released under. /// /// This is interpreted as a SPDX license expression (e.g. `MIT OR @@ -120,8 +126,10 @@ impl Metadata { pub struct ModelMetadata { /// A short, human-friendly name used to identify this model. pub name: String, + /// A description of what this model does. pub description: Option, + /// Arguments that the model uses when calculating its geometry. pub arguments: Vec, } @@ -171,9 +179,11 @@ impl ModelMetadata { pub struct ArgumentMetadata { /// The name used to refer to this argument. pub name: String, + /// A short description of this argument that could be shown to the user /// in something like a tooltip. pub description: Option, + /// Something that could be used as a default if no value was provided. pub default_value: Option, }