From d819aa710e07021d51ce8cbde270e2acd5bc5a70 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 1 Dec 2022 13:14:06 +0100 Subject: [PATCH 1/3] Remove obsolete implementation note --- crates/fj-kernel/src/partial/traits.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/crates/fj-kernel/src/partial/traits.rs b/crates/fj-kernel/src/partial/traits.rs index 6d258e618..bb061d585 100644 --- a/crates/fj-kernel/src/partial/traits.rs +++ b/crates/fj-kernel/src/partial/traits.rs @@ -1,21 +1,6 @@ use crate::{objects::Objects, services::Service}; /// Implemented for objects that a partial object type exists for -/// -/// # Implementation Note -/// -/// This trait is usually implemented for object types themselves, but types -/// that are already managed in the centralized object storage ([#1021]) -/// implement this trait for `Handle` instead. This is necessary, due to the -/// use of this type in [`MaybePartial`], but leads to some not so nice -/// inconsistencies. -/// -/// Once [#1021] is addressed and all types are managed in the centralized -/// object storage, this should be changed to all object types implement this -/// directly. -/// -/// [#1021]: https://github.com/hannobraun/Fornjot/issues/1021 -/// [`MaybePartial`]: super::MaybePartial pub trait HasPartial { /// The type representing the partial variant of this object type Partial: Partial; From bce5068dbfd0ef01f8a68f0385a4da8d32fe25e9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 1 Dec 2022 13:13:39 +0100 Subject: [PATCH 2/3] Update implementation note --- crates/fj-kernel/src/partial/traits.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/partial/traits.rs b/crates/fj-kernel/src/partial/traits.rs index bb061d585..920b13aab 100644 --- a/crates/fj-kernel/src/partial/traits.rs +++ b/crates/fj-kernel/src/partial/traits.rs @@ -45,10 +45,11 @@ pub trait HasPartial { /// # Implementation Note /// /// It would be nicer to require an [`Into`] bound instead of [`From`] (see -/// documentation of those types for more information). But I think we'd need a -/// `where` clause on the associated type to specify that, which is unstable. It -/// should become stable soon though, together with generic associated types: -/// +/// documentation of those types for more information). Unfortunately, this +/// doesn't seem to be possible without a `where` clause on `HasPartial`, which +/// significantly reduces the convenience of using that trait, as the `where` +/// clause needs to be repeated everywhere `HasPartial` is specialized as a +/// bound. pub trait Partial: Default + for<'a> From<&'a Self::Full> { /// The type representing the full variant of this object type Full; From 4e794a70bfdb5498e6f3c1468f8769554a725938 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 1 Dec 2022 14:22:57 +0100 Subject: [PATCH 3/3] Simplify test code --- crates/fj-kernel/src/algorithms/approx/curve.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/curve.rs b/crates/fj-kernel/src/algorithms/approx/curve.rs index bd9401386..1b205d3e5 100644 --- a/crates/fj-kernel/src/algorithms/approx/curve.rs +++ b/crates/fj-kernel/src/algorithms/approx/curve.rs @@ -213,10 +213,7 @@ mod tests { fn approx_line_on_flat_surface() { let mut services = Services::new(); - let surface = - PartialSurface::from_axes(GlobalPath::x_axis(), [0., 0., 1.]) - .build(&services.objects) - .insert(&mut services.objects); + let surface = services.objects.surfaces.xz_plane(); let mut curve = PartialCurve { surface: Some(surface), ..Default::default() @@ -297,10 +294,7 @@ mod tests { fn approx_circle_on_flat_surface() { let mut services = Services::new(); - let surface = - PartialSurface::from_axes(GlobalPath::x_axis(), [0., 0., 1.]) - .build(&services.objects) - .insert(&mut services.objects); + let surface = services.objects.surfaces.xz_plane(); let mut curve = PartialCurve { surface: Some(surface), ..Default::default()