refactor(explorer,tree): remove unwrap to avoid panics

changes

@@ -47,14 +47,12 @@ New:
 - [x] bind "C-n/C-p" to up/down
 - [x] bind "="/"_" to zoom-in/zoom-out
 - [x] Sticky ancestors
+- [x] remove unwrap and expect
 - [] Toggle preview
 - [] search highlight matching word
 - [] Error didn't clear
 - [] should preview be there by default?
 - [] Fix panic bugs (see github comments)
-- [] explorer(preview): use popup instead of custom components
 - [] explorer(preview): overflow where bufferline is there
-- [] explorer(preview): implement scrolling C-j/C-k
 - [] symlink not showing
-- [] remove unwrap and expect
 - [] bug(tree): zb does not work, because clash with explorer 'b'

helix-term/src/ui/explorer.rs

@@ -279,7 +279,8 @@ impl Explorer {
     }
 
     fn render_preview(&mut self, area: Rect, surface: &mut Surface, editor: &Editor) {
-        let item = self.tree.current().item();
+        if let Ok(current) = self.tree.current() {
+            let item = current.item();
             let head_area = render_block(
                 area.clip_bottom(area.height.saturating_sub(2)),
                 surface,
@@ -308,6 +309,7 @@ impl Explorer {
                 );
             })
         }
