From 7dfa1b8dc9fb6d1fef974e7c29a32bff3146c71d Mon Sep 17 00:00:00 2001 From: Michael Gattozzi Date: Sun, 5 Jan 2020 23:19:42 -0500 Subject: Fix the ticket tui so it does not panic The tui for ticket uses indexing in order to have quicker unchecked access into data structures for the data needed that gets displayed in the tui. However, you can't index empty data. We add a check for this in places that expect data to exist so that nothing will cause a panic. We also add fixes to handle the fact that you can't divide by zero in our index modifying logic when pressing the left/right/up/down arrow keys. With this the tui shouldn't panic anymore and can work in an empty state. Closes: 4c37e800-2e38-11ea-b6e0-32f54a3ad7cd 'Having no tickets causes the TUI to crash' --- .../having-no-tickets-causes-the-tui-to-crash.toml | 17 ++++++ .../having-no-tickets-causes-the-tui-to-crash.toml | 17 ------ ticket/src/tui.rs | 62 +++++++++++++--------- 3 files changed, 55 insertions(+), 41 deletions(-) create mode 100644 .dev-suite/ticket/closed/having-no-tickets-causes-the-tui-to-crash.toml delete mode 100644 .dev-suite/ticket/open/having-no-tickets-causes-the-tui-to-crash.toml diff --git a/.dev-suite/ticket/closed/having-no-tickets-causes-the-tui-to-crash.toml b/.dev-suite/ticket/closed/having-no-tickets-causes-the-tui-to-crash.toml new file mode 100644 index 0000000..960ee5d --- /dev/null +++ b/.dev-suite/ticket/closed/having-no-tickets-causes-the-tui-to-crash.toml @@ -0,0 +1,17 @@ +title = 'Having no tickets causes the TUI to crash' +status = 'Closed' +id = '4c37e800-2e38-11ea-b6e0-32f54a3ad7cd' +assignees = [[ + '64c00ccc-761e-4484-86ac-904e461bb300', + 'Michael Gattozzi', +]] +description = ''' +Due to how the ticket tui uses indexing to avoid bounds checks this causes an +issue when you're, well, out of bounds. This was handled properly for when we +have n > 0 elements in the open or closed ticket arrays. An empty state for +when there are no tickets and a check for an empty collection need to be +created so that the indexing will be avoided and cause a crash. +''' +version = 'V1' + +[comments] diff --git a/.dev-suite/ticket/open/having-no-tickets-causes-the-tui-to-crash.toml b/.dev-suite/ticket/open/having-no-tickets-causes-the-tui-to-crash.toml deleted file mode 100644 index e4ffd42..0000000 --- a/.dev-suite/ticket/open/having-no-tickets-causes-the-tui-to-crash.toml +++ /dev/null @@ -1,17 +0,0 @@ -title = 'Having no tickets causes the TUI to crash' -status = 'Open' -id = '4c37e800-2e38-11ea-b6e0-32f54a3ad7cd' -assignees = [[ - '64c00ccc-761e-4484-86ac-904e461bb300', - 'Michael Gattozzi', -]] -description = ''' -Due to how the ticket tui uses indexing to avoid bounds checks this causes an -issue when you're, well, out of bounds. This was handled properly for when we -have n > 0 elements in the open or closed ticket arrays. An empty state for -when there are no tickets and a check for an empty collection need to be -created so that the indexing will be avoided and cause a crash. -''' -version = 'V1' - -[comments] diff --git a/ticket/src/tui.rs b/ticket/src/tui.rs index df62106..e20c112 100644 --- a/ticket/src/tui.rs +++ b/ticket/src/tui.rs @@ -86,11 +86,13 @@ impl<'a> TabsState<'a> { } pub fn next(&mut self) { - self.index = (self.index + 1) % self.titles.len() + if !self.titles.is_empty() { + self.index = (self.index + 1) % self.titles.len() + } } pub fn previous(&mut self) { - if self.index > 0 { + if self.index > 0 && !self.titles.is_empty() { self.index = (self.index - 1) % self.titles.len() } } @@ -123,11 +125,13 @@ impl TicketState { } pub fn next(&mut self) { - self.index = (self.index + 1) % self.len() + if self.len() > 0 { + self.index = (self.index + 1) % self.len() + } } pub fn previous(&mut self) { - if self.index > 0 { + if self.index > 0 && self.len() > 0 { self.index = (self.index - 1) % self.len() } } @@ -331,28 +335,35 @@ fn handle_event( KeyCode::Up => app.tickets.previous(), KeyCode::Down => app.tickets.next(), KeyCode::Backspace => { - let _ = app.tickets.tickets.get_mut(status).unwrap()[app.tickets.index] - .1 - .pop(); + if app.tickets.len() > 0 { + let _ = app.tickets.tickets.get_mut(status).unwrap() + [app.tickets.index] + .1 + .pop(); + } } KeyCode::Char(c) => { - app.tickets.tickets.get_mut(status).unwrap()[app.tickets.index] - .1 - .push(c); + if app.tickets.len() > 0 { + app.tickets.tickets.get_mut(status).unwrap()[app.tickets.index] + .1 + .push(c); + } } KeyCode::Enter => { - let ticket = - &mut app.tickets.tickets.get_mut(status).unwrap()[app.tickets.index]; - if !ticket.1.is_empty() { - let _ = ticket.0.comments.insert( - uuid_v1()?, - ( - user_config.uuid, - Name(user_config.name.clone()), - Comment(ticket.1.clone()), - ), - ); - ticket.1.clear(); + if app.tickets.len() > 0 { + let ticket = &mut app.tickets.tickets.get_mut(status).unwrap() + [app.tickets.index]; + if !ticket.1.is_empty() { + let _ = ticket.0.comments.insert( + uuid_v1()?, + ( + user_config.uuid, + Name(user_config.name.clone()), + Comment(ticket.1.clone()), + ), + ); + ticket.1.clear(); + } } } _ => {} @@ -450,9 +461,12 @@ impl<'a> App<'a> { #[inline] fn comment(&self, tab: &'a str, f: &mut Frame, rect: Rect) { - let (_, s) = &self.tickets.tickets.get(tab).unwrap()[self.tickets.index]; + let tickets = self.tickets.tickets.get(tab).unwrap(); let mut text = String::from("> "); - text.push_str(&s); + if !tickets.is_empty() { + let (_, s) = &tickets[self.tickets.index]; + text.push_str(&s); + } Paragraph::new([Text::raw(text)].iter()) .block(Block::default().borders(Borders::ALL).title("Comment")) -- cgit v1.2.3