Skip to content

Commit ce6a08f

Browse files
imsnifimsnif
authored
fix(floating-panes): handle uncaught errors (#3944)
* fix(floating-panes): handle uncaught errors * style(fmt): rustfmt
1 parent 9d3b4c5 commit ce6a08f

File tree

4 files changed

+119
-59
lines changed

4 files changed

+119
-59
lines changed

‎zellij-server/src/panes/floating_panes/floating_pane_grid.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -761,16 +761,12 @@ impl<'a> FloatingPaneGrid<'a> {
761761
a_pane.y().cmp(&b_pane.y())
762762
}
763763
});
764-
let active_pane_position = panes
765-
.iter()
766-
.position(|(id, _)| id == current_pane_id)
767-
.unwrap();
764+
let active_pane_position = panes.iter().position(|(id, _)| id == current_pane_id)?;
768765

769766
let next_active_pane_id = panes
770767
.get(active_pane_position + 1)
771768
.or_else(|| panes.get(0))
772-
.map(|p| p.0)
773-
.unwrap();
769+
.map(|p| p.0)?;
774770
Some(next_active_pane_id)
775771
}
776772
pub fn previous_selectable_pane_id(&self, current_pane_id: &PaneId) -> Option<PaneId> {

‎zellij-server/src/panes/floating_panes/mod.rs‎

Lines changed: 93 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,16 @@ impl FloatingPanes {
8888
}
8989
pub fn stack(&self) -> Option<FloatingPanesStack> {
9090
if self.panes_are_visible() {
91-
let layers = self
91+
let layers: Vec<PaneGeom> = self
9292
.z_indices
9393
.iter()
94-
.map(|pane_id| self.panes.get(pane_id).unwrap().position_and_size())
94+
.filter_map(|pane_id| self.panes.get(pane_id).map(|p| p.position_and_size()))
9595
.collect();
96-
Some(FloatingPanesStack { layers })
96+
if layers.is_empty() {
97+
None
98+
} else {
99+
Some(FloatingPanesStack { layers })
100+
}
97101
} else if self.has_pinned_panes() {
98102
let layers = self
99103
.z_indices
@@ -257,7 +261,8 @@ impl FloatingPanes {
257261
pub fn position_floating_pane_layout(
258262
&mut self,
259263
floating_pane_layout: &FloatingPaneLayout,
260-
) -> PaneGeom {
264+
) -> Result<PaneGeom> {
265+
let err_context = || format!("failed to find position for floating pane");
261266
let display_area = *self.display_area.borrow();
262267
let viewport = *self.viewport.borrow();
263268
let floating_pane_grid = FloatingPaneGrid::new(
@@ -266,7 +271,9 @@ impl FloatingPanes {
266271
display_area,
267272
viewport,
268273
);
269-
let mut position = floating_pane_grid.find_room_for_new_pane().unwrap(); // TODO: no unwrap
274+
let mut position = floating_pane_grid
275+
.find_room_for_new_pane()
276+
.with_context(err_context)?;
270277
if let Some(x) = &floating_pane_layout.x {
271278
position.x = x.to_position(viewport.cols);
272279
}
@@ -301,7 +308,7 @@ impl FloatingPanes {
301308
.y
302309
.saturating_sub((position.y + position.rows.as_usize()) - viewport.rows);
303310
}
304-
position
311+
Ok(position)
305312
}
306313
pub fn first_floating_pane_id(&self) -> Option<PaneId> {
307314
self.panes.keys().next().copied()
@@ -432,7 +439,7 @@ impl FloatingPanes {
432439
display_area,
433440
viewport,
434441
);
435-
floating_pane_grid.resize(new_screen_size).unwrap();
442+
floating_pane_grid.resize(new_screen_size).non_fatal();
436443
self.set_force_render();
437444
}
438445

@@ -530,17 +537,20 @@ impl FloatingPanes {
530537
Some(p) => {
531538
// render previously active pane so that its frame does not remain actively
532539
// colored
533-
let previously_active_pane = self
534-
.panes
535-
.get_mut(self.active_panes.get(&client_id).unwrap())
536-
.unwrap();
540+
let Some(previously_active_pane) = self.get_active_pane_mut(client_id) else {
541+
log::error!("Failed to get active pane");
542+
return Ok(false);
543+
};
537544

538545
previously_active_pane.set_should_render(true);
539546
// we render the full viewport to remove any ui elements that might have been
540547
// there before (eg. another user's cursor)
541548
previously_active_pane.render_full_viewport();
542549

543-
let next_active_pane = self.panes.get_mut(&p).unwrap();
550+
let Some(next_active_pane) = self.get_pane_mut(p) else {
551+
log::error!("Failed to get next active pane");
552+
return Ok(false);
553+
};
544554
next_active_pane.set_should_render(true);
545555
// we render the full viewport to remove any ui elements that might have been
546556
// there before (eg. another user's cursor)
@@ -588,9 +598,10 @@ impl FloatingPanes {
588598
display_area,
589599
viewport,
590600
);
591-
let pane_id = floating_pane_grid.pane_id_on_edge(direction).unwrap();
592-
self.focus_pane(pane_id, client_id);
593-
self.set_force_render();
601+
if let Some(pane_id) = floating_pane_grid.pane_id_on_edge(direction) {
602+
self.focus_pane(pane_id, client_id);
603+
self.set_force_render();
604+
}
594605
}
595606

596607
pub fn move_active_pane_down(&mut self, client_id: ClientId) {
@@ -641,7 +652,7 @@ impl FloatingPanes {
641652
display_area,
642653
viewport,
643654
);
644-
floating_pane_grid.move_pane_left(&pane_id).unwrap();
655+
floating_pane_grid.move_pane_left(&pane_id).non_fatal();
645656
self.set_force_render();
646657
}
647658
pub fn move_active_pane_right(&mut self, client_id: ClientId) {
@@ -658,7 +669,7 @@ impl FloatingPanes {
658669
display_area,
659670
viewport,
660671
);
661-
floating_pane_grid.move_pane_right(&pane_id).unwrap();
672+
floating_pane_grid.move_pane_right(&pane_id).non_fatal();
662673
self.set_force_render();
663674
}
664675
pub fn move_active_pane(
@@ -667,8 +678,9 @@ impl FloatingPanes {
667678
_os_api: &mut Box<dyn ServerOsApi>,
668679
client_id: ClientId,
669680
) {
670-
let active_pane_id = self.get_active_pane_id(client_id).unwrap();
671-
self.move_pane(search_backwards, active_pane_id)
681+
if let Some(active_pane_id) = self.get_active_pane_id(client_id) {
682+
self.move_pane(search_backwards, active_pane_id)
683+
}
672684
}
673685
pub fn move_pane(&mut self, search_backwards: bool, pane_id: PaneId) {
674686
let new_position_id = {
@@ -685,11 +697,17 @@ impl FloatingPanes {
685697
}
686698
};
687699
if let Some(new_position_id) = new_position_id {
688-
let current_position = self.panes.get(&pane_id).unwrap();
700+
let Some(current_position) = self.panes.get(&pane_id) else {
701+
log::error!("Failed to find current position");
702+
return;
703+
};
689704
let prev_geom = current_position.position_and_size();
690705
let prev_geom_override = current_position.geom_override();
691706

692-
let new_position = self.panes.get_mut(&new_position_id).unwrap();
707+
let Some(new_position) = self.panes.get_mut(&new_position_id) else {
708+
log::error!("Failed to find new position");
709+
return;
710+
};
693711
let next_geom = new_position.position_and_size();
694712
let next_geom_override = new_position.geom_override();
695713
new_position.set_geom(prev_geom);
@@ -698,7 +716,10 @@ impl FloatingPanes {
698716
}
699717
new_position.set_should_render(true);
700718

701-
let current_position = self.panes.get_mut(&pane_id).unwrap();
719+
let Some(current_position) = self.panes.get_mut(&pane_id) else {
720+
log::error!("Failed to find current position");
721+
return;
722+
};
702723
current_position.set_geom(next_geom);
703724
if let Some(geom) = next_geom_override {
704725
current_position.set_geom_override(geom);
@@ -801,8 +822,16 @@ impl FloatingPanes {
801822
panes.sort_by(|(a_id, _a_pane), (b_id, _b_pane)| {
802823
// TODO: continue
803824
Ord::cmp(
804-
&self.z_indices.iter().position(|id| id == *b_id).unwrap(),
805-
&self.z_indices.iter().position(|id| id == *a_id).unwrap(),
825+
&self
826+
.z_indices
827+
.iter()
828+
.position(|id| id == *b_id)
829+
.unwrap_or(0),
830+
&self
831+
.z_indices
832+
.iter()
833+
.position(|id| id == *a_id)
834+
.unwrap_or(0),
806835
)
807836
});
808837
Ok(panes
@@ -832,8 +861,16 @@ impl FloatingPanes {
832861
panes.sort_by(|(a_id, _a_pane), (b_id, _b_pane)| {
833862
// TODO: continue
834863
Ord::cmp(
835-
&self.z_indices.iter().position(|id| id == *b_id).unwrap(),
836-
&self.z_indices.iter().position(|id| id == *a_id).unwrap(),
864+
&self
865+
.z_indices
866+
.iter()
867+
.position(|id| id == *b_id)
868+
.unwrap_or(0),
869+
&self
870+
.z_indices
871+
.iter()
872+
.position(|id| id == *a_id)
873+
.unwrap_or(0),
837874
)
838875
});
839876
Ok(panes
@@ -850,8 +887,16 @@ impl FloatingPanes {
850887

851888
panes.sort_by(|(a_id, _a_pane), (b_id, _b_pane)| {
852889
Ord::cmp(
853-
&self.z_indices.iter().position(|id| id == *b_id).unwrap(),
854-
&self.z_indices.iter().position(|id| id == *a_id).unwrap(),
890+
&self
891+
.z_indices
892+
.iter()
893+
.position(|id| id == *b_id)
894+
.unwrap_or(0),
895+
&self
896+
.z_indices
897+
.iter()
898+
.position(|id| id == *a_id)
899+
.unwrap_or(0),
855900
)
856901
});
857902
panes.iter().find(|(_, p)| p.contains(point)).is_some()
@@ -862,7 +907,7 @@ impl FloatingPanes {
862907
search_selectable: bool,
863908
) -> Option<&mut Box<dyn Pane>> {
864909
self.get_pane_id_at(position, search_selectable)
865-
.unwrap()
910+
.ok()?
866911
.and_then(|pane_id| self.panes.get_mut(&pane_id))
867912
}
868913
pub fn set_pane_being_moved_with_mouse(&mut self, pane_id: PaneId, position: Position) {
@@ -875,7 +920,10 @@ impl FloatingPanes {
875920
// true => changed position
876921
let display_area = *self.display_area.borrow();
877922
let viewport = *self.viewport.borrow();
878-
let (pane_id, previous_position) = self.pane_being_moved_with_mouse.unwrap();
923+
let Some((pane_id, previous_position)) = self.pane_being_moved_with_mouse else {
924+
log::error!("Pane is not being moved with mousd");
925+
return false;
926+
};
879927
if click_position == &previous_position {
880928
return false;
881929
}
@@ -889,7 +937,7 @@ impl FloatingPanes {
889937
);
890938
floating_pane_grid
891939
.move_pane_by(pane_id, move_x_by, move_y_by)
892-
.unwrap();
940+
.non_fatal();
893941
self.set_pane_being_moved_with_mouse(pane_id, *click_position);
894942
self.set_force_render();
895943
true
@@ -962,21 +1010,30 @@ impl FloatingPanes {
9621010
}
9631011
pub fn switch_active_pane_with(&mut self, _os_api: &mut Box<dyn ServerOsApi>, pane_id: PaneId) {
9641012
if let Some(active_pane_id) = self.first_active_floating_pane_id() {
965-
let current_position = self.panes.get(&active_pane_id).unwrap();
1013+
let Some(current_position) = self.panes.get(&active_pane_id) else {
1014+
log::error!("Can't find current position");
1015+
return;
1016+
};
9661017
let prev_geom = current_position.position_and_size();
9671018
let prev_geom_override = current_position.geom_override();
9681019

969-
let new_position = self.panes.get_mut(&pane_id).unwrap();
1020+
let Some(new_position) = self.panes.get_mut(&pane_id) else {
1021+
log::error!("Can't find position");
1022+
return;
1023+
};
9701024
let next_geom = new_position.position_and_size();
9711025
let next_geom_override = new_position.geom_override();
9721026
new_position.set_geom(prev_geom);
9731027
if let Some(geom) = prev_geom_override {
9741028
new_position.set_geom_override(geom);
9751029
}
976-
resize_pty!(new_position, os_api, self.senders, self.character_cell_size).unwrap();
1030+
resize_pty!(new_position, os_api, self.senders, self.character_cell_size).non_fatal();
9771031
new_position.set_should_render(true);
9781032

979-
let current_position = self.panes.get_mut(&active_pane_id).unwrap();
1033+
let Some(current_position) = self.panes.get_mut(&active_pane_id) else {
1034+
log::error!("Can't find current position");
1035+
return;
1036+
};
9801037
current_position.set_geom(next_geom);
9811038
if let Some(geom) = next_geom_override {
9821039
current_position.set_geom_override(geom);
@@ -987,7 +1044,7 @@ impl FloatingPanes {
9871044
self.senders,
9881045
self.character_cell_size
9891046
)
990-
.unwrap();
1047+
.non_fatal();
9911048
current_position.set_should_render(true);
9921049
self.focus_pane_for_all_clients(active_pane_id);
9931050
}

‎zellij-server/src/tab/layout_applier.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ impl<'a> LayoutApplier<'a> {
588588
for floating_pane_layout in floating_panes_layout {
589589
let position_and_size = self
590590
.floating_panes
591-
.position_floating_pane_layout(&floating_pane_layout);
591+
.position_floating_pane_layout(&floating_pane_layout)?;
592592
let pid_to_focus = if floating_pane_layout.already_running {
593593
self.floating_panes.set_geom_for_pane_with_run(
594594
floating_pane_layout.run.clone(),
@@ -682,7 +682,8 @@ impl<'a> LayoutApplier<'a> {
682682
// contain partial positioning information (eg. just x coords with no y or size) or no
683683
// positioning information at all
684684
for (pane, floating_pane_layout) in panes_to_apply.drain(..) {
685-
pane_applier.apply_floating_panes_layout_to_floating_pane(pane, floating_pane_layout);
685+
pane_applier
686+
.apply_floating_panes_layout_to_floating_pane(pane, floating_pane_layout)?;
686687
}
687688

688689
// here we apply positioning on a best-effort basis to any remaining panes we've got (these
@@ -969,17 +970,18 @@ impl<'a> PaneApplier<'a> {
969970
&mut self,
970971
mut pane: Box<dyn Pane>,
971972
floating_panes_layout: FloatingPaneLayout,
972-
) {
973+
) -> Result<()> {
973974
let position_and_size = self
974975
.floating_panes
975-
.position_floating_pane_layout(&floating_panes_layout);
976+
.position_floating_pane_layout(&floating_panes_layout)?;
976977
if let Some(pane_title) = floating_panes_layout.name.as_ref() {
977978
pane.set_title(pane_title.into());
978979
}
979980
if floating_panes_layout.focus.unwrap_or(false) {
980981
self.new_focused_pane_id = Some(pane.pid());
981982
}
982-
self.apply_position_and_size_to_floating_pane(pane, position_and_size)
983+
self.apply_position_and_size_to_floating_pane(pane, position_and_size);
984+
Ok(())
983985
}
984986
pub fn apply_position_and_size_to_floating_pane(
985987
&mut self,

0 commit comments

Comments
 (0)