Skip to content

Commit

Permalink
improve error handling in ui module (#1870)
Browse files Browse the repository at this point in the history
* improve error handling in ui module

* resolve problems in the review
  • Loading branch information
naosense authored Oct 28, 2022
1 parent 668df6b commit c5b1eb0
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 77 deletions.
36 changes: 25 additions & 11 deletions zellij-server/src/panes/floating_panes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use std::rc::Rc;
use std::time::Instant;
use zellij_utils::{
data::{ModeInfo, Style},
errors::prelude::*,
input::command::RunCommand,
pane_size::{Offset, PaneGeom, Size, Viewport},
};
Expand Down Expand Up @@ -234,16 +235,25 @@ impl FloatingPanes {
resize_pty!(pane, os_api);
}
}
pub fn render(&mut self, output: &mut Output) {
pub fn render(&mut self, output: &mut Output) -> Result<()> {
let err_context = || "failed to render output";
let connected_clients: Vec<ClientId> =
{ self.connected_clients.borrow().iter().copied().collect() };
let mut floating_panes: Vec<_> = self.panes.iter_mut().collect();
floating_panes.sort_by(|(a_id, _a_pane), (b_id, _b_pane)| {
self.z_indices
.iter()
.position(|id| id == *a_id)
.unwrap()
.cmp(&self.z_indices.iter().position(|id| id == *b_id).unwrap())
.with_context(err_context)
.fatal()
.cmp(
&self
.z_indices
.iter()
.position(|id| id == *b_id)
.with_context(err_context)
.fatal(),
)
});

for (z_index, (kind, pane)) in floating_panes.iter_mut().enumerate() {
Expand All @@ -266,23 +276,27 @@ impl FloatingPanes {
.get(client_id)
.unwrap_or(&self.default_mode_info)
.mode;
pane_contents_and_ui.render_pane_frame(
*client_id,
client_mode,
self.session_is_mirrored,
);
pane_contents_and_ui
.render_pane_frame(*client_id, client_mode, self.session_is_mirrored)
.with_context(err_context)?;
if let PaneId::Plugin(..) = kind {
pane_contents_and_ui.render_pane_contents_for_client(*client_id);
pane_contents_and_ui
.render_pane_contents_for_client(*client_id)
.with_context(err_context)?;
}
// this is done for panes that don't have their own cursor (eg. panes of
// another user)
pane_contents_and_ui.render_fake_cursor_if_needed(*client_id);
pane_contents_and_ui
.render_fake_cursor_if_needed(*client_id)
.with_context(err_context)?;
}
if let PaneId::Terminal(..) = kind {
pane_contents_and_ui
.render_pane_contents_to_multiple_clients(connected_clients.iter().copied());
.render_pane_contents_to_multiple_clients(connected_clients.iter().copied())
.with_context(err_context)?;
}
}
Ok(())
}
pub fn resize(&mut self, new_screen_size: Size) {
let display_area = *self.display_area.borrow();
Expand Down
13 changes: 9 additions & 4 deletions zellij-server/src/panes/plugin_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,10 @@ impl Pane for PluginPane {
_client_id: ClientId,
frame_params: FrameParams,
input_mode: InputMode,
) -> Option<(Vec<CharacterChunk>, Option<String>)> {
) -> Result<Option<(Vec<CharacterChunk>, Option<String>)>> {
// FIXME: This is a hack that assumes all fixed-size panes are borderless. This
// will eventually need fixing!
if self.frame && !(self.geom.rows.is_fixed() || self.geom.cols.is_fixed()) {
let res = if self.frame && !(self.geom.rows.is_fixed() || self.geom.cols.is_fixed()) {
let pane_title = if self.pane_name.is_empty()
&& input_mode == InputMode::RenamePane
&& frame_params.is_main_client
Expand All @@ -251,10 +251,15 @@ impl Pane for PluginPane {
pane_title,
frame_params,
);
Some(frame.render())
Some(
frame
.render()
.with_context(|| format!("failed to render frame for client {_client_id}"))?,
)
} else {
None
}
};
Ok(res)
}
fn render_fake_cursor(
&mut self,
Expand Down
12 changes: 7 additions & 5 deletions zellij-server/src/panes/terminal_pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ impl Pane for TerminalPane {
client_id: ClientId,
frame_params: FrameParams,
input_mode: InputMode,
) -> Option<(Vec<CharacterChunk>, Option<String>)> {
) -> Result<Option<(Vec<CharacterChunk>, Option<String>)>> {
let err_context = || format!("failed to render frame for client {client_id}");
// TODO: remove the cursor stuff from here
let pane_title = if self.pane_name.is_empty()
&& input_mode == InputMode::RenamePane
Expand Down Expand Up @@ -398,12 +399,12 @@ impl Pane for TerminalPane {
frame.add_exit_status(exit_status.as_ref().copied());
}

match self.frame.get(&client_id) {
let res = match self.frame.get(&client_id) {
// TODO: use and_then or something?
Some(last_frame) => {
if &frame != last_frame {
if !self.borderless {
let frame_output = frame.render();
let frame_output = frame.render().with_context(err_context)?;
self.frame.insert(client_id, frame);
Some(frame_output)
} else {
Expand All @@ -415,14 +416,15 @@ impl Pane for TerminalPane {
},
None => {
if !self.borderless {
let frame_output = frame.render();
let frame_output = frame.render().with_context(err_context)?;
self.frame.insert(client_id, frame);
Some(frame_output)
} else {
None
}
},
}
};
Ok(res)
}
fn render_fake_cursor(
&mut self,
Expand Down
35 changes: 23 additions & 12 deletions zellij-server/src/panes/tiled_panes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::cell::RefCell;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::rc::Rc;
use std::time::Instant;
use zellij_utils::errors::prelude::*;
use zellij_utils::{
data::{ModeInfo, Style},
input::command::RunCommand,
Expand Down Expand Up @@ -378,7 +379,8 @@ impl TiledPanes {
pub fn has_panes(&self) -> bool {
!self.panes.is_empty()
}
pub fn render(&mut self, output: &mut Output, floating_panes_are_visible: bool) {
pub fn render(&mut self, output: &mut Output, floating_panes_are_visible: bool) -> Result<()> {
let err_context = || "failed to render tiled panes";
let connected_clients: Vec<ClientId> =
{ self.connected_clients.borrow().iter().copied().collect() };
let multiple_users_exist_in_session = { self.connected_clients_in_app.borrow().len() > 1 };
Expand Down Expand Up @@ -409,15 +411,17 @@ impl TiledPanes {
.get(client_id)
.unwrap_or(&self.default_mode_info)
.mode;
let err_context =
|| format!("failed to render tiled panes for client {client_id}");
if let PaneId::Plugin(..) = kind {
pane_contents_and_ui.render_pane_contents_for_client(*client_id);
pane_contents_and_ui
.render_pane_contents_for_client(*client_id)
.with_context(err_context)?;
}
if self.draw_pane_frames {
pane_contents_and_ui.render_pane_frame(
*client_id,
client_mode,
self.session_is_mirrored,
);
pane_contents_and_ui
.render_pane_frame(*client_id, client_mode, self.session_is_mirrored)
.with_context(err_context)?;
} else {
let boundaries = client_id_to_boundaries
.entry(*client_id)
Expand All @@ -432,20 +436,27 @@ impl TiledPanes {
pane_contents_and_ui.render_terminal_title_if_needed(*client_id, client_mode);
// this is done for panes that don't have their own cursor (eg. panes of
// another user)
pane_contents_and_ui.render_fake_cursor_if_needed(*client_id);
pane_contents_and_ui
.render_fake_cursor_if_needed(*client_id)
.with_context(err_context)?;
}
if let PaneId::Terminal(..) = kind {
pane_contents_and_ui.render_pane_contents_to_multiple_clients(
connected_clients.iter().copied(),
);
pane_contents_and_ui
.render_pane_contents_to_multiple_clients(connected_clients.iter().copied())
.with_context(err_context)?;
}
}
}
// render boundaries if needed
for (client_id, boundaries) in &mut client_id_to_boundaries {
// TODO: add some conditional rendering here so this isn't rendered for every character
output.add_character_chunks_to_client(*client_id, boundaries.render(), None);
output.add_character_chunks_to_client(
*client_id,
boundaries.render().with_context(err_context)?,
None,
);
}
Ok(())
}
pub fn get_panes(&self) -> impl Iterator<Item = (&PaneId, &Box<dyn Pane>)> {
self.panes.iter()
Expand Down
2 changes: 1 addition & 1 deletion zellij-server/src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ impl Pty {
.bus
.os_input
.as_mut()
.unwrap()
.ok_or_else(|| SpawnTerminalError::GenericSpawnError("os input is none"))?
.spawn_terminal(terminal_action, quit_cb, self.default_editor.clone())?;
let terminal_bytes = task::spawn({
let err_context = || format!("failed to run async task for terminal {terminal_id}");
Expand Down
2 changes: 1 addition & 1 deletion zellij-server/src/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ impl Screen {
let overlay = self.overlay.clone();
for (tab_index, tab) in &mut self.tabs {
if tab.has_selectable_tiled_panes() {
let vte_overlay = overlay.generate_overlay(size);
let vte_overlay = overlay.generate_overlay(size).context(err_context)?;
tab.render(&mut output, Some(vte_overlay))
.context(err_context)?;
} else {
Expand Down
9 changes: 6 additions & 3 deletions zellij-server/src/tab/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub trait Pane {
client_id: ClientId,
frame_params: FrameParams,
input_mode: InputMode,
) -> Option<(Vec<CharacterChunk>, Option<String>)>; // TODO: better
) -> Result<Option<(Vec<CharacterChunk>, Option<String>)>>; // TODO: better
fn render_fake_cursor(
&mut self,
cursor_color: PaletteColor,
Expand Down Expand Up @@ -1329,9 +1329,12 @@ impl Tab {

self.hide_cursor_and_clear_display_as_needed(output);
self.tiled_panes
.render(output, self.floating_panes.panes_are_visible());
.render(output, self.floating_panes.panes_are_visible())
.with_context(err_context)?;
if self.floating_panes.panes_are_visible() && self.floating_panes.has_active_panes() {
self.floating_panes.render(output);
self.floating_panes
.render(output)
.with_context(err_context)?;
}

// FIXME: Once clients can be distinguished
Expand Down
28 changes: 21 additions & 7 deletions zellij-server/src/ui/boundaries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::panes::terminal_character::{TerminalCharacter, EMPTY_TERMINAL_CHARACT
use crate::tab::Pane;
use ansi_term::Colour::{Fixed, RGB};
use std::collections::HashMap;
use zellij_utils::errors::prelude::*;
use zellij_utils::{data::PaletteColor, shared::colors};

use std::fmt::{Display, Error, Formatter};
Expand Down Expand Up @@ -47,18 +48,29 @@ impl BoundarySymbol {
self.color = color;
*self
}
pub fn as_terminal_character(&self) -> TerminalCharacter {
if self.invisible {
pub fn as_terminal_character(&self) -> Result<TerminalCharacter> {
let tc = if self.invisible {
EMPTY_TERMINAL_CHARACTER
} else {
let character = self.boundary_type.chars().next().unwrap();
let character = self
.boundary_type
.chars()
.next()
.context("no boundary symbols defined")
.with_context(|| {
format!(
"failed to convert boundary symbol {} into terminal character",
self.boundary_type
)
})?;
TerminalCharacter {
character,
width: 1,
styles: RESET_STYLES
.foreground(self.color.map(|palette_color| palette_color.into())),
}
}
};
Ok(tc)
}
}

Expand Down Expand Up @@ -528,16 +540,18 @@ impl Boundaries {
}
}
}
pub fn render(&self) -> Vec<CharacterChunk> {
pub fn render(&self) -> Result<Vec<CharacterChunk>> {
let mut character_chunks = vec![];
for (coordinates, boundary_character) in &self.boundary_characters {
character_chunks.push(CharacterChunk::new(
vec![boundary_character.as_terminal_character()],
vec![boundary_character
.as_terminal_character()
.context("failed to render as terminal character")?],
coordinates.x,
coordinates.y,
));
}
character_chunks
Ok(character_chunks)
}
fn rect_right_boundary_is_before_screen_edge(&self, rect: &dyn Pane) -> bool {
rect.x() + rect.cols() < self.viewport.cols
Expand Down
23 changes: 15 additions & 8 deletions zellij-server/src/ui/overlay/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
pub mod prompt;

use crate::ServerInstruction;
use zellij_utils::errors::prelude::*;
use zellij_utils::pane_size::Size;

#[derive(Clone, Debug)]
Expand All @@ -19,7 +20,7 @@ pub struct Overlay {
pub trait Overlayable {
/// Generates vte_output that can be passed into
/// the `render()` function
fn generate_overlay(&self, size: Size) -> String;
fn generate_overlay(&self, size: Size) -> Result<String>;
}

#[derive(Clone, Debug)]
Expand All @@ -28,9 +29,11 @@ pub enum OverlayType {
}

impl Overlayable for OverlayType {
fn generate_overlay(&self, size: Size) -> String {
fn generate_overlay(&self, size: Size) -> Result<String> {
match &self {
OverlayType::Prompt(prompt) => prompt.generate_overlay(size),
OverlayType::Prompt(prompt) => prompt
.generate_overlay(size)
.context("failed to generate VTE output from overlay type"),
}
}
}
Expand All @@ -44,15 +47,17 @@ pub struct OverlayWindow {
}

impl Overlayable for OverlayWindow {
fn generate_overlay(&self, size: Size) -> String {
fn generate_overlay(&self, size: Size) -> Result<String> {
let mut output = String::new();
//let clear_display = "\u{1b}[2J";
//output.push_str(&clear_display);
for overlay in &self.overlay_stack {
let vte_output = overlay.generate_overlay(size);
let vte_output = overlay
.generate_overlay(size)
.context("failed to generate VTE output from overlay window")?;
output.push_str(&vte_output);
}
output
Ok(output)
}
}

Expand All @@ -70,8 +75,10 @@ impl Overlay {
}

impl Overlayable for Overlay {
fn generate_overlay(&self, size: Size) -> String {
self.overlay_type.generate_overlay(size)
fn generate_overlay(&self, size: Size) -> Result<String> {
self.overlay_type
.generate_overlay(size)
.context("failed to generate VTE output from overlay")
}
}

Expand Down
Loading

0 comments on commit c5b1eb0

Please sign in to comment.