From 7355a71b4b933b1df6900e0fb7231874fe81a4a8 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Thu, 25 Apr 2024 10:01:37 +0100 Subject: [PATCH] Fix clippy Some of these suggestions are kind of bad but that's by-the-by --- crates/masonry/src/widget/flex.rs | 4 ++++ crates/xilem_masonry/src/lib.rs | 16 +++++++++++----- crates/xilem_masonry/src/sequence.rs | 11 +++++++---- crates/xilem_masonry/src/vec_splice.rs | 6 +++--- crates/xilem_masonry/src/view/flex.rs | 6 +++--- 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/crates/masonry/src/widget/flex.rs b/crates/masonry/src/widget/flex.rs index b83f4c821..2f289f132 100644 --- a/crates/masonry/src/widget/flex.rs +++ b/crates/masonry/src/widget/flex.rs @@ -237,6 +237,10 @@ impl Flex { pub fn len(&self) -> usize { self.children.len() } + + pub fn is_empty(&self) -> bool { + self.len() == 0 + } } // --- Mutate live Flex - WidgetMut --- diff --git a/crates/xilem_masonry/src/lib.rs b/crates/xilem_masonry/src/lib.rs index 392bff0b9..0ef328e5f 100644 --- a/crates/xilem_masonry/src/lib.rs +++ b/crates/xilem_masonry/src/lib.rs @@ -1,3 +1,4 @@ +#![allow(clippy::comparison_chain)] use std::{any::Any, collections::HashMap}; use masonry::{ @@ -49,10 +50,10 @@ masonry::declare_widget!(RootWidgetMut, RootWidget); impl Widget for RootWidget { fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) { - self.pod.on_pointer_event(ctx, event) + self.pod.on_pointer_event(ctx, event); } fn on_text_event(&mut self, ctx: &mut EventCtx, event: &TextEvent) { - self.pod.on_text_event(ctx, event) + self.pod.on_text_event(ctx, event); } fn on_status_change(&mut self, _: &mut LifeCycleCtx, _: &StatusChange) { @@ -60,7 +61,7 @@ impl Widget for RootWidget { } fn lifecycle(&mut self, ctx: &mut LifeCycleCtx, event: &LifeCycle) { - self.pod.lifecycle(ctx, event) + self.pod.lifecycle(ctx, event); } fn layout(&mut self, ctx: &mut LayoutCtx, bc: &BoxConstraints) -> Size { @@ -70,7 +71,7 @@ impl Widget for RootWidget { } fn paint(&mut self, ctx: &mut PaintCtx, scene: &mut Scene) { - self.pod.paint(ctx, scene) + self.pod.paint(ctx, scene); } fn children(&self) -> SmallVec<[WidgetRef<'_, dyn Widget>; 16]> { @@ -114,7 +115,8 @@ where let changed = next_view.rebuild(&mut self.view_cx, &self.current_view, element); if !changed.changed { - tracing::debug!("TODO: Skip some work?") + // Masonry manages all of this itself - ChangeFlags is probably not needed? + tracing::debug!("TODO: Skip some work?"); } self.current_view = next_view; } @@ -216,6 +218,10 @@ impl ChangeFlags { } pub struct ViewCx { + /// The map from a widgets id to its position in the View tree. + /// + /// This includes only the widgets which might send actions + /// This is currently never cleaned up widget_map: HashMap>, id_path: Vec, } diff --git a/crates/xilem_masonry/src/sequence.rs b/crates/xilem_masonry/src/sequence.rs index d0056248a..eefcee45a 100644 --- a/crates/xilem_masonry/src/sequence.rs +++ b/crates/xilem_masonry/src/sequence.rs @@ -4,12 +4,13 @@ use masonry::{widget::WidgetMut, Widget, WidgetPod}; use crate::{ChangeFlags, MasonryView, MessageResult, ViewCx, ViewId}; +#[allow(clippy::len_without_is_empty)] pub trait ElementSplice { /// Insert a new element at the current index in the resulting collection (and increment the index by 1) fn push(&mut self, element: WidgetPod>); /// Mutate the next existing element, and add it to the resulting collection (and increment the index by 1) // TODO: This should actually return `WidgetMut`, but that isn't supported in Masonry itself yet - fn mutate<'a>(&'a mut self) -> WidgetMut>; + fn mutate(&mut self) -> WidgetMut>; /// Delete the next n existing elements (this doesn't change the index) fn delete(&mut self, n: usize); /// Current length of the elements collection @@ -91,7 +92,7 @@ impl> ViewSequence, app_state: &mut State, ) -> MessageResult { - self.message(&id_path, message, app_state) + self.message(id_path, message, app_state) } fn count(&self) -> usize { @@ -140,7 +141,7 @@ impl> app_state: &mut State, ) -> MessageResult { if let Some(this) = self { - this.message(&id_path, message, app_state) + this.message(id_path, message, app_state) } else { MessageResult::Stale(message) } @@ -163,7 +164,7 @@ impl> ViewSequence(id), |cx| child.build(cx, elements)); - }) + }); } fn rebuild( @@ -187,6 +188,8 @@ impl> ViewSequence prev.len() { + // This suggestion from clippy is kind of bad, because we use the absolute index in the id + #[allow(clippy::needless_range_loop)] for ix in prev.len()..n { let id_u64: u64 = ix.try_into().unwrap(); let id = NonZeroU64::new(id_u64 + 1).unwrap(); diff --git a/crates/xilem_masonry/src/vec_splice.rs b/crates/xilem_masonry/src/vec_splice.rs index fef1dde23..f7333c40e 100644 --- a/crates/xilem_masonry/src/vec_splice.rs +++ b/crates/xilem_masonry/src/vec_splice.rs @@ -94,15 +94,15 @@ impl<'a, 'b, T> VecSplice<'a, 'b, T> { impl ElementSplice for VecSplice<'_, '_, masonry::WidgetPod>> { fn push(&mut self, element: masonry::WidgetPod>) { - self.push(element) + self.push(element); } - fn mutate<'a>(&'a mut self) -> WidgetMut> { + fn mutate(&mut self) -> WidgetMut> { unreachable!("VecSplice can only be used for `build`, not rebuild") } fn delete(&mut self, n: usize) { - self.delete(n) + self.delete(n); } fn len(&self) -> usize { diff --git a/crates/xilem_masonry/src/view/flex.rs b/crates/xilem_masonry/src/view/flex.rs index 59231bb5e..a96cce337 100644 --- a/crates/xilem_masonry/src/view/flex.rs +++ b/crates/xilem_masonry/src/view/flex.rs @@ -38,7 +38,7 @@ where "ViewSequence shouldn't leave splice in strange state" ); for item in elements.drain(..) { - view = view.with_child_pod(item); + view = view.with_child_pod(item).with_default_spacer(); } WidgetPod::new(view) } @@ -75,7 +75,7 @@ impl ElementSplice for FlexSplice<'_> { self.ix += 1; } - fn mutate<'a>(&'a mut self) -> WidgetMut<'a, Box> { + fn mutate(&mut self) -> WidgetMut> { #[cfg(debug_assertions)] let mut iterations = 0; #[cfg(debug_assertions)] @@ -109,7 +109,7 @@ impl ElementSplice for FlexSplice<'_> { deleted_count += 1; } } - self.element.remove_child(self.ix) + self.element.remove_child(self.ix); } }