[BUG] Panic when opening empty menubar subtree #776

Activity

1. bycruz

   added

   bug

   on Mar 27, 2024
2. gyscos commented on Mar 27, 2024

   

   gyscos

   on Mar 27, 2024

   Owner

   More actions

   Thanks for the report!

   Indeed, not panicking sounds like a better behavior :)

   👍React with 👍1Reacted by Oncelock
3. correabuscar commented on Jun 11, 2024

   

   correabuscar

   on Jun 11, 2024

   Contributor

   More actions

   Tentative fix would be:

   diff --git a/cursive-core/src/views/menu_popup.rs b/cursive-core/src/views/menu_popup.rs
   index d3050ba..9fe5698 100644
   --- a/cursive-core/src/views/menu_popup.rs
   +++ b/cursive-core/src/views/menu_popup.rs
   @@ -130,7 +130,11 @@ impl MenuPopup {
                } else if cycle {
                    // Only cycle once to prevent endless loop
                    cycle = false;
   -                self.focus = self.menu.children.len() - 1;
   +                let len = self.menu.children.len();
   +                if len == 0 {
   +                    break;
   +                }
   +                self.focus = len - 1;
                } else {
                    break;
                }
   @@ -143,12 +147,16 @@ impl MenuPopup {
    
        fn scroll_down(&mut self, mut n: usize, mut cycle: bool) {
            while n > 0 {
   -            if self.focus + 1 < self.menu.children.len() {
   +            let len = self.menu.children.len();
   +            if self.focus + 1 < len {
                    self.focus += 1;
                } else if cycle {
                    // Only cycle once to prevent endless loop
                    cycle = false;
                    self.focus = 0;
   +                if len == 0 {
   +                    break;
   +                }
                } else {
                    // Stop if we're at the bottom.
                    break;
   @@ -236,13 +244,19 @@ impl MenuPopup {
                Event::Key(Key::Home) => self.focus = 0,
                Event::Key(Key::End) => self.focus = self.menu.children.len().saturating_sub(1),
    
   -            Event::Key(Key::Right) if self.menu.children[self.focus].is_subtree() => {
   +            Event::Key(Key::Right)
   +                if (self.focus < self.menu.children.len()
   +                    && self.menu.children[self.focus].is_subtree()) =>
   +            {
                    return match self.menu.children[self.focus] {
                        menu::Item::Subtree { ref tree, .. } => self.make_subtree_cb(tree),
                        _ => unreachable!("Child is a subtree"),
                    };
                }
   -            Event::Key(Key::Enter) if self.menu.children[self.focus].is_enabled() => {
   +            Event::Key(Key::Enter)
   +                if (self.focus < self.menu.children.len()
   +                    && self.menu.children[self.focus].is_enabled()) =>
   +            {
                    return self.submit();
                }
                Event::Mouse {
   @@ -263,7 +277,8 @@ impl MenuPopup {
                    event: MouseEvent::Release(MouseButton::Left),
                    position,
                    offset,
   -            } if self.menu.children[self.focus].is_enabled()
   +            } if (self.focus < self.menu.children.len()
   +                && self.menu.children[self.focus].is_enabled())
                    && position
                        .checked_sub(offset)
                        .map(|position| position.y == self.focus)
   @@ -320,7 +335,13 @@ impl View for MenuPopup {
            scroll::draw_box_frame(
                self,
                printer,
   -            |s, y| s.menu.children[y].is_delimiter(),
   +            |s, y| {
   +                if s.menu.children.len() > y {
   +                    s.menu.children[y].is_delimiter()
   +                } else {
   +                    false
   +                }
   +            },
                |_s, _x| false,
            );
    

   But maybe this isn't the proper way to do it. Like, some indirection(via some new function) might be wanted instead.
4. correabuscar

   added a commit that references this issue on Jun 11, 2024

   tentatively fixes gyscos#776

   This issue will close once commit 161835a is merged into the 'main' branch.

   161835a
5. correabuscar

   added a commit that references this issue on Jun 20, 2024

   tentatively fixes gyscos#776

   This issue will close once commit b187d2f is merged into the 'main' branch.

   b187d2f