apply review feedback

src/multi.rs

@@ -134,7 +134,7 @@ impl MultiProgress {
 
         let idx = state.insert(location);
         pb.set_draw_target(ProgressDrawTarget::new_remote(self.state.clone(), idx));
-        state.elements.get_mut(idx).unwrap().pb = pb.weak_bar_state();
+        state.members.get_mut(idx).unwrap().pb = pb.weak_bar_state();
         pb
     }
 
@@ -173,9 +173,9 @@ impl MultiProgress {
 #[derive(Debug)]
 pub(crate) struct MultiState {
     /// The collection of states corresponding to progress bars
-    /// the state is None for bars that have not yet been drawn or have been removed
-    elements: Vec<MultiStateElement>,
-    /// Set of removed bars, should have corresponding `None` elements in the `elements` vector
+    members: Vec<MultiStateMember>,
+    /// Set of removed bars, should have corresponding members in the `members` vector with a
+    /// `draw_state` of `None`.
     free_set: Vec<usize>,
     /// Indices to the `draw_states` to maintain correct visual order
     ordering: Vec<usize>,
@@ -192,7 +192,7 @@ pub(crate) struct MultiState {
 impl MultiState {
     fn new(draw_target: ProgressDrawTarget) -> Self {
         Self {
-            elements: vec![],
+            members: vec![],
             free_set: vec![],
             ordering: vec![],
             draw_target,
@@ -208,21 +208,21 @@ impl MultiState {
         extra_lines: Option<Vec<String>>,
         now: Instant,
     ) -> io::Result<()> {
-        // Reap all consecutive 'zombie' progress bars from head of elements
+        // Reap all consecutive 'zombie' progress bars from head of the list
         let mut zombies = vec![];
         let mut adjust = 0;
         for index in self.ordering.iter() {
-            let element = &self.elements[*index];
-            if element.is_zombie {
-                zombies.push(*index);
-                adjust += element
-                    .draw_state
-                    .as_ref()
-                    .map(|d| d.lines.len())
-                    .unwrap_or_default();
-            } else {
+            let member = &self.members[*index];
+            if !member.is_zombie {
                 break;
             }
+
+            zombies.push(*index);
+            adjust += member
+                .draw_state
+                .as_ref()
+                .map(|d| d.lines.len())
+                .unwrap_or_default();
         }
 
         for index in zombies {
@@ -253,14 +253,14 @@ impl MultiState {
         draw_state.lines.append(&mut self.orphan_lines);
 
         for index in self.ordering.iter() {
-            let element = &mut self.elements[*index];
-            if let Some(state) = &element.draw_state {
+            let member = &mut self.members[*index];
+            if let Some(state) = &member.draw_state {
                 draw_state.lines.extend_from_slice(&state.lines[..]);
             }
 
             // Mark the dead progress bar as a zombie - will be reaped on next draw
-            if element.pb.upgrade().is_none() {
-                element.is_zombie = true;
+            if member.pb.upgrade().is_none() {
+                member.is_zombie = true;
             }
         }
 
@@ -281,8 +281,8 @@ impl MultiState {
     }
 
     pub(crate) fn draw_state(&mut self, idx: usize) -> DrawStateWrapper<'_> {
-        let element = self.elements.get_mut(idx).unwrap();
-        let state = element.draw_state.get_or_insert(DrawState {
+        let member = self.members.get_mut(idx).unwrap();
+        let state = member.draw_state.get_or_insert(DrawState {
             move_cursor: self.move_cursor,
             alignment: self.alignment,
             ..Default::default()
@@ -309,12 +309,12 @@ impl MultiState {
     fn insert(&mut self, location: InsertLocation) -> usize {
         let idx = match self.free_set.pop() {
             Some(idx) => {
-                self.elements[idx] = MultiStateElement::default();
+                self.members[idx] = MultiStateMember::default();
                 idx
             }
             None => {
-                self.elements.push(MultiStateElement::default());
-                self.elements.len() - 1
+                self.members.push(MultiStateMember::default());
+                self.members.len() - 1
             }
         };
 
@@ -359,7 +359,7 @@ impl MultiState {
             return;
         }
 
-        self.elements[idx] = MultiStateElement::default();
+        self.members[idx] = MultiStateMember::default();
         self.free_set.push(idx);
         self.ordering.retain(|&x| x != idx);
 
@@ -371,18 +371,21 @@ impl MultiState {
     }
 
     fn len(&self) -> usize {
-        self.elements.len() - self.free_set.len()
+        self.members.len() - self.free_set.len()
     }
 }
 
 #[derive(Default)]
-struct MultiStateElement {
+struct MultiStateMember {
+    /// Draw state will be `None` for members that haven't been drawn before, or for entries that
+    /// correspond to something in the free set.
     draw_state: Option<DrawState>,
+    /// This will be a valid reference unless the containing member is actually in the free set.
     pb: Weak<Mutex<BarState>>,
     is_zombie: bool,
 }
 
-impl Debug for MultiStateElement {
+impl Debug for MultiStateMember {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("MultiStateElement")
             .field("draw_state", &self.draw_state)
@@ -469,19 +472,19 @@ mod tests {
 
         let state = mp.state.read().unwrap();
         // the removed place for p1 is reused
-        assert_eq!(state.elements.len(), 4);
+        assert_eq!(state.members.len(), 4);
         assert_eq!(state.len(), 3);
 
         // free_set may contain 1 or 2
         match state.free_set.last() {
             Some(1) => {
                 assert_eq!(state.ordering, vec![0, 2, 3]);
-                assert!(state.elements[1].draw_state.is_none());
+                assert!(state.members[1].draw_state.is_none());
                 assert_eq!(p4.index().unwrap(), 2);
             }
             Some(2) => {
                 assert_eq!(state.ordering, vec![0, 1, 3]);
-                assert!(state.elements[2].draw_state.is_none());
+                assert!(state.members[2].draw_state.is_none());
                 assert_eq!(p4.index().unwrap(), 1);
             }
             _ => unreachable!(),
@@ -581,10 +584,10 @@ mod tests {
 
         let state = mp.state.read().unwrap();
         // the removed place for p1 is reused
-        assert_eq!(state.elements.len(), 2);
+        assert_eq!(state.members.len(), 2);
         assert_eq!(state.free_set.len(), 1);
         assert_eq!(state.len(), 1);
-        assert!(state.elements[0].draw_state.is_none());
+        assert!(state.members[0].draw_state.is_none());
         assert_eq!(state.free_set.last(), Some(&0));
 
         assert_eq!(state.ordering, vec![1]);