Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update doc comments in partial object API #1409

Merged
merged 3 commits into from Dec 1, 2022
Merged

Update doc comments in partial object API #1409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d819aa7
Remove obsolete implementation note
hannobraun Dec 1, 2022
bce5068
Update implementation note
hannobraun Dec 1, 2022
4e794a7
Simplify test code
hannobraun Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 2 additions & 8 deletions crates/fj-kernel/src/algorithms/approx/curve.rs
View file
Open in desktop
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
24 changes: 5 additions & 19 deletions crates/fj-kernel/src/partial/traits.rs
View file
Open in desktop
@@ -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<T>` 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<Full = Self>;
Expand Down Expand Up @@ -60,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:
/// <https://github.com/rust-lang/rust/issues/44265>
/// 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;
Expand Down