From 8d7da821e00e8c7d83f26f3701a738c3820cef30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Leonard=20Sch=C3=BCngel?= <schuengel.leonard@gmail.com>
Date: Sun, 11 Feb 2024 02:59:54 +0100
Subject: [PATCH] Adjust the api for filtering and mapping

- Node does not have an empty variant in this branch so the filter
method must return an option instead
- The question of how to handle the retain case is still open.
---
 src/dock_state/tree/mod.rs  |  8 +------
 src/dock_state/tree/node.rs | 47 ++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src/dock_state/tree/mod.rs b/src/dock_state/tree/mod.rs
index 710e8cb..ec0dcb6 100644
--- a/src/dock_state/tree/mod.rs
+++ b/src/dock_state/tree/mod.rs
@@ -714,13 +714,7 @@ impl<Tab> Tree<Tab> {
         } = self;
         let nodes = nodes
             .iter()
-            .filter_map(|node| {
-                let node = node.filter_map_tabs(function.clone());
-                match node {
-                    Node::Leaf { ref tabs, .. } => (!tabs.is_empty()).then_some(node),
-                    _ => Some(node),
-                }
-            })
+            .filter_map(|node| node.filter_map_tabs(function.clone()))
             .collect();
         Tree {
             nodes,
diff --git a/src/dock_state/tree/node.rs b/src/dock_state/tree/node.rs
index 8a35f55..b0f5ea9 100644
--- a/src/dock_state/tree/node.rs
+++ b/src/dock_state/tree/node.rs
@@ -272,8 +272,8 @@ impl<Tab> Node<Tab> {
     }
 
     /// Returns a new [`Node`] while mapping and filtering the tab type.
-    /// If this [`Node`] remains empty, it will change to [`Node::Empty`].
-    pub fn filter_map_tabs<F, NewTab>(&self, function: F) -> Node<NewTab>
+    /// If this [`Node`] is empty after filtering, `None` is returned.
+    pub fn filter_map_tabs<F, NewTab>(&self, function: F) -> Option<Node<NewTab>>
     where
         F: FnMut(&Tab) -> Option<NewTab>,
     {
@@ -285,43 +285,41 @@ impl<Tab> Node<Tab> {
                 active,
                 scroll,
             } => {
-                // TODO(LennysLounge): Fix this
-                todo!()
-                // let tabs: Vec<_> = tabs.iter().filter_map(function).collect();
-                // if tabs.is_empty() {
-                //     Node::Empty
-                // } else {
-                //     Node::Leaf {
-                //         rect: *rect,
-                //         viewport: *viewport,
-                //         tabs,
-                //         active: *active,
-                //         scroll: *scroll,
-                //     }
-                // }
+                let tabs: Vec<_> = tabs.iter().filter_map(function).collect();
+                if tabs.is_empty() {
+                    None
+                } else {
+                    Some(Node::Leaf {
+                        rect: *rect,
+                        viewport: *viewport,
+                        tabs,
+                        active: *active,
+                        scroll: *scroll,
+                    })
+                }
             }
             Node::Vertical {
                 rect,
                 fraction,
                 above,
                 below,
-            } => Node::Vertical {
+            } => Some(Node::Vertical {
                 rect: *rect,
                 fraction: *fraction,
                 above: *above,
                 below: *below,
-            },
+            }),
             Node::Horizontal {
                 rect,
                 fraction,
                 left,
                 right,
-            } => Node::Horizontal {
+            } => Some(Node::Horizontal {
                 rect: *rect,
                 fraction: *fraction,
                 left: *left,
                 right: *right,
-            },
+            }),
         }
     }
 
@@ -331,11 +329,12 @@ impl<Tab> Node<Tab> {
         F: FnMut(&Tab) -> NewTab,
     {
         self.filter_map_tabs(move |tab| Some(function(tab)))
+            .expect("Mapping tabs in a node should always produce a node")
     }
 
     /// Returns a new [`Node`] while filtering the tab type.
-    /// If this [`Node`] remains empty, it will change to [`Node::Empty`].
-    pub fn filter_tabs<F>(&self, mut predicate: F) -> Node<Tab>
+    /// If this [`Node`] is empty after filtering, `None` is returned.
+    pub fn filter_tabs<F>(&self, mut predicate: F) -> Option<Node<Tab>>
     where
         F: Clone + FnMut(&Tab) -> bool,
         Tab: Clone,
@@ -353,7 +352,7 @@ impl<Tab> Node<Tab> {
             tabs.retain_mut(predicate);
         }
 
-        // TODO(LennyLounge): Fix this
-        todo!();
+        // TODO(LennyLounge): If no tabs are retained, then the node should
+        // be removed. How should this api work then?
     }
 }