+    }
 
     fn new_create_folder_prompt(&mut self) -> Result<()> {
         let folder_path = self.nearest_folder()?;
@@ -338,14 +340,14 @@ impl Explorer {
     }
 
     fn nearest_folder(&self) -> Result<PathBuf> {
-        let current = self.tree.current();
-        if current.item().is_parent() {
-            Ok(current.item().path.to_path_buf())
+        let current = self.tree.current()?.item();
+        if current.is_parent() {
+            Ok(current.path.to_path_buf())
         } else {
-            let parent_path = current.item().path.parent().ok_or_else(|| {
+            let parent_path = current.path.parent().ok_or_else(|| {
                 anyhow::anyhow!(format!(
                     "Unable to get parent path of '{}'",
-                    current.item().path.to_string_lossy()
+                    current.path.to_string_lossy()
                 ))
             })?;
             Ok(parent_path.to_path_buf())
@@ -353,7 +355,7 @@ impl Explorer {
     }
 
     fn new_remove_prompt(&mut self) -> Result<()> {
-        let item = self.tree.current().item();
+        let item = self.tree.current()?.item();
         match item.file_type {
             FileType::Folder => self.new_remove_folder_prompt(),
             FileType::File => self.new_remove_file_prompt(),
@@ -361,8 +363,8 @@ impl Explorer {
         }
     }
 
-    fn new_rename_prompt(&mut self, cx: &mut Context) {
-        let path = self.tree.current_item().path.clone();
+    fn new_rename_prompt(&mut self, cx: &mut Context) -> Result<()> {
+        let path = self.tree.current_item()?.path.clone();
         self.prompt = Some((
             PromptAction::RenameFile,
             Prompt::new(
@@ -373,10 +375,11 @@ impl Explorer {
             )
             .with_line(path.to_string_lossy().to_string(), cx.editor),
         ));
+        Ok(())
     }
 
     fn new_remove_file_prompt(&mut self) -> Result<()> {
-        let item = self.tree.current_item();
+        let item = self.tree.current_item()?;
         ensure!(
             item.path.is_file(),
             "The path '{}' is not a file",
@@ -395,7 +398,7 @@ impl Explorer {
     }
 
     fn new_remove_folder_prompt(&mut self) -> Result<()> {
-        let item = self.tree.current_item();
+        let item = self.tree.current_item()?;
         ensure!(
             item.path.is_dir(),
             "The path '{}' is not a folder",
@@ -616,7 +619,7 @@ impl Explorer {
             };
             let line = prompt.line();
 
-            let current_item_path = explorer.tree.current_item().path.clone();
+            let current_item_path = explorer.tree.current_item()?.path.clone();
             match (&action, event) {
                 (PromptAction::CreateFolder, key!(Enter)) => explorer.new_folder(line)?,
                 (PromptAction::CreateFile, key!(Enter)) => explorer.new_file(line)?,
@@ -682,7 +685,7 @@ impl Explorer {
     }
 
     fn change_root_to_current_folder(&mut self) -> Result<()> {
-        self.change_root(self.tree.current_item().path.clone())
+        self.change_root(self.tree.current_item()?.path.clone())
     }
 
     fn change_root_parent_folder(&mut self) -> Result<()> {
@@ -715,7 +718,7 @@ impl Explorer {
     }
 
     fn rename_current(&mut self, line: &String) -> Result<()> {
-        let item = self.tree.current_item();
+        let item = self.tree.current_item()?;
         let path = PathBuf::from(line);
         if let Some(parent) = path.parent() {
             std::fs::create_dir_all(parent)?;
@@ -726,13 +729,13 @@ impl Explorer {
     }
 
     fn remove_folder(&mut self) -> Result<()> {
-        let item = self.tree.current_item();
+        let item = self.tree.current_item()?;
         std::fs::remove_dir_all(&item.path)?;
         self.tree.refresh()
     }
 
     fn remove_file(&mut self) -> Result<()> {
-        let item = self.tree.current_item();
+        let item = self.tree.current_item()?;
         std::fs::remove_file(&item.path)?;
         self.tree.refresh()
     }
@@ -796,7 +799,7 @@ impl Component for Explorer {
                 key!(']') => self.change_root_to_current_folder()?,
                 key!('[') => self.go_to_previous_root(),
                 key!('d') => self.new_remove_prompt()?,
-                key!('r') => self.new_rename_prompt(cx),
+                key!('r') => self.new_rename_prompt(cx)?,
                 key!('-') | key!('_') => self.decrease_size(),
                 key!('+') | key!('=') => self.increase_size(),
                 _ => {

helix-term/src/ui/tree.rs

@@ -439,15 +439,16 @@ impl<T: TreeViewItem> TreeView<T> {
         self.tree.regenerate_index();
     }
 
-    fn move_to_parent(&mut self) {
-        if let Some(parent) = self.current_parent() {
+    fn move_to_parent(&mut self) -> Result<()> {
+        if let Some(parent) = self.current_parent()? {
             let index = parent.index;
             self.set_selected(index)
         }
+        Ok(())
     }
 
     fn move_to_children(&mut self, filter: &String) -> Result<()> {
-        let current = self.current_mut();
+        let current = self.current_mut()?;
         if current.is_opened {
             self.set_selected(self.selected + 1);
             Ok(())
@@ -526,17 +527,17 @@ impl<T: TreeViewItem> TreeView<T> {
         params: &mut T::Params,
         selected_index: usize,
         filter: &String,
-    ) {
-        let selected_item = self.get_mut(selected_index);
+    ) -> Result<()> {
+        let selected_item = self.get_mut(selected_index)?;
         if selected_item.is_opened {
             selected_item.close();
             self.regenerate_index();
-            return;
+            return Ok(());
         }
 
         if let Some(mut on_open_fn) = self.on_opened_fn.take() {
-            let mut f = || {
-                let current = self.current_mut();
+            let mut f = || -> Result<()> {
+                let current = self.current_mut()?;
                 match on_open_fn(&mut current.item, cx, params) {
                     TreeOp::GetChildsAndInsert => {
                         if let Err(err) = current.open(filter) {
@@ -545,11 +546,13 @@ impl<T: TreeViewItem> TreeView<T> {
                     }
                     TreeOp::Noop => {}
                 };
+                Ok(())
             };
-            f();
+            f()?;
             self.regenerate_index();
-            self.on_opened_fn = Some(on_open_fn)
-        }
+            self.on_opened_fn = Some(on_open_fn);
+        };
+        Ok(())
     }
 
     fn set_search_str(&mut self, s: String) {
@@ -675,36 +678,36 @@ impl<T: TreeViewItem> TreeView<T> {
         })
     }
 
-    fn get(&self, index: usize) -> &Tree<T> {
-        self.tree
-            .get(index)
-            .expect(format!("Tree: index {index} is out of bound").as_str())
+    fn get(&self, index: usize) -> Result<&Tree<T>> {
+        self.tree.get(index).ok_or_else(|| {
+            anyhow::anyhow!("Programming error: TreeView.get: index {index} is out of bound")
+        })
     }
 
-    fn get_mut(&mut self, index: usize) -> &mut Tree<T> {
-        self.tree
-            .get_mut(index)
-            .expect(format!("Tree: index {index} is out of bound").as_str())
+    fn get_mut(&mut self, index: usize) -> Result<&mut Tree<T>> {
+        self.tree.get_mut(index).ok_or_else(|| {
+            anyhow::anyhow!("Programming error: TreeView.get_mut: index {index} is out of bound")
+        })
     }
 
-    pub fn current(&self) -> &Tree<T> {
+    pub fn current(&self) -> Result<&Tree<T>> {
         self.get(self.selected)
     }
 
-    pub fn current_mut(&mut self) -> &mut Tree<T> {
+    pub fn current_mut(&mut self) -> Result<&mut Tree<T>> {
         self.get_mut(self.selected)
     }
 
-    fn current_parent(&self) -> Option<&Tree<T>> {
-        if let Some(parent_index) = self.current().parent_index {
-            Some(self.get(parent_index))
+    fn current_parent(&self) -> Result<Option<&Tree<T>>> {
+        if let Some(parent_index) = self.current()?.parent_index {
+            Ok(Some(self.get(parent_index)?))
         } else {
-            None
+            Ok(None)
         }
     }
 
-    pub fn current_item(&self) -> &T {
-        &self.current().item
+    pub fn current_item(&self) -> Result<&T> {
+        Ok(&self.current()?.item)
     }
 
     pub fn winline(&self) -> usize {
@@ -1013,8 +1016,11 @@ impl<T: TreeViewItem + Clone> TreeView<T> {
         }
 
         let count = std::mem::replace(&mut self.count, 0);
+        (|| -> Result<EventResult> {
             match key_event {
-            key!(i @ '0'..='9') => self.count = i.to_digit(10).unwrap() as usize + count * 10,
+                key!(i @ '0'..='9') => {
+                    self.count = i.to_digit(10).unwrap_or(0) as usize + count * 10
+                }
                 key!('k') | key!(Up) | ctrl!('p') => self.move_up(1.max(count)),
                 key!('j') | key!(Down) | ctrl!('n') => self.move_down(1.max(count)),
                 key!('z') => {
@@ -1025,14 +1031,11 @@ impl<T: TreeViewItem + Clone> TreeView<T> {
                         _ => {}
                     }));
                 }
-            key!('h') | key!(Left) => self.move_to_parent(),
-            key!('l') | key!(Right) => match self.move_to_children(filter) {
-                Ok(_) => {}
-                Err(err) => cx.editor.set_error(err.to_string()),
-            },
+                key!('h') | key!(Left) => self.move_to_parent()?,
+                key!('l') | key!(Right) => self.move_to_children(filter)?,
                 shift!('H') => self.move_left(1),
                 shift!('L') => self.move_right(1),
-            key!(Enter) | key!('o') => self.on_enter(cx, params, self.selected, filter),
+                key!(Enter) | key!('o') => self.on_enter(cx, params, self.selected, filter)?,
                 ctrl!('d') => self.move_down_half_page(),
                 ctrl!('u') => self.move_up_half_page(),
                 key!('g') => {
@@ -1059,10 +1062,14 @@ impl<T: TreeViewItem + Clone> TreeView<T> {
                 key!(Home) => self.move_leftmost(),
                 key!(End) => self.move_rightmost(),
                 ctrl!('o') => self.jump_backward(),
-            _ => return EventResult::Ignored(None),
-        }
-
+                _ => return Ok(EventResult::Ignored(None)),
+            };
+            Ok(EventResult::Consumed(None))
+        })()
+        .unwrap_or_else(|err| {
+            cx.editor.set_error(format!("{err}"));
             EventResult::Consumed(None)
+        })
     }
 
     fn handle_filter_event(&mut self, event: &KeyEvent, cx: &mut Context) -> EventResult {
@@ -1577,7 +1584,7 @@ mod test_tree_view {
             .trim()
         );
 
-        view.move_to_parent();
+        view.move_to_parent().unwrap();
         assert_eq!(
             render(&mut view),
             "
@@ -1591,7 +1598,7 @@ mod test_tree_view {
         );
 
         view.move_to_last_line();
-        view.move_to_parent();
+        view.move_to_parent().unwrap();
         assert_eq!(
             render(&mut view),
             "
@@ -1751,7 +1758,7 @@ krabby_patty
             .trim()
         );
 
-        view.move_to_parent();
+        view.move_to_parent().unwrap();
         assert_eq!(
             render(&mut view),
             "
@@ -1764,7 +1771,7 @@ krabby_patty
             .trim()
         );
 
-        view.move_to_parent();
+        view.move_to_parent().unwrap();
         assert_eq!(
             render(&mut view),
             "
@@ -1777,7 +1784,7 @@ krabby_patty
             .trim()
         );
 
-        view.move_to_parent();
+        view.move_to_parent().unwrap();
         assert_eq!(
             render(&mut view),
             "
@@ -1988,7 +1995,7 @@ krabby_patty
         view.refresh_with_filter(&"ar".to_string()).unwrap();
 
         // 3. Get the current item
-        let item = view.current_item();
+        let item = view.current_item().unwrap();
 
         // 3a. Expects no failure
         assert_eq!(item.name, "who_lives_in_a_pine")
@@ -2214,7 +2221,7 @@ krabby_patty
 
         // 5.3 Move up
         view.move_up(1);
-        assert_eq!(view.current_item().name, "baaa");
+        assert_eq!(view.current_item().unwrap().name, "baaa");
         assert_eq!(
             render(&mut view),
             "